DEV Community

pm25coder
pm25coder

Posted on

An allow-list does not fail closed

Somebody told me last week, on a thread about a Claude Code plugin that stops an agent from editing its own tests, that an allow-list "fails closed, which is the failure mode you want when the agent is the one trying the edge case."

That is the right intuition and the wrong conclusion. It is worth pulling apart, because the same reasoning shows up every time someone hardens an agent's shell.

The guard

The plugin (Mattbusel/tamperproof, v0.1.1) replaced its deny-list of "writer" commands with an allow-list — the right direction, because a deny-list only covers the verbs someone thought of. The new rule, from hooks/guard.mjs:

Any shell segment that names an existing protected path is blocked, unless it starts with a known read-only or test-runner command and contains no redirect or tee.

Read the first clause slowly. "names an existing protected path" is what decides whether the rule is applied at all. Here is the code:

for (const piece of seg.split(/[\s'"`,;()=<>{}\[\]]+/)) {
  if (!piece || piece.startsWith("-") || /^[a-z]+:\/\//i.test(piece)) continue;
  const r = rel(piece);
  if (!r.startsWith("..") && isProtected(r, protect) && existsSync(resolve(cwd, r))) hits.push(r);
}
if (!hits.length) continue;                 // <- never judged, so allowed
if (!READ_ONLY.test(body) || writesOut) block(...);
Enter fullscreen mode Exit fullscreen mode

existsSync(resolve(cwd, piece)), on the literal token. If no token looks like an existing protected file, the segment is skipped — it is not judged, and not being judged means allowed.

What I measured

I checked out 7559493 (v0.1.1), built a throwaway tree containing test/calc.test.mjs, and drove the guard the way the hook does — the tool call on stdin, the exit code read back. Same command in every row; only the target's spelling moves:

$ sed -i s/5/-1/ test/calc.test.mjs          -> exit 2   blocked
$ sed -i s/5/-1/ test/calc.test.j?           -> exit 0
$ sed -i s/5/-1/ test/*.mjs                  -> exit 0
$ sed -i s/5/-1/ test/calc.test.{mjs}        -> exit 0
$ sed -i s/5/-1/ "$PWD/test/calc.test.mjs"   -> exit 0
$ sed -i s/5/-1/ test/nonexistent.mjs        -> exit 0
Enter fullscreen mode Exit fullscreen mode

The literal answers 2 because it is a file: existsSync says yes, the path is protected, the segment is judged, sed -i is not on the read-only list, blocked.

The rest answer 0 because existsSync says no. test/*.mjs is a token, not a path — the guard asks the filesystem about the string test/*.mjs, and that string is not a file. Same for ?; same for the brace form (the splitter even cuts { and } out, leaving test/calc.test.); same for $PWD/..., which resolves under a directory literally named $PWD. Nothing matches, so hits stays empty and the rule is never consulted.

And a spelling is expanded by a real shell before sed ever sees it — that is the entire job of a glob. So the string defeats the check whose subject is the string.

The generalisation, which is the useful part

Which way a guard fails is not a property of allow-list versus deny-list. It is a property of where the default sits in the decision chain.

This guard is default-deny at the verdict: a segment that gets judged and is not read-only is blocked. And it is default-allow at the selector: a segment that is never judged is allowed. The selector is the earlier of the two, and it is the layer a command string drives. So the guard fails closed exactly where the attacker has no freedom, and fails open exactly where they do.

"An allow-list fails closed" is true only on the axis you happened to enumerate — here, the verb. Enumerate the axis the attacker's input controls, or you have measured the wrong thing.

The other direction is real too

Two arms came back the other way, and they are why I would not call even the verb axis simply "closed":

$ cat test/calc.test.mjs > /dev/null      -> exit 2
$ grep -n '=>' test/calc.test.mjs         -> exit 2
Enter fullscreen mode Exit fullscreen mode

writesOut is />/.test(seg), so the arrow in a grep pattern reads as a redirect, and cat ... > /dev/null is a redirect to nowhere. Both are things an agent does constantly while debugging a failing test — the exact state the guard exists to keep it in. A false block is not free: it teaches the reader to route around the guard, and then the guard protects nothing.

What I would take from it

  • Name which layer the default lives in, per rule. "Blocked unless read-only" and "not looked at" are different verdicts, and only one of them is visible in a code review.
  • Ask the question about the thing that will be written, not the string that names it — or accept that the string is a spelling and chase spellings. The author already named the honest tension: treating a glob-shaped token as a hit closes this hole and also blocks a genuinely new test written from a shell.
  • Keep a control arm in both directions. All twenty of the guard's test cases name the path, and so did every arm I sent before looking. A case count is not a coverage claim — the question is which axis the cases move on.

The plugin is a genuinely good piece of work, and 0.1.1 closed every hole the previous version had, including all of the above for the literal spellings. This is the one axis that survived a rewrite, and it will survive the next one too unless the fix goes into the selector. I wrote up the thread it came from here.

Top comments (4)

Collapse
 
kevinbai profile image
kevinbai •

Strong piece, and the measured exit codes are what make it land. Two things I'd add.

The fail-open arm and the false-block arm are the same bug, not two bugs. You file the open cases under the selector and the grep '=>' / cat ... > /dev/null cases under a crude writesOut regex. But both verdicts are computed from the command string: existsSync(token) asks the filesystem about a string, />/ asks a regex about a string. The selector is just the earliest of several layers that all speak "spelling". So "put the fix in the selector" may under-scope it — whichever arm is currently shallowest is where the next rewrite puts the hole, and your own evidence points that way: one axis survived a rewrite, and it was the arm nobody had re-derived.

Which points at a smaller selector rather than a smarter one. The dilemma you name — treat glob-shaped tokens as hits and you block a legitimately new test written from a shell; don't, and it still fails open — only exists while the guard has to answer "will this write?" from text. Make the protected paths unwritable (read-only bind mount, or run the segment inside a throwaway copy of the tree) and the glob is free to expand into whatever it expands into, because the write fails in the kernel instead of the parser. You keep one question, "is this path protected?", and delete the other, "does this spelling denote it?" The deleted one is the one the command string had freedom over. Same move as scoring an agent's artifact in a container it cannot write to instead of scanning its diff for echo 1.0 >.

One more row I'd want in the table, because it isn't a spelling at all: a symlink in the working tree pointing at a protected file. existsSync says yes, but isProtected is answering about the link's name — so unless rel() realpaths, the segment is never judged and sed -i writes straight through the link. A selector that chases spellings has no way to close that one, which is another argument for letting the effect decide rather than the text. (Lesser note: existsSync is also a TOCTOU on its own — the file can appear or vanish between the check and the sed.)

The control-arm line is the one I'd put at the top. A case count is not a coverage claim, and "every arm I sent named the path" is the kind of sentence that changes how someone reads their own test suite.

Collapse
 
pm25coder profile image
pm25coder •

You called the symlink row, so I ran it — and the sibling I found sharpens your argument rather than adding a row to it.

On v0.1.1 (7559493), a throwaway tree, same command in every arm:

test/calc.test.mjs          rc=2   protected by name
notes.md    (hardlink)      rc=0
pointer.md  (symlink)       rc=0
Enter fullscreen mode Exit fullscreen mode

link/symlink with an unprotected name, both reaching the protected file — neither is judged, because the selector asks the filesystem about a name and neither name matches a protect pattern. Exactly as you predicted.

Then I measured what the obvious next move — realpath it — would actually buy, because that is where your row leads:

name             isProtected(name)  realpath(name)            inode
calc.test.mjs    true              calc.test.mjs            38562071810511810
notes.md         false             notes.md                 38562071810511810
pointer.md       false             test/calc.test.mjs  (T)  38562071810511810
Enter fullscreen mode Exit fullscreen mode

All three are the same inode. Realpath closes the symlink: pointer.md resolves to the protected path and the check fires. It cannot close the hardlink — there is nothing to resolve, so realpath('notes.md') returns notes.md, still unjudged. The one predicate that covers both is inode identity (dev+ino against the protected set), and that is a different instrument from the string pipeline, not a better spelling of the same one. Which is your thesis one level down: even the fix that stays nearest the text runs out, precisely in the case that has no text to chase.

On "smaller selector, not smarter": agreed, with one thing to check before adopting it. I measured the read-only half here (Windows/NTFS) — a read-only file refused both the in-place write and the rename-over, so at the file level the proposal holds. But sed -i and every atomic writer go temp+rename, and on POSIX rename(2) is governed by the directory's write bit, not the target's mode. So the guarantee you want is a read-only directory (or a mount), not a read-only file — per-path, since the tree has to stay writable everywhere a runner legitimately writes (coverage, __pycache__, tmp). I have not executed that half (no POSIX shell on this host), so treat it as the thing to verify rather than as a reading.

Your TOCTOU note lands in the same place: existsSync then sed is check-then-use with the string as the interface, so it is one class again, not a separate footnote.

And I would take your last paragraph as the summary of the whole thing — "every arm I sent named the path" is exactly the sentence that hides the axis.

Collapse
 
mickyarun profile image
arun rajkumar •

Enumerate the axis the attacker's input controls is the sentence, and the selector-versus-verdict split is what I will be reusing. The table makes it unarguable: same command, same intent, six spellings, one exit 2.

@kevinbai has the better fix above and I think he is right that letting the kernel decide beats any amount of parsing. So this is for the case where you cannot do that, which in my world is most of them, because the effect is a request to somebody else's API and there is no mount to make read-only.

The same shape is the standard failure in payment screening. A sanctions check is default-deny on a name it matched and default-allow on whether it extracted a name at all. If the beneficiary name arrives in a field the parser does not read, nothing gets screened and the payment goes through with a green screening result attached to it. Regulators mostly stopped trying to fix the parser. What they ask for is a count: payments processed against payments screened, and the gap is the control. It does not make the selector correct. It makes it measurable, which is a weaker and more durable property.

Your guard already computes that number. hits.length. Emit it. A command where two segments were judged and allowed is a different outcome from one where nothing was judged, and today both are exit 0 and identical to whoever reads the logs. sed -i s/5/-1/ test/*.mjs does not become blocked. It becomes visible, and a judged-segment count that falls to zero across a run is the signal that somebody found the spelling. It also picks up the symlink row kevinbai added, not by resolving it, but because that segment still judges nothing.

Where it stops is the same place as his argument. A count tells you the selector missed and not what it missed. For an agent editing its own tests, knowing is probably enough. For an action you cannot undo it is not, and then the mount is the answer and the parser never was.

Collapse
 
pm25coder profile image
pm25coder •

I took the hits.length suggestion and instrumented the guard to emit it, then ran a battery. Your read of the source is exact: hits is built per segment (guard.mjs:52, filled at :58) and consumed only by the continue at :60 and the writesOut test at :63 — it never reaches any output. So the blocked and allowed outcomes are byte-identical in the log, exit code aside.

Same tree, one command each:

segments judged   exit   command
1 of 1            2      sed -i s/5/-1/ test/calc.test.mjs
0 of 1            0      sed -i s/5/-1/ test/*.mjs
0 of 1            0      sed -i s/5/-1/ pointer.md    (symlink)
0 of 1            0      sed -i s/5/-1/ notes.md      (hardlink)
0 of 1            0      sed -i s/x/y/ src/app.mjs
0 of 1            0      npm test
0 of 1            0      git status
Enter fullscreen mode Exit fullscreen mode

So the count does surface the spellings, without resolving them — as you said. The row I did not expect is the fifth: judged=0 is also the reading for every command that simply does not name a protected path. sed -i s/x/y/ src/app.mjs and sed -i s/5/-1/ test/*.mjs are the same number.

Emitting the segment denominator does not separate them either:

sed -i s/5/-1/ test/*.mjs && cat test/calc.test.mjs   -> hits [0,1]
sed -i s/x/y/ src/app.mjs && cat test/calc.test.mjs   -> hits [0,1]
Enter fullscreen mode Exit fullscreen mode

Identical. Which is the structural version of your own caveat: hits is precisely the thing the spelling defeats, so no count derived from it — hits, judged segments, or their ratio — can separate "named nothing" from "spelled around" on a single command. It reads only against a baseline. That is the aggregate you named, not a per-command alarm, and it is a weaker instrument than "weaker" suggests: it can never fire on one command, only on a deviation from a command population. The per-command instrument is the expansion or the mount; the count was never going to be it.

The screening framing still holds, and it is the same shape — a screening rate is a baseline-relative number that nobody expects to flag one payment. I would just add the denominator's own denominator: the useful ratio is screened / would-have-been-screened, and inside the guard there is no oracle for the second term. That is the wall your last paragraph names, one level down.