CLAUDE.md: two rulings, and two claims that overstated their enforcement - #282
Merged
Conversation
Both found reviewing the change that exists to correct overstatements. .credentials.json was listed as an instance of the command-table rule. It is not: it is a projection of a SECRET. Same mechanism, different rule, and listing it makes the generalisation read as 'everything snug generates is a command table'. CLAUDE.md already describes it correctly under Identity and credentials, forty lines below. The #73 sentence claimed the projection 'closes both directions at once' as a general property. #269 shipped EXISTS-ONLY: --ro-bind-data over an absent path makes bwrap CREATE the mountpoint on the host (measured, a 0-byte -r--r--r-- file in the repo), which rejectGeneratedOntoHost refuses. So on a clean repo where the payload CREATES the file, the outward half stays open, and --dry-run distinguishes the two states rather than implying coverage. The inbound half is closed either way, and it is the sharper threat: a hostile repo shipping a command table is the exists case by construction.
vyskocilm
changed the title
Independent review of this PR's four passages against one test — would an agent about to write code behave differently because of this sentence. Cut, all narration rather than instruction: - the #271 archaeology (what the test does, the #270 Authored comparison, the roll-call of which other rules are review-enforced). Kept the two clauses that change behaviour: no check catches a demote, and a list of spellings is not the fix. Folded back into the paragraph it interrupted. - #73's reproduction — claude -p, no trust dialog, the 0-byte file. It is already in types.go's HostDestExists comment, validate.go, and CLAUDE-SETTINGS.md 4.5. Kept the one clause an implementer hits: a projection mounts only where the host file already exists. - the .credentials.json taxonomy parenthesis, which corrected a classification with no behavioural consequence mid-definition. - snug setup's sequencing and its trusted-for-where-it-sits line; issue #30 carries both. Net added lines 56 -> 49. Noted for later, not acted on: the file now says 'review-enforced, nothing checks this' in three places (#207, #219, #271), each cross-referencing the others. A fourth should collapse them into one named list rather than a fourth cross-reference.
vyskocilm
changed the title
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.
Two rulings, and two claims in
mainthat said a rule was enforced when it was not.The corrections
Invariant 1 claimed a grep that does not exist (#271). The text said the no-demote half is "checked by grepping for a demote and finding none (
TestPolicyHasNoRestrictionOperation)". That test only assertsAccess.Jointakes the max — aDerive()rewriting everyAccessRWtoAccessROships green. Now says no check catches a demote, that this half is review-enforced, and that a list of demote spellings is not the fix."Closed twice over" was wrong (shipped in #268). The measured route is a bind covering
$HOME, whose source is a directory, so #219'sS_IFSOCKcheck never fires on it — #220 closes that one, #219 closes the direct spelling. Two routes, one check each, neither redundant. An agent reading the old text could have deleted #220's guard believing #219 covered it.The rulings
Reading and building a profile never needs network access. Parsing, resolving, validating and
--dry-runare offline and stay that way. Ergonomics that need the network go tosnug setup(#30), a separate binary with its owngo.mod.A command table snug exposes is REINTERPRETED, not bound. Generalises
~/.gitconfig,~/.claude.jsonandinstalled_plugins.json. Never a warning, which reports a breach instead of preventing one; never a bare read-only bind, which stops the editing and supplies every command in it. Its limit is stated: a projection mounts only where the host file already exists.Three commits, deliberately not squashed into one
The sequence is the useful part.
28af49dadded the four passages;58f6f44fixed two overstatements found reviewing them (.credentials.jsonis a secret projection, not a command table; the #73 claim asserted a general property that #269 shipped exists-only);31e2fadcut them back after an independent review, on one test — would an agent about to write code behave differently because of this sentence.That review removed #73's reproduction and #271's archaeology as narration. Both are already carried by the issues,
types.go'sHostDestExistscomment,validate.go, andCLAUDE-SETTINGS.md§4.5, which is where this file's own rules say they belong. Net added lines 56 → 49.Noted, not acted on: the file now says "review-enforced, nothing checks this" in three places (#207, #219, #271), each cross-referencing the others. A fourth should collapse them into one named list rather than add a fourth cross-reference.
Docs only, no code.