mirror of
https://github.com/PR0M3TH3AN/SeedPass.git
synced 2026-10-27 07:52:03 +00:00
test(agent): pin token expiry at its exact boundary; give the daemon a clock
Mutation testing found `token.expires_at <= now` could become `<` with every test still passing. The suite checked well before and well after expiry, which cannot tell the two apart — so a token was free to be usable for one more request than it claimed, which is the wrong direction for a credential. The daemon now takes an injectable clock, so the boundary can be asserted exactly rather than raced against wall time: at T-1 the token works, at exactly T it is refused. Every expiry decision AND every expiry assignment goes through that clock — a token issued against real time while checked against test time would never expire, which would have made the test meaningless. The TOTP timestamp-skew window uses it too, since a clock that moved one but not the other is a difference no production path should have either. Also pins the owner capability: an empty or absent capability is refused, and so is one of the right length but wrong content — the latter proving the constant-time comparison is what rejects it rather than the length check. One survivor is now recorded as genuinely unkillable rather than chased. `!presented || !this.capability` cannot change behaviour when flipped to `&&`, because the length comparison below rejects an empty string either way. The harness learned a `mutation-equivalent:` marker that must carry a REASON, so future survivor lists mean "look at this" instead of "this again" — and the judgement stays re-checkable rather than merely trusted. The harness also gained `--offset`, so a long target is measured in foreground-sized slices. That is a safety property, not a convenience: this script writes broken security code into the working tree, and it must never run unattended alongside commits again. agent slice 1 (mutants 1-6): 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
c2a6a11472
commit
12b945ca7e
3 files changed
+167
-16
No files matched your search
@@ -173,6 +173,13 @@ export class AgentDaemon {
|
||||
private readonly defaultTtl = DEFAULT_TTL_SECONDS,
|
||||
/** Profile root; required for token-mode vault/secret serving. */
|
||||
private readonly appDir?: string,
|
||||
/**
|
||||
* Seconds since the epoch. Injectable so expiry can be tested at its
|
||||
* exact boundary rather than approximately — "expires at T" has to mean
|
||||
* denied AT T, and a test that only checks T-1 and T+1 cannot tell
|
||||
* `<=` from `<`.
|
||||
*/
|
||||
private readonly now: () => number = () => Date.now() / 1000,
|
||||
) {}
|
||||
|
||||
/**
|
||||
@@ -203,6 +210,10 @@ export class AgentDaemon {
|
||||
*/
|
||||
private isOwner(msg: Record<string, unknown>): boolean {
|
||||
const presented = String(msg["cap"] ?? "");
|
||||
// mutation-equivalent: turning this `||` into `&&` cannot change the
|
||||
// outcome — an empty string on either side fails the length comparison
|
||||
// below, so the early return is a statement of intent rather than the
|
||||
// thing doing the rejecting. Kept because the rule is worth stating.
|
||||
if (!presented || !this.capability) return false;
|
||||
const a = Buffer.from(presented);
|
||||
const b = Buffer.from(this.capability);
|
||||
@@ -261,7 +272,7 @@ export class AgentDaemon {
|
||||
);
|
||||
if (!token) return { deny: "unknown token" };
|
||||
if (token.revoked) return { deny: "token revoked" };
|
||||
if (token.expires_at <= Date.now() / 1000) return { deny: "token expired" };
|
||||
if (token.expires_at <= this.now()) return { deny: "token expired" };
|
||||
if (token.uses_remaining <= 0) return { deny: "token exhausted" };
|
||||
if (!token.scopes.includes(action)) return { deny: `scope '${action}' not granted` };
|
||||
// "read" is not consumption; secret-bearing actions decrement uses
|
||||
@@ -408,7 +419,10 @@ export class AgentDaemon {
|
||||
let ts: number | undefined;
|
||||
if (msg["timestamp"] !== undefined) {
|
||||
const requested = Number(msg["timestamp"]);
|
||||
const now = Math.floor(Date.now() / 1000);
|
||||
// Same clock as expiry: a test clock that moved one but not the other
|
||||
// would let a token expire while the skew window still measured real
|
||||
// time, which is a difference no production path should have either.
|
||||
const now = Math.floor(this.now());
|
||||
if (!Number.isFinite(requested) || Math.abs(requested - now) > TIMESTAMP_SKEW_SECONDS) {
|
||||
return {
|
||||
error: {
|
||||
@@ -502,7 +516,7 @@ export class AgentDaemon {
|
||||
}
|
||||
|
||||
private expire(): void {
|
||||
const now = Date.now() / 1000;
|
||||
const now = this.now();
|
||||
for (const [fp, held] of this.held) {
|
||||
if (held.expiresAt <= now) this.forget(fp);
|
||||
}
|
||||
@@ -523,7 +537,7 @@ export class AgentDaemon {
|
||||
private highRiskTag(fingerprint: string): string {
|
||||
const session = this.highRisk.get(fingerprint);
|
||||
if (!session) return "";
|
||||
if (session.expiresAt <= Date.now() / 1000) {
|
||||
if (session.expiresAt <= this.now()) {
|
||||
this.highRisk.delete(fingerprint);
|
||||
return "";
|
||||
}
|
||||
@@ -585,7 +599,7 @@ export class AgentDaemon {
|
||||
scopes: scopes as TokenScope[],
|
||||
kinds: (kinds as string[] | undefined) ?? null,
|
||||
label_regex: String(msg["label_regex"] ?? ".*"),
|
||||
expires_at: Math.floor(Date.now() / 1000 + ttl),
|
||||
expires_at: Math.floor(this.now() + ttl),
|
||||
uses_remaining: uses,
|
||||
revoked: false,
|
||||
...(Array.isArray(msg["exec_allowlist"]) && (msg["exec_allowlist"] as string[]).length
|
||||
@@ -738,7 +752,7 @@ export class AgentDaemon {
|
||||
if (generateFingerprint(mnemonic) !== fingerprint) {
|
||||
return { ok: false, error: "fingerprint does not match the supplied seed" };
|
||||
}
|
||||
const expiresAt = Math.floor(Date.now() / 1000 + ttl);
|
||||
const expiresAt = Math.floor(this.now() + ttl);
|
||||
// The audit key is derived from the held seed, so the seed must go in
|
||||
// before the record is written — but an audit failure must then undo
|
||||
// it. Otherwise a `put` that reports ok:false can leave the seed
|
||||
@@ -774,7 +788,7 @@ export class AgentDaemon {
|
||||
}
|
||||
const ttl = positiveIntField(msg["ttl"], 300);
|
||||
if (ttl === null) return { ok: false, error: "ttl must be a positive integer" };
|
||||
const expiresAt = Math.floor(Date.now() / 1000 + ttl);
|
||||
const expiresAt = Math.floor(this.now() + ttl);
|
||||
this.highRisk.set(fingerprint, { tag, expiresAt });
|
||||
try {
|
||||
// Records THAT it was unlocked and for how long — never the tag.
|
||||
|
||||
@@ -8,14 +8,22 @@
|
||||
|
||||
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
import { createConnection } from "node:net";
|
||||
import { mkdtemp, readFile, writeFile, stat } from "node:fs/promises";
|
||||
import { mkdtemp, readFile, writeFile, stat, mkdir } from "node:fs/promises";
|
||||
import { join as joinPath } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import process from "node:process";
|
||||
import { mnemonics } from "@seedpass/test-vectors";
|
||||
import { encryptV3, deriveIndexKeyBytes, utf8 } from "@seedpass/core";
|
||||
import { generateFingerprint } from "@seedpass/core";
|
||||
import { buildProgram, AgentDaemon, agentSocketPath, type ProgramIo } from "../src/index.js";
|
||||
import {
|
||||
buildProgram,
|
||||
AgentDaemon,
|
||||
AgentClient,
|
||||
agentSocketPath,
|
||||
INDEX_FILENAME,
|
||||
type ProgramIo,
|
||||
} from "../src/index.js";
|
||||
|
||||
const MNEMONIC = mnemonics["abandon12"]!;
|
||||
const FINGERPRINT = generateFingerprint(MNEMONIC);
|
||||
@@ -496,3 +504,93 @@ describe("a use-scoped token cannot crash the agent", () => {
|
||||
expect(status.map((p: { fingerprint: string }) => p.fingerprint)).toContain(FINGERPRINT);
|
||||
});
|
||||
});
|
||||
|
||||
describe("the owner capability", () => {
|
||||
it("refuses an empty or absent capability", async () => {
|
||||
// Mutation testing showed `!presented || !this.capability` could become
|
||||
// `&&` without any test noticing: the length comparison below it happens
|
||||
// to reject an empty string too, so the early return is redundant. It is
|
||||
// still the line that STATES the rule, and an empty credential being
|
||||
// refused is worth asserting outright rather than relying on a length
|
||||
// check further down to imply it.
|
||||
for (const cap of ["", undefined]) {
|
||||
const r = await rawRequest({
|
||||
op: "status",
|
||||
...(cap !== undefined && { cap }),
|
||||
});
|
||||
expect(r["ok"]).toBe(false);
|
||||
expect(String(r["error"])).toMatch(/owner/i);
|
||||
}
|
||||
});
|
||||
|
||||
it("refuses a capability of the right length but wrong content", async () => {
|
||||
const real = (await readFile(daemon.capabilityPath, "utf8")).trim();
|
||||
// Same length, so the length check cannot be what rejects it — this is
|
||||
// the constant-time comparison doing its job.
|
||||
const forged = "x".repeat(real.length);
|
||||
expect(forged).toHaveLength(real.length);
|
||||
const r = await rawRequest({ op: "status", cap: forged });
|
||||
expect(r["ok"]).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("expiry is denied AT the boundary, not after it", () => {
|
||||
/**
|
||||
* Mutation testing found `token.expires_at <= now` could become `<` with
|
||||
* every test still passing: the suite checked well before and well after
|
||||
* expiry, which cannot tell the two apart. "Expires at T" has to mean
|
||||
* denied AT T — otherwise a token is usable for one more request than it
|
||||
* claims, which is the wrong direction for a credential.
|
||||
*
|
||||
* The daemon takes an injectable clock so this can be pinned exactly
|
||||
* rather than raced against wall time.
|
||||
*/
|
||||
const FROZEN = 1_800_000_000;
|
||||
|
||||
async function daemonAt(now: () => number): Promise<{ d: AgentDaemon; sock: string }> {
|
||||
const dir = await mkdtemp(join(tmpdir(), "seedpass-expiry-"));
|
||||
// `put` writes an audit record, which needs the profile directory and an
|
||||
// index to serve from.
|
||||
const profile = join(dir, FINGERPRINT);
|
||||
await mkdir(profile, { recursive: true });
|
||||
await writeFile(
|
||||
join(profile, INDEX_FILENAME),
|
||||
await encryptV3(
|
||||
deriveIndexKeyBytes(MNEMONIC),
|
||||
utf8(JSON.stringify({ schema_version: 4, entries: {} })),
|
||||
),
|
||||
);
|
||||
const sock = join(dir, "agent.sock");
|
||||
const d = new AgentDaemon(sock, 900, dir, now);
|
||||
await d.start();
|
||||
return { d, sock };
|
||||
}
|
||||
|
||||
it("denies a token at the exact instant it expires", async () => {
|
||||
let clock = FROZEN;
|
||||
const { d, sock } = await daemonAt(() => clock);
|
||||
try {
|
||||
const client = new AgentClient(sock);
|
||||
await client.put(FINGERPRINT, MNEMONIC, 900);
|
||||
const issued = await client.tokenIssue({
|
||||
fingerprint: FINGERPRINT,
|
||||
name: "boundary",
|
||||
scopes: ["read"],
|
||||
ttl: 60,
|
||||
uses: 5,
|
||||
});
|
||||
|
||||
// One second before expiry: usable.
|
||||
clock = FROZEN + 59;
|
||||
await expect(client.vaultEntries(FINGERPRINT, issued.token)).resolves.toBeDefined();
|
||||
|
||||
// EXACTLY at expiry: denied. This is the assertion `<` would fail.
|
||||
clock = FROZEN + 60;
|
||||
await expect(client.vaultEntries(FINGERPRINT, issued.token)).rejects.toThrow(
|
||||
/expired/,
|
||||
);
|
||||
} finally {
|
||||
await d.stop();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -121,6 +121,14 @@ SECURITY_HINTS = re.compile(
|
||||
|
||||
SKIP_LINE = re.compile(r"^\s*(//|\*|/\*)")
|
||||
|
||||
# A line preceded by this marker has been triaged as an EQUIVALENT mutant:
|
||||
# the mutation provably cannot change behaviour, so no test can kill it and
|
||||
# chasing it is wasted effort. Marking it keeps future survivor lists
|
||||
# meaningful -- a survivor should mean "look at this", not "this again".
|
||||
# The marker must say WHY, so the judgement can be re-checked rather than
|
||||
# trusted.
|
||||
EQUIVALENT_MARKER = "mutation-equivalent:"
|
||||
|
||||
|
||||
@dataclass
|
||||
class Mutant:
|
||||
@@ -130,11 +138,29 @@ class Mutant:
|
||||
description: str
|
||||
|
||||
|
||||
def _marked_equivalent(lines: list[str], line_no: int) -> bool:
|
||||
"""Is the code at `line_no` (1-based) preceded by an equivalence marker?"""
|
||||
idx = line_no - 2 # the line directly above, 0-based
|
||||
while idx >= 0 and SKIP_LINE.match(lines[idx]):
|
||||
if EQUIVALENT_MARKER in lines[idx]:
|
||||
return True
|
||||
idx -= 1
|
||||
return False
|
||||
|
||||
|
||||
def generate(source: str) -> list[Mutant]:
|
||||
mutants: list[Mutant] = []
|
||||
for i, line in enumerate(source.splitlines(), start=1):
|
||||
lines = source.splitlines()
|
||||
for i, line in enumerate(lines, start=1):
|
||||
if SKIP_LINE.match(line) or not SECURITY_HINTS.search(line):
|
||||
continue
|
||||
# Skip lines a human has already judged unkillable, with a reason.
|
||||
# Scans the whole contiguous comment block above, not just the line
|
||||
# immediately before: the marker belongs at the START of an
|
||||
# explanation, and requiring it on the last line would mean writing
|
||||
# the reason upside down.
|
||||
if _marked_equivalent(lines, i):
|
||||
continue
|
||||
for old, new in OPERATORS:
|
||||
if old not in line:
|
||||
continue
|
||||
@@ -179,6 +205,17 @@ def main() -> int:
|
||||
parser.add_argument(
|
||||
"--limit", type=int, default=0, help="cap mutants per target (0 = all)"
|
||||
)
|
||||
parser.add_argument(
|
||||
"--offset",
|
||||
type=int,
|
||||
default=0,
|
||||
help=(
|
||||
"skip the first N mutants. With --limit this runs a slice, so a "
|
||||
"long target can be measured in foreground-sized pieces rather "
|
||||
"than backgrounded -- this script writes broken security code "
|
||||
"into the tree, and must not run unattended alongside commits."
|
||||
),
|
||||
)
|
||||
parser.add_argument("--timeout", type=int, default=420)
|
||||
args = parser.parse_args()
|
||||
|
||||
@@ -196,10 +233,12 @@ def main() -> int:
|
||||
rel, tests = TARGETS[name]
|
||||
path = CLI / rel
|
||||
original = path.read_text()
|
||||
mutants = generate(original)
|
||||
all_mutants = generate(original)
|
||||
mutants = all_mutants[args.offset :]
|
||||
if args.limit:
|
||||
mutants = mutants[: args.limit]
|
||||
print(f"\n=== {name} ({rel}): {len(mutants)} mutants ===")
|
||||
span = f"{args.offset + 1}-{args.offset + len(mutants)} of {len(all_mutants)}"
|
||||
print(f"\n=== {name} ({rel}): mutants {span} ===")
|
||||
|
||||
# Restore the file whatever happens, including Ctrl-C. Leaving a
|
||||
# deliberate security hole in the tree would be a spectacular own goal.
|
||||
@@ -211,7 +250,7 @@ def main() -> int:
|
||||
)
|
||||
|
||||
try:
|
||||
for idx, mutant in enumerate(mutants, start=1):
|
||||
for idx, mutant in enumerate(mutants, start=args.offset + 1):
|
||||
lines = original.splitlines(keepends=True)
|
||||
ending = "\n" if lines[mutant.line_no - 1].endswith("\n") else ""
|
||||
lines[mutant.line_no - 1] = mutant.mutated + ending
|
||||
@@ -222,7 +261,7 @@ def main() -> int:
|
||||
)
|
||||
if code != 0:
|
||||
overall_counts["invalid"] += 1
|
||||
print(f" [{idx}/{len(mutants)}] line {mutant.line_no}: invalid (does not typecheck)")
|
||||
print(f" [{idx}/{len(all_mutants)}] line {mutant.line_no}: invalid (does not typecheck)")
|
||||
continue
|
||||
|
||||
code, _ = run(
|
||||
@@ -230,11 +269,11 @@ def main() -> int:
|
||||
)
|
||||
if code != 0:
|
||||
overall_counts["killed"] += 1
|
||||
print(f" [{idx}/{len(mutants)}] line {mutant.line_no}: killed ({mutant.description})")
|
||||
print(f" [{idx}/{len(all_mutants)}] line {mutant.line_no}: killed ({mutant.description})")
|
||||
else:
|
||||
overall_counts["survived"] += 1
|
||||
overall_survivors.append((f"{rel}:{mutant.line_no}", mutant))
|
||||
print(f" [{idx}/{len(mutants)}] line {mutant.line_no}: SURVIVED ({mutant.description})")
|
||||
print(f" [{idx}/{len(all_mutants)}] line {mutant.line_no}: SURVIVED ({mutant.description})")
|
||||
finally:
|
||||
restore()
|
||||
# Prove the restore worked rather than assuming it: a mutant left
|
||||
|
||||
Reference in new issue
Block a user