mirror of
https://github.com/PR0M3TH3AN/SeedPass.git
synced 2026-11-06 04:42:35 +00:00
fix(agent): an empty exec allowlist now fails closed; test the allowlist at all
Mutation slice 2 scored 0% before this, and the reason was blunt: **the exec allowlist had no test whatsoever.** The clipboard restriction could be inverted, weakened, or deleted outright and the suite stayed green. That control matters because the clipboard is readable by every process in the session. A token restricted to `cat` that may still reach the clipboard is not restricted to anything — the holder simply copies the secret out, and the allowlist looks enforced while containing nothing. Now tested: a command-restricted token is refused the clipboard, a token with no allowlist is not (so the rule is conditional rather than a blanket ban passing as one), an allowlisted command is permitted, and one outside the list is refused without the sink ever running. **A fail-open found while triaging one of the survivors.** An empty `exec_allowlist` was DROPPED at issue time, which made the token unrestricted. Automation that computes an allowlist and arrives at zero entries means "permit nothing" and was silently handed "permit everything" — a fail-open in the one field whose entire purpose is restriction. It is now refused at issue time with an error saying which of the two readings the caller probably wanted. Reachable only over the raw socket protocol, since commander requires at least one value. **An audit-accuracy gap.** A denial and a missing entry deliberately return the SAME message, so the response cannot be used to enumerate ids — the audit log is where the difference is supposed to be recorded, and nothing read it. The two reasons could be swapped with every test passing. Testing it needed two corrections of my own: the CLI resolves references locally so the request never reaches the daemon, and the shared token in that file is revoked by an earlier test, so both calls were refused at the token check instead. It also needed a stronger assertion than the one I first wrote — checking that both reasons merely APPEAR passes when they are swapped, which is exactly the mutation. It now asserts the pairing: which id got which reason. Three survivors are recorded as genuinely unkillable with reasons, rather than chased: the two `length > 0` comparisons cannot differ from `>= 0` now that an empty allowlist cannot be stored, and the capability early-return is redundant with the length check below it. agent slices 1-2 (mutants 1-16): 100% of viable mutants caught. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN
This commit is contained in:
1 parent
12b945ca7e
commit
17b94d3e26
3 files changed
+219
-2
No files matched your search
@@ -375,6 +375,10 @@ export class AgentDaemon {
|
||||
|
||||
if (sinkSpec) {
|
||||
const allowed = auth.token.exec_allowlist;
|
||||
// mutation-equivalent: `length > 0` cannot differ from `length >= 0`
|
||||
// here. An empty allowlist is refused at issue time, so a stored token
|
||||
// either has no allowlist (undefined, short-circuiting the `&&`) or a
|
||||
// non-empty one.
|
||||
if (allowed && allowed.length > 0 && sinkSpec.sink === "clipboard") {
|
||||
// The clipboard is readable by every process in the session, so it
|
||||
// defeats an allowlist just as thoroughly as an arbitrary command.
|
||||
@@ -387,6 +391,8 @@ export class AgentDaemon {
|
||||
error: { ok: false, error: "denied: clipboard not permitted by this token" },
|
||||
};
|
||||
}
|
||||
// mutation-equivalent: as above, an empty allowlist never reaches a
|
||||
// stored token, so `> 0` and `>= 0` decide identically here.
|
||||
if (sinkSpec.sink !== "clipboard" && allowed && allowed.length > 0) {
|
||||
// The allowlist constrains the command WORD only. The token holder
|
||||
// still chooses every argument, and the child receives the secret in
|
||||
@@ -591,6 +597,21 @@ export class AgentDaemon {
|
||||
const uses = positiveIntField(msg["uses"], 1);
|
||||
if (uses === null) return { ok: false, error: "uses must be a positive integer" };
|
||||
const secret = randomBytes(24).toString("base64url");
|
||||
// An empty exec_allowlist used to be DROPPED, which turned it into an
|
||||
// unrestricted token — a fail-open. Automation that computes an
|
||||
// allowlist and arrives at zero entries meant "permit nothing" and
|
||||
// silently received "permit everything". Refusing is the only reading
|
||||
// that cannot be a surprise: a token permitting no command is useless,
|
||||
// so asking for one is a mistake worth reporting rather than
|
||||
// reinterpreting.
|
||||
if (Array.isArray(msg["exec_allowlist"]) && (msg["exec_allowlist"] as string[]).length === 0) {
|
||||
return {
|
||||
ok: false,
|
||||
error:
|
||||
"exec_allowlist is empty, which would permit no command at all. " +
|
||||
"Omit it for an unrestricted token, or name the commands to allow.",
|
||||
};
|
||||
}
|
||||
const record: TokenRecord = {
|
||||
id: `tok-${randomBytes(6).toString("hex")}`,
|
||||
name: String(msg["name"] ?? "agent"),
|
||||
@@ -602,7 +623,7 @@ export class AgentDaemon {
|
||||
expires_at: Math.floor(this.now() + ttl),
|
||||
uses_remaining: uses,
|
||||
revoked: false,
|
||||
...(Array.isArray(msg["exec_allowlist"]) && (msg["exec_allowlist"] as string[]).length
|
||||
...(Array.isArray(msg["exec_allowlist"])
|
||||
? { exec_allowlist: msg["exec_allowlist"] as string[] }
|
||||
: {}),
|
||||
};
|
||||
|
||||
Reference in new issue
Block a user