diff --git a/js/packages/cli/test/api.test.ts b/js/packages/cli/test/api.test.ts index eea9e1c2..f7bb5ed7 100644 --- a/js/packages/cli/test/api.test.ts +++ b/js/packages/cli/test/api.test.ts @@ -252,11 +252,40 @@ describe("what the API refuses", () => { ["relays", "wss://one.example"], ["relays", []], ["min_uppercase", "lots"], + // Values that LOOK numeric to Number() and are not numbers. Number(true) + // is 1, Number(null) and Number("") are both 0, so without an explicit + // check `inactivity_timeout: true` becomes a one-second timeout and + // `clipboard_clear_delay: null` becomes zero — a setting silently + // changed to something the caller never asked for. + ["inactivity_timeout", true], + ["clipboard_clear_delay", null], + ["min_uppercase", ""], + ["nostr_retry_delay", false], + // Zero KDF iterations is not a slow KDF, it is no KDF — and it would be + // written into the config of a vault whose master password protects + // everything else. + ["kdf_iterations", 0], + ["kdf_iterations", -1], ] as const) { const res = await call("PUT", `/api/v1/config/${key}`, { body: { value } }); expect(res.status).toBe(400); } + // Zero is not negative, and the two must not be conflated: a + // clipboard delay of 0 is a real choice (clear on the next tick), while + // a negative one is nonsense. + expect( + (await call("PUT", "/api/v1/config/clipboard_clear_delay", { body: { value: 0 } })) + .status, + ).toBe(200); + expect((await call("GET", "/api/v1/config/clipboard_clear_delay")).json.value).toBe(0); + // One iteration is pointless but it is a number the caller may legitimately + // choose; the floor is where nonsense starts, not where good advice does. + expect( + (await call("PUT", "/api/v1/config/kdf_iterations", { body: { value: 1 } })).status, + ).toBe(200); + await call("PUT", "/api/v1/config/kdf_iterations", { body: { value: 200000 } }); + // And the well-typed versions still go through, so the check is // rejecting the value rather than the key. expect( diff --git a/scripts/mutation_test.py b/scripts/mutation_test.py index 0908f8eb..d340f556 100644 --- a/scripts/mutation_test.py +++ b/scripts/mutation_test.py @@ -94,6 +94,10 @@ TARGETS: dict[str, tuple[str, list[str]]] = { "highrisk": ("src/highRisk.ts", ["test/highRisk.test.ts"]), "sinks": ("src/sinks.ts", ["test/sinks.test.ts", "test/tokens.test.ts"]), "approvals": ("src/approvals.ts", ["test/highRisk.test.ts"]), + # The config normalizers are the API's only defence against a well-typed + # nonsense value being stored permanently in an encrypted file nobody + # reads back by hand. + "config": ("src/configFile.ts", ["test/api.test.ts"]), } # Operator swaps that change a decision rather than a value's shape. @@ -124,6 +128,13 @@ SECURITY_HINTS = re.compile( re.IGNORECASE, ) +# A guard whose body REFUSES is security-relevant whether or not it happens to +# use the vocabulary above. src/configFile.ts generated zero mutants because +# its validators say `if (n <= 0) throw new ConfigValueError(...)` -- refusals +# in every meaningful sense, containing not one word the hint list knows. This +# catches them by what the line DOES rather than what it is about. +REFUSAL = re.compile(r"\bthrow new \w*(Error|Exception)\b|\breturn \{ ?ok: false") + SKIP_LINE = re.compile(r"^\s*(//|\*|/\*)") # A line preceded by this marker has been triaged as an EQUIVALENT mutant: @@ -157,7 +168,16 @@ def generate(source: str) -> list[Mutant]: mutants: list[Mutant] = [] lines = source.splitlines() for i, line in enumerate(lines, start=1): - if SKIP_LINE.match(line) or not SECURITY_HINTS.search(line): + if SKIP_LINE.match(line): + continue + # A block-form guard puts its refusal on a following line, so look + # ahead a little rather than only at the condition itself. + block_refuses = any(REFUSAL.search(l) for l in lines[i : i + 3]) + if not ( + SECURITY_HINTS.search(line) + or REFUSAL.search(line) + or (line.rstrip().endswith("{") and block_refuses) + ): continue # Skip lines a human has already judged unkillable, with a reason. # Scans the whole contiguous comment block above, not just the line @@ -180,7 +200,7 @@ def generate(source: str) -> list[Mutant]: # entirely, that check is unverified. # Block form: `if (cond) {` guard = re.match(r"^(\s*)if \((.+)\) \{\s*$", line) - if guard and SECURITY_HINTS.search(guard.group(2)): + if guard and (SECURITY_HINTS.search(guard.group(2)) or block_refuses): mutants.append( Mutant( i, @@ -201,7 +221,7 @@ def generate(source: str) -> list[Mutant]: if ( inline and not line.rstrip().endswith("{") - and SECURITY_HINTS.search(inline.group(2)) + and (SECURITY_HINTS.search(inline.group(2)) or REFUSAL.search(inline.group(3))) ): mutants.append( Mutant(