fix(egress): make the high-volume flag's documented contract true - #96
Merged
Conversation
`_egress_flag_bytes` guards only against non-numeric values, so `=0` is all-digits and reaches the comparison as a real 0. The human receipt renderer always guarded this (`thr+0>0` in the awk), but the JSON path did not: with a 0 threshold every reached host satisfied `bytes >= thr` and was written `"high_volume":true` - including a 0-byte row. The one value a user would pick to turn the flag OFF turned it on for everything, and the persisted record is the surface `egress --export` and `ls --json` re-serve. Guards the threshold before the comparison so both renderers agree, in an `if` rather than a longer && chain so it cannot trip errexit. Covers `=0` on a normal and a zero-byte row, plus the malformed-value fallback that was never asserted. Verified as a real tripwire: the =0 case fails against the unfixed slice, not just passes against the fixed one.
…ess` docs/configuration.md documented `sluice egress --json` as emitting `"high_volume":true` on a host row. It never did - the only emitter was _persist_receipt, so the field rode only on the at-exit receipt and the records `egress --export` and `ls --json` re-serve. A CI gate written against the documented contract parsed for a key that was never there and silently never fired. This makes the documented behaviour true rather than weakening the doc. Also extends cmd_egress's byte render through GB/TB. The ladder shipped in the receipt renderer only, so `sluice egress` still printed a 5 GiB transfer as "5222.4 MB" while the receipt beside it said "5.10 GB" - the same volume, two answers, on the two surfaces a reader compares first. The ladder was already duplicated between _human_bytes and the receipt awk; adding a third copy here would have made the unit boundaries a set of magic numbers in three places that must agree or the surfaces disagree. Hoisted it to one _AWK_HUMAN fragment instead, prepended as awk program text at each site. That keeps it fork-free - these rows format inside a single awk pass, so calling _human_bytes per row would fork an awk per host, the opposite direction from f17b632 - while leaving exactly one definition. The B branch unifies on sprintf("%d B"), which coerces a non-numeric count to 0 rather than echoing it back into the render; the two inline copies previously concatenated it raw. The flag compares tx+rx (field 4), so a large inbound transfer trips it too, and the boundary is inclusive - both now pinned by tests. Note cmd_egress reads the whole-boot window while the receipt is run-scoped, so the same threshold flags at least as often here; that is a decision, not an accident. Verified under one-true-awk, mawk and gawk (make test-awk, 0 failures) and by eyeballing the real render under each - byte-identical output on all three.
Three fixes to what SLUICE_EGRESS_FLAG_BYTES actually does, now that `sluice egress` carries the field: - "a single reached host's tx bytes" understated it. The flag compares field 4 of egress_rows, and that accumulator is `tx + rx` - a large inbound transfer trips it exactly as an upload does. Reading the old wording, someone tuning the threshold against an upload budget would set it too high. - Names every surface that carries the tag rather than the one that did not. - States the `0`-disables and non-numeric-falls-back contract, which was never written down. Sibling bullets are explicit about malformed handling; this one was silent while `=0` was the value most likely to be reached for. Also registers `high_volume` in the operations.md schema table, which declares itself the single registry for JSON contract fields - it was emitted on every persisted record but absent from the registry, so anything generated from that table missed it. Links the knob rather than restating the default a third time.
Two guards a user could not learn from the docs: - SLUICE_ALLOW_HOME appeared only in THREAT_MODEL.md, which states the guarantee but is not where anyone looks up a knob. It is the sole escape from a hard refusal, and it is settable in sluice.config.sh (the config is sourced before the guard dispatches), so it belongs in Wiring rather than under "Environment-only knobs". - hardening.md described the mask's symlink handling backwards: "symlink matches are skipped (a mount over a link would shadow its target)". That was true before the mask began resolving a matched link physically and shadowing its in-project target. A reader following the old text would believe a symlinked .env was exposed and go build a second mechanism for it. The git-history limit stays owned by THREAT_MODEL.md, which hardening.md already links - restating it here is how this paragraph drifted in the first place.
…ount forbid-laundering is documented as a ceiling like any other, but it has a consequence nothing recorded: every shipped agent preset allowlists a model or stream API that laundering_host classifies, so a policy carrying the directive refuses all ten. An operator would deploy it org-wide and discover that `sluice agent` no longer runs anywhere. Notes too that SLUICE_LAUNDERING_OK silences the session warning but does not override a policy refusal - it is checked on the warn path, never in apply_policy - and that the directive takes no argument, so the remedy is selective deployment rather than a relaxation. This is a standing property, not fallout from the recent classification work: most presets already refused before it; that work took coverage to all ten. agents.md gains the matching two sentences on the session-start note, linking the knob and the gate rather than restating either. The preset count was stale in ROADMAP.md (nine, ten ship) and in the agent-demo alt text. The count is a straight fix in ROADMAP; in the alt text it is not - the GIF was recorded before the tenth preset and genuinely renders nine rows, so "ten" would make the alt lie about the image beside it. Both alts drop the count instead, which is honest against either recording; docs/agents.md keeps the full description and the README carries a shorter teaser, so there is one owner to re-sync when the GIF is re-recorded.
The repo carries no in-tree release history, so the only way to find what changed in a version was to already know the Releases page exists. A pointer rather than a CHANGELOG.md: the GitHub releases already carry Keep-a-Changelog bodies and nothing generates them from a file, so a committed copy would be a second thing to hand-maintain and a second thing to drift.
Findings from an adversarial pass over this branch. Each was reproduced against the real code before being taken as real. - SLUICE_EGRESS_FLAG_BYTES accepted any all-digit string. At >= 2^63 the shell comparison emits "[: integer expected" on stderr once per reached host, which breaks a `2>&1 | jq` consumer; between 2^53 and 2^63 the shell compares exactly while awk compares as an IEEE double, so the JSON and human renders disagree about the same host. Bounded to 15 digits at the single owner, which fixes both renderers and both call sites at once. ~1 PB is far past any real threshold. - The shared awk ladder's `%d` was documented as coercing a non-numeric byte count to 0. It does not: a non-numeric value string-compares past every rung and falls out the bottom as "0.00 TB" - a garbage count rendering as petabytes on an audit surface. Added `b+=0` so the claim is true, since the point of one shared ladder is that the next call site can rely on it. Every current caller gates on *[!0-9]* first, so this is defence for the one that forgets. - The errexit rationale on the =0 guard was wrong: `&&` lists are already exempt, so the old chain could never have tripped it. Dropped the claim; the `if` and the `-gt 0` term stay, the latter being the actual fix. - docs/hardening.md's new symlink sentence asserted a safety property that does not hold: a target outside the mount was said to "already dangle in the box". In a linked git worktree the git common dir IS mounted, so a masked-pattern symlink pointing into it resolves and stays readable - while doctor reports masked:[], unmasked_secrets:[] and broken_symlinks:[]. mask_matches scopes to PROJECT_DIR while symlinks_outside_scope scopes to PROJECT_DIR plus the common dir, and that divergence is the mechanism. The code behaviour is pre-existing; only the false justification was new. Names the exception instead. Also anchors a test that could not fail: the under-threshold human-render case refuted a tag without asserting the row rendered, so it passed when cmd_egress early-returned "nothing yet" - and passed unchanged on main. And unsets SLUICE_EGRESS_FLAG_BYTES in the receipt suite's setup, so an exported value in the environment cannot rewrite what the default-threshold assertions mean.
demo-beats designs the VHS tape beats for what has actually shipped: it inventories user-visible capabilities since a tag, scores each on three independent lenses (security story / recordability / coverage gap), designs a beat for the survivors, then has a skeptic try to make each recording pass for the wrong reason. Defaults to main, because a beat designed against an unmerged branch can be designed against code that changes in review; args.ref and args.include_uncommitted opt into unmerged work when that is what you want. The skeptic stage is the point. A laundering tape was once rejected because resolve_engine runs ~725 lines before warn_laundering, so with no engine on PATH the command dies early - and since die also exits 1, a tape asserting `|| echo "refused (exit $?)"` still prints its success line and the recording proves nothing. Every beat now carries a cannot_pass_vacuously field, and the shortlist step must argue explicitly about any capability the lenses disagree on (one was previously dropped with no argument made against it). attack-changes parsed args unconditionally as JSON, so the invocation its own skill documents (`args: "main"`) threw before a single agent started - it had never run via that path. preflight already had the tolerant form; both now accept a bare ref, an object, or an object as a JSON string.
…anges is due triage-tests was the only workflow here with no adversarial stage: it root-caused each failure cluster and returned the diagnosis unchecked. A root cause is a claim until someone re-derives it, and acting on a plausible-but-wrong one means editing the wrong file - or rewriting a test to agree with broken code, which CLAUDE.md names as how a broken `ls` shipped with a green suite. Adds a skeptic per diagnosis that must establish the CAUSAL link rather than a correlation, rule out an environmental cause (Docker down, a stale bin/sluice), and decide explicitly whether the TEST or the CODE is wrong - surfaced as test_is_the_bug so it can never be quietly resolved by making the test pass. preflight now reports when the diff touches a security path and the attack pass is warranted, rather than invoking it. Composing them was the alternative and is worse: attack-changes runs ~20 agents for ~20 minutes, an order of magnitude past this gate, and preflight is run on every branch while CLAUDE.md warrants the attack pass only on a security path. Auto-escalation would bury that spend in a routine check. The gate agent classifies from what the changed lines DO, not the filename, so a docs-only touch of an egress file does not trip it.
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.
docs/configuration.mddocumentedsluice egress --jsonas emitting"high_volume":trueon a host row. It never did — the only emitter was thepersisted receipt. A CI gate written against the documented contract parsed for a
key that was never there and silently never fired. This makes the documented
behaviour true rather than weakening the doc, then corrects what the docs say
about the measure and the surfaces.
Code
high_volumeand the GB/TB ladder now reachsluice egress(human and--json). The ladder previously shipped in the receipt renderer only, so a5 GiB transfer printed as
5222.4 MBon one surface and5.10 GBon the other.SLUICE_EGRESS_FLAG_BYTES=0disables the flag. It is all-digits, so itpassed the numeric guard and reached the comparison as a real
0; every reachedhost then satisfied
bytes >= 0, including 0-byte rows. The value you would pickto turn the flag off turned it on for everything.
[: integer expectedon stderr once per reached host, breaking a2>&1 | jqconsumer; between 2^53 and 2^63 the shell compared exactly while awk compared as
an IEEE double, so the two renderers disagreed about the same host.
The unit ladder was already duplicated between
_human_bytesand the receipt awk.Rather than add a third copy, it is hoisted to one
_AWK_HUMANfragment prependedas awk program text — one definition, still zero forks per row (these format
inside a single awk pass, so calling
_human_bytesper row would fork an awk perhost).
grep 1099511627776 src/returns one line where it returned three.Docs
The measure was documented as "tx bytes"; the accumulator is
tx + rx, so a largedownload trips it too. Also registers
high_volumein theoperations.mdschematable that declares itself the single registry, documents
SLUICE_ALLOW_HOME(theonly escape from a hard refusal, previously in no knob reference), corrects the
SLUICE_MASKsymlink limit — which described the behaviour backwards — and recordsthat
forbid-launderingrefuses every shipped agent preset.Verification
Adversarial pass over the branch diff found 10 confirmed defects, 9 new,
all fixed here. Three were false claims in my own comments and docs:
%din the shared ladder was documented as coercing a non-numeric count to0. It does not — a non-numeric value string-compares past every rung and renders
0.00 TB, a garbage count reading as petabytes on an audit surface. Fixed withb+=0so the claim is true.=0guard was wrong:&&lists are already exempt.docs/hardening.mdasserted a symlink target outside the mount "already danglesin the box". In a linked git worktree the git common dir is mounted, so a
masked-pattern link into it stays readable while
doctorreports three greenchecks. The code behaviour is pre-existing; only the false justification was new.
One new test could not fail — it refuted a tag without asserting the row rendered,
so it passed when
cmd_egressearly-returned, and passed unchanged onmain. Nowanchored.
414 unit tests, 0 failures.
make test-awkclean under one-true-awk, mawk andgawk; real render eyeballed under all three, byte-identical. Linux VM leg not run —
relying on CI's
Linux, Docker.🤖 Generated with Claude Code
https://claude.ai/code/session_017TqRkhSka9ArLr4svJv858