Skip to content

Add annotate for bulk comments/renames, and say what to do about --raw - #225

Open
subwire wants to merge 1 commit into
mainfrom
bulk-annotation
Open

Add annotate for bulk comments/renames, and say what to do about --raw#225
subwire wants to merge 1 commit into
mainfrom
bulk-annotation

Conversation

@subwire

@subwire subwire commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

Problem

Two remaining gaps from D3CTF 2026, where we ran DecLib as the sole decompiler interface for a fleet of autonomous CTF agents.

Annotation is produced in bulk. You read a binary, build up a mapping of address to label, then want to write it back. Applying that one comment set at a time is N round-trips — so people bypassed the CLI:

  • One challenge passed renames={0x5440: 'detect_branch_trampoline', 0x5674: 'detect_any_hook', ...} straight to DecompilerClient.
  • Another skipped DecLib entirely — import idapro, open the .i64, loop ida_funcs.set_func_cmt.

--raw inside batch was rejected with no hint about the alternative (7 hits in our logs).

Change

decompiler annotate --file notes.json
echo '{"0x401000": "parses the header"}' | decompiler annotate

Accepts either a list of records:

[{"addr": "0x401000", "comment": "parses the header", "name": "parse_hdr"}]

…or the shorthand {addr: comment} object — the shape people already build by hand.

Backed by apply_annotations on DecompilerInterface, which (like decompile_many in #222) defaults to an in-process loop and so collapses N round-trips into one for every backend, with no per-backend code.

Three deliberate robustness choices:

  • Renames are verified by read-back, not assumed. A backend can accept the write and quietly not apply it, and a silently miscounted rename is worse than a reported failure.
  • Per-item failures are returned to the caller (address, field, reason) rather than only reaching the server log, and one bad record no longer discards the batch.
  • Failure text is never blank. An exception raised without arguments has an empty str(), which surfaced as failed 0xdeadbe00 (name): with no reason — the same class of bug as Propagate server-side error text instead of dropping it #220. It now reads KeyError (no message).

For batch, the --raw rejection stays (raw text on stdout would corrupt structured results), but the message now names the replacement: the text is already in results[].result.text.

Verification

Against IDA on a real binary:

case result
combined comment + rename comments: 1, names: 1, failed: 0; both verified present afterwards
shorthand {addr: comment} 2 comments applied
unmapped address failed: 1, reason KeyError (no message), exit 1
partial batch (1 good, 1 bad) good one applied, bad one reported, exit 1
clean batch exit 0

Tests

Bulk apply keeps going past a bad record; missing-address and empty records are reported rather than raised; a blank exception cannot produce a blank reason; the --raw message names its replacement.

🤖 Generated with Claude Code

@mahaloz

mahaloz commented Jul 26, 2026

Copy link
Copy Markdown
Member

Fix conflicts plz.

Two remaining gaps from a CTF where DecLib was the sole decompiler
interface.

**Bulk annotation.** Annotation is produced in bulk -- you read a binary,
build a mapping of address to label, then want to write it back. Applying
that one `comment set` at a time is N round-trips, so people drove the
Python client directly instead: one challenge passed
`renames={0x5440: 'detect_branch_trampoline', ...}` straight to
DecompilerClient, and another skipped DecLib entirely, opening the IDB
with idapro and calling ida_funcs.set_func_cmt in a loop.

    decompiler annotate --file notes.json
    echo '{"0x401000": "parses the header"}' | decompiler annotate

Accepts a list of {"addr", "comment"?, "name"?} records, or the shorthand
{addr: comment} object -- the shape people already build by hand. Backed
by `apply_annotations` on DecompilerInterface, which like decompile_many
defaults to an in-process loop and so collapses N round-trips into one for
every backend.

Renames are verified by read-back rather than assumed: a backend can
accept the write and quietly not apply it, and a silently miscounted
rename is worse than a reported failure. Per-item failures are returned to
the caller (address, field, reason) instead of only reaching the server
log, and one bad record no longer discards the batch.

Failure text is never blank. An exception raised without arguments has an
empty str(), which surfaced as "failed 0xdeadbe00 (name): " with no reason
at all -- the same class of bug as the propagate-server-error-text branch.
It now reads "KeyError (no message)".

**`--raw` inside `batch`.** The rejection is correct -- batch results are
structured, and raw text on stdout would corrupt them -- but "not allowed"
left the caller guessing, and we logged 7 hits on it. The message now
names the replacement: the text is already in results[].result.text.

Tests: bulk apply keeps going past a bad record, missing/empty records are
reported rather than raised, a blank exception cannot produce a blank
reason, and the --raw message names its replacement.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@subwire

subwire commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator Author

Rebased onto current main — conflicts resolved, mergeable: true.

The conflicts were all against #222 landing (decompile_many / cmd_export sit right next to apply_annotations / cmd_annotate), so both sides are kept in full. Verified after resolving: everything parses, no duplicated definitions were introduced, and both feature sets test green together (TestAnnotateBatch, TestBatchRawMessage, TestExportArgs — 8 tests, all passing).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants