feat(016): harness-native security tooling — structural search, a durable finding ledger, and an external scanner tier that fails closed - #12
Merged
Conversation
bee can confine what a model does but gives it little worth doing in a security investigation — the tool surface is bash, file access, and regex search. Anything more means installing a pile of scanners and widening the executable allowlist for each one. This is the spec kit for a two-tier tool surface organised by one rule: build it into the harness when a maintained crate is the actual engine, shell out only when the value is a curated rule corpus or a heavy non-Rust runtime. The native tier runs as a bee subcommand inside the scope, exactly as `search` already does, so the LSM mediates every open. The external tier wraps a scanner as an adapter — bee builds argv from typed inputs, the binary enters the allowlist inode-pinned through the existing grant path, and its SARIF is normalised into bee's own finding shape. Three measurements shaped the design rather than being discovered later: - An Opengrep SARIF over one 3-line file is 1,912,546 bytes, of which the results array is 839 — 99.96% is the embedded 1074-rule block, and the whole document is 19x DEFAULT_OUTPUT_CAP. Piping it through stdout would truncate into unparseable JSON and surface as "found nothing", the exact failure FR-012 forbids. Hence a two-child pipeline: the scanner writes SARIF to a scope-internal path, then `bee sarif-worker` normalises and bounds it, also in scope. The harness never opens the report. - opengrep exits 0 with findings present; it only returns non-zero under --error. Success comes from invocations[].executionSuccessful, never the exit code. And --config auto needs network egress a scanning scope does not have, so it is refused at argv construction. - CodeQL's compiled-language extraction pushes --begin-tracing and --trace-process-name because it intercepts the build's process spawns. That would mean admitting every compiler and linker a build invokes — an unbounded, un-attenuable widening — so build-mode:none languages only, with traced ones declined explicitly. MSRV is split by audience: ast-grep-core 0.45 needs 1.88, so the application package declares it while the embeddable core crates keep their 1.85 promise. One line, and Cargo can express it, unlike a feature-conditional floor. Constitution check passes with no violations. Principle I is carried structurally by a three-state ToolOutcome where Completed-with-empty is unreachable from any error path — a missing scanner reporting "no issues" is the worst failure mode a security harness has, and this is the same fail-closed doctrine as a32e156 applied one layer up. 69 tasks across 9 phases; MVP is the first 22. Everything through US4 is host-testable, so the first slice needs no VM. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ol seam Implements the 016 MVP: Setup, Foundational, US1 (ast_grep) and US4 (cvss). 26 of 69 tasks. US2 (finding ledger), US3 (scanner adapter), US5 (gix history) and US6 (CodeQL) are specified and contracted but not built. The foundational piece is `ToolOutcome<T>` — Completed / Unavailable / Failed — which makes Constitution I structural rather than a matter of discipline. `Completed` with an empty payload means "ran and found nothing" and is unreachable from any error path; everything that could not run is `Unavailable`, everything that ran and broke is `Failed`, and neither rendering shares phrasing with a clean result. A missing scanner reporting "no issues" is the worst failure mode a security harness has, because the operator reads the silence as assurance. Nine tests in tests/fail_closed_tools.rs hold that line, including one asserting the payload renderer never runs on a refusal. `ast_grep` follows the seam src/search.rs established: the tool execs `bee astgrep-worker` through run_child, so the tree-sitter parse happens in a scope-joined child and every file it opens is LSM-mediated. It earns its place beside `search` by matching the parse tree rather than bytes — the fixture holds `v.unwrap()` three times, as a call, in a comment, and in a string literal, and only the call is returned (SC-007). Two things surfaced during implementation that the plan had not anticipated, both recorded as research R12/R13: - `SupportLang` keeps every variant regardless of which grammar features are enabled; only the parser lookup is conditional, and its off-branch is `unimplemented!()`. Asking for an uncompiled language would panic rather than refuse. bee therefore answers "is this language available?" from its own LANGS table and never constructs a SupportLang it cannot back. That table also feeds the tool schema's `lang` enum. - `Pattern::try_new` accepts syntactically broken patterns, because tree-sitter recovers from bad input. `fn $(((` builds fine and then matches nothing — exit 0, no output, indistinguishable from a clean scan. That is the FR-012 confusion arriving through the one input the model authors freely, so patterns are additionally gated on `Pattern::has_error()`. `cvss` is the FR-005 requirement: the model states the vector, the crate computes the number. Writing its reference test demonstrated the point — a hand-estimated 1.6 for AV:L/AC:H/PR:H/UI:R/C:L was wrong, and working the formula through gives the 1.8 the crate returns. The tool registry lists only tools that exist. git_log, record_finding, list_findings and scan are absent rather than present-and-broken, so a scenario naming one gets the ordinary unknown-tool rejection, which is the truthful answer today. MSRV is split by audience: the application package declares 1.88 for ast-grep-core while the embeddable core crates keep 1.85. gix needs an explicit `sha1` feature under default-features = false — gix-hash refuses to compile without a hash backend. Every feature builds standalone and the default build is unchanged. Constitution V is held by extending tests/core_deps_guard.rs with the five new crates. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…only a human can write
Implements US2 of 016-native-tools, plus T053 from US4. 13 of the slice's
remaining tasks; US3 (external scanners) follows in the next commit.
A finding that lives only in a transcript dies with the episode. The next
run rediscovers it, the operator re-reads it, and any judgement they
formed — *this one is a false positive, we sanitise upstream* — has to be
formed again. Two requirements fix that and pull against each other:
re-runs must merge (FR-018/019), and a human's verdict must outrank every
later automated rediscovery (FR-020).
A mutable findings.json satisfies neither safely. Merging becomes
read-modify-write, which races, and a rediscovery that rewrites an entry
can drop the verdict attached to it. So .bee/findings/ledger.jsonl is
append-only and fold() derives the current state: merging is a property of
the fold rather than of a write, and a verdict cannot be overwritten
because nothing is ever overwritten. The log is text, one self-contained
JSON object per line, meant to be committed and reviewed in a pull
request (FR-023, Constitution IV).
Identity is hash(path ␟ class ␟ normalised title) and deliberately
excludes the line number: code moves, and an identity that included the
line would mint a duplicate every time someone added an import above it.
Lines are attributes of a sighting. The ␟ separator is not decoration —
bare concatenation lets ("a/b", "c") and ("a", "b/c") collide into one
entry.
Adjudication is `bee findings adjudicate`, and has no tool equivalent.
The value of a verdict is that a person formed it; if the agent could
write verdicts, "a human marked this a false positive" would stop meaning
anything and FR-020 would protect nothing, since the same actor would sit
on both sides of it. So the model-facing surface is append-a-sighting and
the operator-facing surface is append-a-verdict.
Two things surfaced during implementation:
- MAX_EVENT_BYTES was specified as 4096. PIPE_BUF *is* 4096, and what is
written is the event plus its newline — so a maximal event writes 4097
bytes and falls one past the atomicity the whole lock-free design
relies on. The cap is 4000. The concurrent-append test (8 writers × 40
events, asserting all 320 lines present and individually parseable) is
what exposed it; the research had left that test as optional, and
writing it was the right call.
- Severity may not be asserted (FR-005), and the honest enforcement is
refusal rather than recomputation. record_finding takes a *vector* and
the score is computed from it; a call carrying a score is rejected
outright, because silently discarding it would leave the caller
believing the ledger holds their number.
`[security]` joins the layered configuration as an operator-only section,
beside `[policy] ceiling`. findings_dir answers the read-only-project
question (research open question 2): the ledger is bee's own state, the
same category as the transcript, so when the project cannot hold it, it
moves rather than forcing the project writable. When neither location can
be written, recording *fails* — a finding reported as recorded but absent
from the ledger is the worst outcome available.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ThGXvjMjNZ9jGs9UxU1QiX
…s a clean one Implements US3 of 016-native-tools, plus the SC-006 and Constitution II gates (T063/T064) and the tool-surface documentation (T067). US5 (gix history) and US6 (CodeQL) remain specified and unbuilt. bee borrows a scanner's *rule corpus* rather than reimplementing it, and gives back the part the scanner does not have: running it inside an enforced scope, deciding its argv from typed inputs, and refusing to report a run that failed as a run that found nothing. Three structural decisions carry the security properties. **Two children, not one.** Measured: an Opengrep SARIF over a single three-line file is 1,912,538 bytes, of which 99.96% is the embedded rule catalogue, to deliver 839 bytes of results — 19× DEFAULT_OUTPUT_CAP. Capturing that on stdout truncates it into unparseable JSON, which surfaces as "the scanner found nothing": exactly the failure FR-012 exists to prevent. So the scanner writes a file and `bee sarif-worker` normalises it. That the normaliser is a *second scope-joined child* rather than harness code is the whole point — reading the report in the harness would open a file inside the sandbox (Constitution III). **Success comes from the report, never the exit status.** Measured: `opengrep scan --sarif --quiet` exits 0 with a finding present; it only returns non-zero on findings when --error is passed. Exit status conflates "findings exist" with "run failed" and is wrong in both directions. The ladder is: finished within budget → report parses → executionSuccessful → only then a result. **Two authorities, separated.** Whether a binary may execute is a capability and lives in policy as an inode-pinned exec.allow entry, going through the ordinary derive/ceiling path. How it is configured — which rules, what timeout — confers no authority on its own and lives in the operator-only `[security.scanners.<name>]` config, so policy keeps exactly the surface it had. What gets scanned is the model's to choose; ScanRequest has three fields and none of them is free-form (FR-007). One decision stricter than the plan called for: an *unpinned* exec.allow entry does not grant a scanner. An unpinned entry authorises the path, so a binary swapped at that path still runs — precisely what SC-010 forbids. For an ordinary tool that is the operator's trade to make; for a third-party binary running over the code bee is meant to be securing, it is not a trade worth offering. probe() also re-reads the inode immediately before spawn, so a swap between grant and run is caught in a host build where no LSM is holding the same pin. Two bugs found while testing, both worth stating because the tests nearly missed them: - The report path was keyed on run_id, so two scans in one episode wrote the same file and each parsed what the other was still writing. Now unique per call. Parallel tests found it; two concurrent episodes sharing a project would have found it in production. - remove_file + recreate readily *reuses* the inode, which left the pin matching and quietly turned both pin-mismatch tests into tests of nothing. They now swap via rename and assert the inode actually changed first. tests/scanner_adapter.rs covers every fail-closed case against a stub scanner — no grant, missing binary, substituted binary (with a sentinel proving it never executed), no report written, unparseable report, timeout, executionSuccessful:false — and asserts each is distinguishable from a clean scan. A genuinely clean scan still reads as clean, which is the other half of FR-012. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ThGXvjMjNZ9jGs9UxU1QiX
Walking quickstart.md end to end (T066) turned up a gap in the FR-005 guard.
`RecordArgs` traps a caller-supplied `severity_score`, but that is one spelling.
A caller who nested `severity = { score: 9.8 }` had the whole object dropped by
serde: no score reached the ledger, so FR-005 was never violated — but the
caller was silently ignored rather than told, which is the precise failure the
trap field's own doc-comment says it exists to prevent. `deny_unknown_fields`
closes every other spelling at once.
The walkthrough also found quickstart.md documenting commands the binary does
not have. `bee scan` and `bee cvss` were never subcommands — both are
model-facing tools, and a scan is something an episode does under the policy
that grants it, so the live path goes through a scenario rather than a flag.
(`--policy` is mutually exclusive with `--host`, so an unenforced build reaches
the grant path only via a scenario's own `policy_path` — worth stating, since
SC-011 promises the whole slice is host-testable, and it is, just not that way.)
`--test cvss_tool` named a target that does not exist; the cvss tests are unit
tests, because a tool that opens no files and spawns no child gives an
out-of-process test nothing to observe. Every command in the file is now one
that was actually run.
Two findings left open rather than patched, filed as T070 and T072: the SARIF
adapter cannot populate `advisory_level` without materialising the rule
catalogue that research R4 says not to read, and a repl_command test fails on
any non-enforce build — pre-existing on main, verified on both branches.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a security-analysis tier to bee, in two halves: tools whose engine is a Rust crate compiled into the binary, and adapters for external scanners the operator explicitly grants. Every family is default-off, so a build that selects none of them has the dependency graph it had before this branch.
The organising constraint is Constitution I: a tool that could not run must never render as a tool that ran and found nothing.
ToolOutcome<T>makes that a type rather than a convention —Unavailable { reason }andFailed { reason }have no rendering in common with a clean scan, and each carries an audit event.What ships (US1–US4)
ast_grep— structural search over the tree-sitter parse tree, so the same construct in live code, in a comment, and in a string literal are three different things and only the first is returned. One Cargo feature per grammar; a language that is not compiled in says what is, rather than returning empty..bee/findings/, folded into a view. Identity excludes the line number, so a finding whose line moved merges rather than duplicating. Human verdicts survive automated rediscovery, andbee findings adjudicateis CLI-only: a verdict is worth something because a person formed it, so it is never a model-facing tool.exec.allowgrant. A binary merely present onPATHis not a grant; a binary substituted after the grant is never executed. The scanner's SARIF is written to a file and normalised by a second scope-joined child, because the report is ~1.9 MB of which 99.96% is rule catalogue — 18.7× the output cap. Success is read from the report, never from the exit code.cvss— the model states the vector, the crate computes the score. A caller-supplied score is refused rather than silently recomputed.Deferred, already contracted
git_logviagix(US5) and the CodeQL bundle (US6). Both are specified incontracts/; neither is in this slice.Verification
quickstart.mdwas walked end to end on a clean checkout (T066) — every command in it has now actually been run, and four that had drifted from the binary were corrected. The whole slice is host-testable with no kernel enforcement and no VM (SC-011).Two known items are filed rather than fixed, both recorded in
tasks.md:Finding.advisory_levelcannot be populated from Opengrep without materialising the rule catalogue that research R4 says not to read. Harmless (an advisory level never becomesSeverity), but a field that can never be filled is a lie in the data model.repl_command::no_provider_at_all_reports_what_is_missingfails on any non-enforcebuild. Pre-existing onmain, verified by running it on both branches; it is the only failing test in the suite.cargo fmt --checkandcargo clippy --workspace --all-targets --features sec,astgrep-rust,astgrep-python -D warningsare clean.core_deps_guardconfirms no new crate reachedbee-core/bee-common.🤖 Generated with Claude Code