Conversation
The legend said a smoke token names a check in script/smoke.sh, but the checker grepped script/*.sh, so a smoke token could resolve against script/claims-vm.sh, which CI never runs. Grep smoke.sh alone, the file the legend names. Both pending rows listed security.md as where the claim appears, but neither sentence is in that file yet. Say the sentence lands with its PR instead of pointing at a place it is not. Refs: #76 Signed-off-by: Panagiotis Moustafellos <pmoust@nofire.ai>
The legend said a smoke token names a check in script/smoke.sh, but the checker grepped script/*.sh, so a smoke token could resolve against script/claims-vm.sh, which CI never runs. Grep smoke.sh alone, the file the legend names. Both pending rows listed security.md as where the claim appears, but neither sentence is in that file yet. Say the sentence lands with its PR instead of pointing at a place it is not. Refs: #76 Signed-off-by: Panagiotis Moustafellos <pmoust@nofire.ai>
66a73a9 to
886f598
Compare
docs/security.md makes specific, testable promises: the guest sees one directory, a symlinked workspace is refused, an image under our registry with a bad signature stops the boot, credentials are forwarded by name and not ambient, a denied billing key does not reach the guest. Nothing in CI proved any of them, so a promise could quietly stop being true in a refactor and no test would notice. Several of these have regressed or nearly regressed, which is why this matters now. Add docs/claims.md, a table with one row per public claim: the exact sentence, where it appears, and the test that defends it. Each test is named with a typed token so the mapping is machine-checkable rather than prose a reader has to trust. Add script/check-claims.sh, run in CI, which fails when a row names a Go test or a smoke check that does not exist. A row without a live test is the drift this catches, so a rename that orphans a claim fails the build rather than the next refactor. Its --self-test mode proves the guard is live by running it against a row that names a missing test. Most claims already had tests, named here rather than duplicated. Two new checks go into script/smoke.sh on the run path: a denied billing key never reaches the guest env line, and only the declared credential names do. Both assert on the run's argv, because brig hands the runtime child its whole environment, so a value being visible to the stub proves nothing about the guest. The isolation boundary needs a real VM to prove, since a stub runtime cannot show that a host file is unreadable from the guest. Those rows are marked as needing a VM and defended by the make claims-vm target, which boots a real sandbox where a runtime exists and skips cleanly where none does. Two further claims, the BRIG_ALLOW_DENIED fail-open switch and digest pinning, live on branches not yet merged; their rows are present and marked pending so the claims are tracked until the tests land. Fixes: #22 Signed-off-by: Panagiotis Moustafellos <pmoust@nofire.ai>
check-claims.sh resolves both go: and smoke: tokens, but its --self-test fixture held only a go: row. The two tokens resolve through independent paths (go test -list versus a grep over script/*.sh), so the smoke branch could break -- for instance a mutation that auto-passes every smoke row -- while the self-test and the real check both stayed green. smoke is 10 of the table's 24 tokens, so half the guard was unproven. Add a smoke: row naming a check that cannot exist alongside the go: row, and assert each is reported missing by name rather than only that the run failed. Checking the aggregate fail flag was the loophole: with either branch broken the other still trips the flag, so the aggregate stays 1 and the old assertion passed. Breaking either resolver now fails the self-test. Also clarify the pending BRIG_ALLOW_DENIED row. On this branch the behaviour is not merely untested, it is false: BRIG_ALLOW_DENIED=false still forwards the denied variable today, and #63 is the fix. Note that in the row and document that pending means "fixed on an unmerged branch", not "unknown", so a reader does not read pending as a passing claim. Refs: #22 Signed-off-by: Panagiotis Moustafellos <pmoust@nofire.ai>
The legend said a smoke token names a check in script/smoke.sh, but the checker grepped script/*.sh, so a smoke token could resolve against script/claims-vm.sh, which CI never runs. Grep smoke.sh alone, the file the legend names. Both pending rows listed security.md as where the claim appears, but neither sentence is in that file yet. Say the sentence lands with its PR instead of pointing at a place it is not. Refs: #76 Signed-off-by: Panagiotis Moustafellos <pmoust@nofire.ai>
Both pending rows named work that has since merged. BRIG_ALLOW_DENIED's strict reading landed with #63 and the digest pin with #67, so the rows now name the tests that defend them and the table has no pending rows at all. The sentences themselves were not in security.md, only in the table's Where-it-appears column, which is the same fault this PR fixed for the pending rows earlier: a column pointing at a claim the doc does not make. Both are true and tested now, so security.md states them: every off spelling of BRIG_ALLOW_DENIED leaves the denied variable behind, and the digest that booted is the digest that was verified. Refs: #22 Signed-off-by: Panagiotis Moustafellos <pmoust@nofire.ai>
886f598 to
059be4d
Compare
ananos
left a comment
ananos
left a comment
There was a problem hiding this comment.
I like this a lot -- a claim with no test being a visible row rather than a silent gap is the right mechanism, and the vm / pending states keep it honest about what CI can actually prove.
One gap to close before it comes out of draft, inline.
| | "A `scheme://` value read from the environment is refused as an unresolved secret-manager reference." | security.md, Credentials (the three rules) | `smoke:a secret-manager reference is not forwarded` + `go:TestUnresolvedReferencesAreRejectedButOrdinaryURLsAreNot` | existing | | ||
| | "Nothing is written into the workspace." (no credential lands in the workspace) | security.md, Credentials | `smoke:no credential is written into the workspace` | existing | | ||
| | "`BRIG_ALLOW_DENIED=false` does not forward the denied variable" (the fail-open switch) | security.md, Credentials | `go:TestOffSpellingsDoNotForwardADeniedCredential` | existing | | ||
| | "the digest that booted is the digest that was verified" | security.md, Guest images | `go:TestVerifyResolvesVerifiesAndPinsAMatchingDigest` | existing | |
There was a problem hiding this comment.
The table has no rows for the network claims, and #130 added two to security.md yesterday:
- a policy bound to a sandbox is enforced at the gateway, and a run that cannot enforce it is refused rather than booted unconstrained;
--network isolatedgives the sandbox a network of its own, and is refused where brig owns no network.
Both have tests already, so they would come in as existing rather than pending: the refusals are covered in script/smoke.sh and in internal/runtime (TestNerdctlRefusesAPolicyItCannotEnforce, TestIsolatedIsRefusedWhereBrigOwnsNoNetwork), and the enforcement itself is measured in docs/manual-tests/egress-policy.md, which is vm territory.
Worth rebasing on main and adding them -- a claims table that misses the newest claims is the exact failure it exists to prevent. Happy to write the rows if that helps.
ananos
left a comment
ananos
left a comment
There was a problem hiding this comment.
I still like this a lot. A claim with no test being a visible row rather than a silent gap is the right mechanism, and retiring the pending rows in 059be4d was the right call -- pending was the one status that could quietly mean "this claim is not defended" while looking tracked.
The typed-token scheme has aged well too: go: checked against go test -list, smoke: matched on its ok line, vm for what only a real boot can show. That a stub cannot prove isolation, and that nothing here pretends otherwise, is the honest part.
One gap, unchanged since I first read this, and now the more serious for it. Plus a note on where the branch sits.
| | Claim (exact sentence from the docs) | Where it appears | Defended by | Status | | ||
| | --- | --- | --- | --- | | ||
| | "the agent sees one directory and the credentials you chose to give it, and nothing else on the host." | security.md, intro | `vm` | needs a VM | | ||
| | "It does not have your keychain, your SSH agent, your secret manager, or any other directory on the host." | security.md, The boundary | `vm` | needs a VM | |
There was a problem hiding this comment.
The table still has no rows for the network and policy claims, and #130 merged three days ago, so docs/security.md on main now carries two promises this suite does not track:
- a policy bound to a sandbox is enforced at the gateway, and a run that cannot enforce it is refused rather than booted unconstrained;
--network isolatedgives the sandbox a network of its own, and is refused where brig owns no network.
Both already have tests, so they come in as existing rather than needing anything written. The refusals are covered by go:TestNerdctlRefusesAPolicyItCannotEnforce and go:TestIsolatedIsRefusedWhereBrigOwnsNoNetwork -- and note the first of those got stronger this morning: #149 moved the GUI refusal into nerdctl.CanRun so the join path is covered too, which is the same shape. The enforcement itself is measured in docs/manual-tests/egress-policy.md, which is vm territory by this table's own rule.
I am flagging it again rather than letting it go because it is precisely the failure this page exists to prevent: the newest security claims are the ones with no row, and a reader would take the table's silence for "nothing more is promised". Happy to write the two rows if that is quicker than you doing it.
|
Separately from the table itself: this branch is 85 commits behind main and has been open since 25 August. The suite reads Worth rebasing before spending effort on the rows, so the run you are debugging is the real one. |
2 participants
Summary
docs/security.mdmakes specific, testable promises, and nothing in CI provedany of them, so each held until a refactor quietly ended one. Several came
close recently. This ties every testable sentence on that page to the test that
defends it, and fails CI when a row names a test that no longer exists, so a
claim cannot be left silently undefended after a rename.
Related issues
Closes #22
Changes
docs/claims.md: fourteen rows, each the exact sentence, where it appears,and a typed token naming its defence:
go:TestName,smoke:<check text>,vm, orpending.script/check-claims.sh: resolves every token,go:againstgo test -list ./...andsmoke:against theoklines inscript/*.sh,and fails on any that resolves to nothing.
--self-testfirst runs it againsta row that names a missing test and confirms it fails, so the guard is proven
live before it is trusted.
claims table: self-test, then the check.make claimsruns thesame thing locally.
environment it receives: only the declared credential names reach the guest,
a denied billing key does not, a
scheme://value is refused, and nocredential is written into the workspace.
script/claims-vm.shandmake claims-vmfor the two rows only a real VM canprove (the guest sees one directory; the keychain, SSH agent and other host
directories are not reachable). It boots a sandbox where a runtime is present
and skips cleanly where none is.
pending, for the fail-open switch fix in fix(wrap): require an explicit value for security switches #63 and the digestpin in feat(verify): verify the digest that boots, not the tag #67; each flips to a real token when its PR merges.
docs/security.md; nothing else there.Checklist
make allpasses (vet, test, build)script/smoke.shpassesgo test ./... -race, if the change touches concurrency, subprocesses or the daemonbrig run <agent>), if it touches the run, exec or credential pathdocs/security.md,docs/profiles.md,docs/brigd.md)claims-vm.shwas seen to skip cleanly on a box with no runtime. Its boot half,the two VM rows, has not been run against a real sandbox yet, and it should be
once before this merges, since those two rows are the ones the page leads with.
Verified here beyond the gates: the checker refuses a row naming a missing Go
test and a row naming a missing smoke check, and its self-test passes.
Credentials and the sandbox boundary
No behaviour changes. This is the mechanism that keeps the claims about both
promises honest after the fact: every sentence the page makes is now either
defended by a named test, marked as needing a VM, or marked pending a specific
PR, and CI refuses a fourth state.