Application security audit — `witness-dao`

Scope: witness-dao/ (TypeScript, ESM, Node >= 20, zero runtime dependencies) — full src/ tree, test/ tree, package.json, pnpm-lock.yaml, .npmrc, governance-rules/. Date: 2026-07-25 · Type: adversarial read-only appsec review. No source file was modified. Assumed adversary: can influence (a) responses from Snapshot / Safe tx-service / EVM RPC / OTS calendars / Rekor / RFC-3161 TSAs / the Witness API, (b) CLI arguments and config, (c) MCP tool inputs from an AI agent, (d) the contents of the governance rules directory. Method: full read of every file in src/, plus working proof-of-concept exploits in /tmp for the load-bearing claims. Findings marked CONFIRMED were executed; THEORETICAL were reasoned from code but not run.

Verdict

No — not as-is. This is unusually careful code: the cryptographic core, the honesty/grading model, the ASN.1 framing, the Safe pagination SSRF guard and the rules-path traversal defences are all better than what I normally find, and the zero-dependency supply chain is genuinely clean. But the codebase has a systemic gap that its own design does not account for: it treats third-party HTTP responses and the governance rules directory as trusted inputs to hand-rolled parsers, with no size limit, no egress allowlist, and no resource budget. Concretely, I confirmed by execution: arbitrary command execution when the rules directory (or any ancestor) contains an attacker-written .git/config; a 146 KiB reply from a plain-HTTP RFC-3161 timestamp server that stalls the event loop for 5 seconds and scales unboundedly; a ~1 MiB eth_getLogs response that kills the process with an uncatchable heap OOM; one OTS calendar reply that turns into up to 320,000 outbound requests against a public third party and persists so it re-fires every cadence; a minted Witness API key that lands in a world-readable file (mode 0644) or through a symlink, because fs.writeFile's mode argument is ignored on an already-existing path; and Markdown injection that forges a PROVEN / ANCHORED_VALID evidence row into the artefact intended for counsel and a court. For a tool whose entire value proposition is "evidence you can rely on against a determined challenge", the last one is the most damaging in kind even though it is not the most severe in class. There are also no rate limits, no circuit breaker, and no HTTP response-size cap anywhere in the codebase, and src/cli/state.ts — which handles the credential file — has zero test coverage. The remediation list below is short and mostly cheap: items 1–7 are a few days of work, and after them I would be comfortable with an enterprise pilot. Two structural items (MCP tool authorisation, and treating the rules repo as a trust boundary) need a design decision, not just a patch.

Deployment-grade note: this reads as production-intended code, not a PoC — the disclaimer machinery, exit-code contract and determinism guarantees are only worth building for production. I have graded it accordingly. If it is in fact a PoC, items 1, 3 and 7 still need fixing before it touches a real API key.

Findings

Severity counts: 1 Critical, 6 High, 6 Medium, 5 Low, 3 Info.

Per-finding detail

A-01 — Critical — Arbitrary command execution via git config in the rules directory

Location: src/binding/rules.ts:398 (git status --porcelain), runner at :427-466.

Mechanism. The rules resolver correctly uses execFile with an argument array, never a shell string, and correctly terminates options with -- at both call sites (:358, :398). Argument injection is genuinely closed. But the attack does not need argument injection: git status executes the value of the repo-local core.fsmonitor configuration variable as a hook command. git discovers the repository by walking upward from -C <cwd>, so .git/config in rulesDir or in any ancestor directory is honoured. The GIT_OPTIONAL_LOCKS=0 and GIT_TERMINAL_PROMPT=0 already set at :439-443 do not prevent it, and env: { ...process.env } hands the subprocess the full environment including WITNESS_API_KEY.

PoC (executed, git 2.34.1, Linux):

git init -q /tmp/gitpoc/rules && cd /tmp/gitpoc/rules
echo "# rule" > default.md && git add . && git commit -qm init
git config core.fsmonitor '/bin/sh -c "id > /tmp/gitpoc/PWNED_fsmonitor; echo"'
touch default.md   # make the index look stale
GIT_OPTIONAL_LOCKS=0 GIT_TERMINAL_PROMPT=0 \
  git -C /tmp/gitpoc/rules status --porcelain -- /tmp/gitpoc/rules/default.md
# => /tmp/gitpoc/PWNED_fsmonitor exists, containing `uid=1066(...)`

Also confirmed with the .git directory in the parent of rulesDir, i.e. rulesDir need not be the repository root.

Realistic exploit in this deployment. Three paths, all plausible for a scheduled CI job or an MCP server:

1. rulesDir (or an ancestor) is on a shared or world-writable volume — a CI workspace, a network share, anything under /tmp. A local co-tenant writes .git/config and waits for the next witness-dao seal run.
2. The rules directory is populated by unpacking an archive. A tar/zip can contain .git/config; a git clone cannot, which is why this is not a pure "hostile repo" attack.
3. WITNESS_DAO_RULES_DIR / --rules-dir is itself attacker-influenced (explicitly in the stated threat model, item (b)) — point it at any directory the attacker already controls.

Git's safe.directory "dubious ownership" check does not help: it only fires when the repository is owned by a different user, and my PoC ran same-user.

Fix. Pass the config on the command line, where -c outranks repo config — verified to block the PoC:

execFile(gitPath, [
  '-C', cwd,
  '-c', 'core.fsmonitor=false',
  '-c', 'core.hooksPath=/dev/null',
  '-c', 'core.pager=cat',
  '-c', 'diff.external=',
  '-c', 'protocol.allow=never',
  ...args,
], { /* ... */ })

Additionally: stop passing the full process.env — pass a minimal allowlist (PATH, HOME, LANG, plus the GIT_* values already set) so the Witness key never enters a subprocess; and consider refusing to operate when stat(rulesDir).uid !== process.getuid(). Document the rules directory as a trust boundary in LIMITATIONS.md.

A-02 — High — No SSRF controls on any primary endpoint; redirects followed

Location: src/core/http.ts:82-87; validation at src/config.ts:172-200; stripTrailingSlash at src/seal/witness-client.ts:547-556.

Mechanism. Every egress URL is caller- or config-controlled: WITNESS_BASE_URL, WITNESS_DAO_RPC, WITNESS_DAO_SNAPSHOT_URL, WITNESS_DAO_REKOR_URL, WITNESS_DAO_OTS_CALENDARS, and safeTxServiceUrls from the config file. The only validation performed is isHttpUrl() — a scheme check, nothing more. I grepped the entire src/ tree: there is no check anywhere for 169.254.169.254, 127.0.0.0/8, 10./172.16./192.168., ::1, [::ffff:127.0.0.1], .internal, or DNS names resolving into private space. fetch is called with no redirect option, so undici's default 'follow' applies (grep for redirect in src/ returns only an unrelated comment), meaning even a correctly configured endpoint can 302 the client to http://169.254.169.254/latest/meta-data/iam/security-credentials/.

What is genuinely handled — stated for calibration. Three real mitigations exist and I verified each:

• Cross-origin Authorization stripping. I ran the concrete test the brief asked for: a local server 302s to a different origin while the client sends authorization, cookie and x-custom. Node 22.22.3's undici strips authorization and cookie, and retains x-custom. So the Witness bearer token is not leaked to a redirect target. This is the difference between High and Critical here — but note it is an unasserted dependency on runtime behaviour, not a property of this code, and no test in test/ covers it.
• src/binding/safe.ts:502-521 explicitly validates the Safe tx-service next pagination cursor is same-origin, with a comment naming it as an SSRF primitive. Correct and well done.
• src/anchor/ots.ts:1265-1331 allowlists OTS upgrade URIs by origin against the configured calendars. Correct.

The gap is that these three good instincts were not generalised to the primary endpoints.

Realistic exploit. In an enterprise Azure/AWS deployment, the CLI runs unattended in CI with a service identity. An operator-supplied or accidentally-templated WITNESS_DAO_RPC=1=http://169.254.169.254/metadata/instance?api-version=2021-02-01 returns the instance metadata document, which httpRequest then embeds verbatim (truncated to 300 chars) into an HTTP_BAD_JSON / HTTP_STATUS error message that lands in the CI log and in --json output. On AWS IMDSv1 the same shape yields IAM credentials. scrubSecrets will not catch these — it only knows values already present in the environment.

Fix. Add a single egress guard in httpRequest:

1. Set redirect: 'error' (this codebase's endpoints do not legitimately need redirects) — or at minimum redirect: 'manual' with the same origin/IP validation re-applied to the Location.
2. Resolve the hostname and reject loopback, link-local, private, CGNAT, unique-local and multicast ranges (both IPv4 and IPv4-mapped IPv6) before connecting, with an explicit allowPrivateEgress escape hatch for self-hosted hubs and local test doubles. Re-check after DNS resolution to close the rebinding window.
3. Fold the check into config.validate() too, so a bad endpoint fails at load rather than at first request — consistent with the existing (good) fail-fast philosophy in that function.

A-03 — High — No HTTP response-size cap anywhere

Location: src/core/http.ts:150-168 (parseBody), :189-195 (safeText).

Mechanism. parseBody calls res.arrayBuffer() or res.text() with no Content-Length pre-check, no streaming, and no byte budget. truncate(text, 300) at :197 runs after the whole body is already in memory and only shortens the error string. Every downstream parser — the DER decoder, the OTS tree reader, the ABI hex decoder, the Rekor JSON handler, the Snapshot GraphQL envelope — therefore receives an attacker-sized buffer.

The codebase does have size caps elsewhere, which makes the omission look like an oversight rather than a decision: MAX_LINE_BYTES (16 MiB, MCP stdin), MAX_RULE_BYTES (1 MiB, rule files), maxBuffer (1 MiB, git stdout), MAX_DECODED_BYTES/MAX_DECODED_ELEMENTS (ABI), MAX_VARUINT_BYTES (OTS). None of them bounds an HTTP response body.

Fix. One change with the widest blast radius in this report: reject on Content-Length above a per-responseType budget, and stream-read with a running byte counter that aborts the controller when exceeded (a Content-Length header cannot be trusted). Suggested defaults: 8 MiB JSON, 1 MiB for bytes (an OTS proof or a timestamp token is kilobytes), 64 KiB for safeText error bodies. This alone blunts A-04, A-05 and A-06.

A-04 — High — Quadratic BigInt accumulation in the RFC-3161 DER integer decoder

Location: src/anchor/rfc3161.ts:484-497; reached from :762 (PKIStatusInfo.status), :972 (TSTInfo.serialNumber), :992 (decodeAccuracy).

Mechanism. for (const byte of node.content) v = v * 256n + BigInt(byte) over a DER INTEGER of unbounded length. Each iteration re-multiplies a BigInt whose width grows with the input, so the cost is super-quadratic in limb operations. The ASN.1 framing around it is well hardened (see "what is well done"), but there is no length cap on the INTEGER content itself — and a PKIStatus is at most 4 bytes, a serial number at most 20.

Measured (Node 22.22.3): decodeInteger alone: 50 KB → 336 ms; 200 KB → 13,654 ms (4× input, 40× time). End-to-end through the real parseTimeStampResp: 49 KiB reply → 325 ms; 146 KiB reply → 5,119 ms of synchronous event-loop block. Extrapolated: 1 MiB ≈ 250 s, 10 MiB ≈ hours. Unbounded, because A-03 caps nothing.

Realistic exploit. This is the smallest payload-to-damage ratio in the repo and the easiest to reach, because both default TSAs are plain HTTP: http://timestamp.digicert.com and http://timestamp.sectigo.com (rfc3161.ts:96-99). Any on-path attacker — a hostile Wi-Fi, a compromised egress proxy, a BGP/DNS hijack — substitutes a 150 KiB response and blocks the process. Because the block is synchronous inside the BigInt loop, it freezes everything: the MCP server stops answering ping, concurrent seals stall, the CLI hangs past its own timeoutMs (the AbortController cannot fire while the event loop is blocked). anchorRfc3161 walks two TSAs, so one attacker gets two bites.

Fix. Cap node.content.length in decodeInteger (reject > 32 bytes; callers here need ≤ 20) and return a WitnessDaoError with a code. Independently, move the default TSAs to HTTPS or document loudly that RFC-3161 responses are unauthenticated in transit and that the timestamp token's own signature is the only integrity guarantee.

A-05 — High — ABI offset aliasing → uncatchable heap OOM

Location: src/binding/governor.ts:829-868 (decodeDynamic), reached from :959 (parseLog).

Mechanism. For an array of dynamic elements, each element's offset word is read independently at :857-860. Nothing requires the offsets to be distinct, so all N elements may point at the same large string. MAX_DECODED_BYTES bounds a single element; MAX_DECODED_ELEMENTS bounds the count; neither bounds the product. parseLog applies no length limit to log.data, and maxLogs (default 50,000) only counts rows.

Measured: 1.0 MiB of data → 1,000 MiB of decoded strings in 1.2 s (×970 amplification; ×199 at n=200, ×492 at n=500 — linear in element count). At n=20,000 with a 2 GB heap: FATAL ERROR: Reached heap limit Allocation failed - JavaScript heap out of memory. This is process death, not a catchable exception — the fail-open seal handling and the problems[] channel cannot report it, so from the operator's side the evidence run simply vanishes with no gap recorded. That is precisely the silent-hole failure src/cli/state.ts exists to prevent. bytes[] is ~2× worse because bufferToHex expands.

Realistic exploit. A DAO configures WITNESS_DAO_RPC at a public RPC provider (the normal case). That provider is compromised, rug-pulled, or MITM'd, and returns one eth_getLogs row with topic0 = ProposalCreated and a few MiB of crafted data.

Fix. Carry a single mutable byte budget through the whole decodeAbiParameters call rather than per-element, and reject duplicate or overlapping element offsets (a well-formed encoder never emits them). Cap log.data length in parseLog.

A-06 — High — Unbounded OTS upgrade fan-out, persisted across restarts

Location: src/anchor/ots.ts:1319-1345 (upgradeOtsProof), enabled by :595 and :603-613 (readTimestamp), aggravated by :705-731 (mergeTimestamps) and :1381-1406 (spliceAt).

Mechanism. Two independent defects compose:

• Breadth is unbounded. MAX_TREE_NODES = 20_000 is incremented once per node at :595, but the while (tag === TAG_FORK) loop at :607-610 accepts unlimited branches on a single node. MAX_TREE_DEPTH = 256 is correctly enforced — the breach is width, not depth. Measured: 200,000 branches on one node parsed from an 8.8 MiB reply in 207 ms, no limit hit.
• Fan-out is unbounded. Promise.allSettled(targets.map(...)) with no concurrency cap. The origin allowlist at :1328 compares only u.origin, so https://a.pool.opentimestamps.org/1, /2, … are all "allowed" and all distinct dedupe keys.

Measured: a 985 KiB calendar reply → 20,000 distinct upgrade targets; 3,974 KiB → 80,000 distinct targets, i.e. that many simultaneous HTTP GETs, ×4 with the default retries: 3 = up to 320,000 requests at the real a.pool.opentimestamps.org. Separately, mergeTimestamps is O(n·m) (2,000 branches → 21 ms; 16,000 → 947 ms; extrapolated 200,000 → ~150 s per merge, and submitToCalendars:1239 merges up to four calendars sequentially into a growing tree), and spliceAt re-serialises the whole subtree twice per target (20,000 targets ≈ 2,150 s).

Realistic exploit. The poisoned proof is accepted and persisted by submitToCalendars at :1225, so it re-detonates on every subsequent witness-dao anchor upgrade cadence run — a self-sustaining DDoS-by-proxy against the free public calendar infrastructure this product depends on, launched from the enterprise's own IP. Expect the IP to be blocked, and expect the DAO's Bitcoin anchoring — the only thing that makes records compliance-grade — to stop working as a result.

Fix. Count branches against MAX_TREE_NODES at :607; add a hard MAX_BRANCHES_PER_NODE (16 is generous); cap targets.length and run the fan-out through a small concurrency limiter (4–8); index merge candidates by a Map keyed on the operation instead of a linear branches.find; drop the serialise-to-compare in spliceAt. Also validate the path, not just the origin, of an upgrade URI, or cap distinct paths per origin.

A-07 — High — Minted Witness API key written world-readable / through a symlink

Location: src/cli/state.ts:201-205 (writeSecretFile), called from src/cli/run.ts:1034 (keys mint). Same defect in writeJsonFile at :82-87.

Mechanism. The comment at :204 claims "0600 before any content is written: never a window where the file is readable." That is not what fs.writeFile does. The mode option is passed to open(2) and is only applied when the file is created (O_CREAT); on an existing path the flag is 'w' (O_TRUNC), the mode is ignored, and the existing permissions survive. There is no O_EXCL, and writeFile follows symlinks.

PoC (executed):

case A: fresh file                                     -> mode 600   (as documented)
case B: file pre-created 0666 by an attacker           -> mode 644   (SECRET, world-readable)
case C: symlink pre-created at the target path         -> secret written through the link,
                                                          target mode 644

Realistic exploit. keys mint defaults to ${stateDir}/minted-api-key — a predictable path (.witness-dao/minted-api-key relative to cwd). mkdir at :202 uses the default 0777 & ~umask, so on a typical umask 022 the containing state directory is 0755, i.e. traversable. A local co-tenant on a shared build host, or any process running as another user on the same box, pre-creates that path as an empty 0666 file (or a symlink into a directory they can read) and harvests the Witness bearer token the next time an operator mints one. The same mechanism applies to writeJsonFile's temp path ${path}.${process.pid}.tmp — predictable, no O_EXCL, symlink-followed — giving an arbitrary-file-write primitive as the CLI's user, and rename then clobbers the target.

This is the one place the codebase handles a live credential on disk, and it is the one place with zero test coverage (see A-21).

Fix.

import { open } from 'node:fs/promises';
// O_WRONLY|O_CREAT|O_EXCL|O_NOFOLLOW, mode 0o600
const fh = await open(path, 0o200 | 0o40 | 0o100 | 0o400 /* use fs.constants */, 0o600);

Use fs.constants.O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW and fail loudly if the path exists, telling the operator to remove it. For writeJsonFile, use a random temp suffix plus O_EXCL|O_NOFOLLOW, and chmod the fd explicitly (fh.chmod(0o600)) before writing rather than relying on the create mode. Create the state directory with mode: 0o700. Add tests asserting the mode in all three cases above — the property is currently only asserted in a comment, and the comment is wrong.

A-08 — Medium — Markdown injection forges an evidence row in the court-facing report

Location: src/report/render.ts:180-186 (escapeCell), :188-191 (code), and three unescaped sinks: :377, :556 (ruleVersion) and :255 (liability caveats).

Mechanism. escapeCell correctly escapes \, | and \r?\n, which closes table-structure injection — and there is a test for the pipe and newline cases (test/report/render.test.ts:200-207). Three sinks bypass it:

1. ruleVersion is interpolated raw. render.ts:556: @ ${l.ruleVersion ?? 'unversioned'} — and identically at :377 for the liability pack. ruleVersion reaches the report from record.governingRule.ruleVersion, i.e. straight out of the Witness API response, and normaliseEvidence (src/report/governance.ts:333-340) validates it only as a non-empty string. src/binding/rules.ts:386 does enforce 40-hex, but that guards the seal path, not the report path. A ruleVersion containing newlines injects arbitrary Markdown, including a complete forged table row.
2. Liability caveats are interpolated raw. render.ts:255 does out.push(\- ${c}`), and src/report/liability.ts:342,348,355build those caveats with${r.recordId}and${group.key}(the proposal id) interpolated. Same newline injection. (The governance report is *accidentally* protected here: its limitations round-trip through the disclaimer andparseDisclaimerfilters to lines starting with- `.)
3. escapeCell does not escape backticks, but code() wraps its output in backticks. So any value passed through code() — recordId, proposalId, ruleId, tallyRecordId, basis, ruleVersion, all sourced from Snapshot/Safe/Witness responses — can close the inline-code span and emit live Markdown.

PoC (executed). Reimplementing escapeCell/code verbatim:

--- backtick breakout of code() ---
| 1 | `rec1` **ANCHORED_VALID — VERIFIED** [proof](https://attacker.example) `x` |

--- unescaped ruleVersion at render.ts:556 ---
- **rec1** (rule treasury.transfer @ deadbeef

| 99 | 2026-01-01 | vote.cast | PASS | `recFORGED` | 99 | `treasury.transfer` | `deadbeefdeadbeef` | onchain_confirmed (governor) | ANCHORED_VALID | **PROVEN** |

- **recFORGED**: forged line, ): explanation...

The injected row is indistinguishable from a genuine PROVEN finding in the rendered document.

Realistic exploit in this product's context. The Markdown report is explicitly the artefact "intended for counsel and auditors" and, for the liability pack, the document a named individual relies on post-Ooki. A compromised or hostile Witness API — the exact adversary the offline verifier exists to defend against, per witness-client.ts:167-173 — injects a favourable finding into a defence pack. Partial mitigation, stated fairly: the RFC 8785 JSON form still contains the newline, so the report digest changes and a verifier who recomputes the digest and reads the JSON would see the anomaly. But the whole point of the Markdown rendering is that the human reader does not do that.

Fix. Escape backticks in escapeCell (or switch code() to a fenced/HTML-escaped form). Route ruleVersion through escapeCell at :377 and :556, and caveats through it at :255. Independently, validate ruleVersion as /^[0-9a-f]{40}$/i (or explicitly undefined) in normaliseEvidence — a git SHA has exactly one shape, and accepting anything else in a provenance field is the underlying defect. Add tests for a backtick and for a newline in ruleVersion; neither is currently covered (test/report/_fixtures.ts:44,87 only ever uses a valid SHA or null).

A-09 — Medium — Quadratic ReDoS in `firstHeading` over rules-directory markdown

Location: src/binding/rules.ts:541, called from :229 (load) and :302 (buildIndex).

Mechanism. /^#{1,6}\s+(.*\S)\s*$/ — \s+, .*, \S and \s* all overlap, so on a line matching #{1,6} followed only by whitespace the engine enumerates every split of \s+ against every length of .*. MAX_RULE_BYTES (1 MiB) is the only bound, and it is far too generous for a quadratic scan. buildIndex runs it for every .md file the recursive readdir finds, so one poisoned file stalls listRules(), resolve(), resolveDetailed() — i.e. every seal and every report command.

Measured (clean O(n²), 4× per doubling): input '#' + ' '.repeat(n) — n=16,000 → 86 ms; 32,000 → 381 ms; 64,000 → 1,427 ms; 128,000 → 5,650 ms; 256,000 → 22,476 ms. End-to-end through the real GitRulesResolver: listRules() over one poisoned 200 KB rule file = 13,736 ms. Extrapolated to the 1 MiB cap: ≈ 6.3 minutes of blocked event loop per file, synchronous and unabortable.

Realistic exploit. A merged PR to the governance-rules repo, or any writer to WITNESS_DAO_RULES_DIR. The payload looks like an innocuous blank heading and would survive review. Rated Medium rather than High because the rules repo is a reviewed artefact — but note that the same trust assumption is what A-01 breaks completely.

Fix. One character: /^#{1,6}\s+(\S.*?)\s*$/. Or drop the regex: line.replace(/^#{1,6}/, '').trim().

A-10 — Medium — `eth_getLogs` chunk loop: up to 400,000 requests from one call

Location: src/binding/governor.ts:514-590; secondary at :628-632.

Mechanism. The brief's premise is wrong on mechanism and I want to be precise: this is an iterative while loop (span = span/2; continue), not recursion. There is no stack growth and no 2^depth explosion, and MAX_CHUNK_ITERATIONS = 100_000 (:105, enforced :529) does bound it. The problem is that the bound is enormous. On a range-cap error the loop halves span and continues without advancing cursor (:553-557); after each success it doubles back toward chunkSize (:589). An RPC that rejects any span > 1 but answers span == 1 produces ~2 requests per block until the guard trips.

Measured against a local hostile RPC: exactly 100,000 eth_getLogs requests (100,001 RPC calls) in 22.0 s, then GOVERNOR_CHUNK_LOOP. Against a remote host with the default timeoutMs: 30_000 that is a multi-day hang. Worse: if the cap error arrives as HTTP 429 or 503 — both in RETRYABLE_STATUS (http.ts:30) — each iteration costs 4 requests plus 6 s of jittered backoff, giving **400,000 requests over ~7 days**. Any of the ~12 patterns in isRangeCapError (:677-697) triggers it, including a bare "limit exceeded". A hostile eth_blockNumber head with fromBlock: 0 reaches the same place with no error at all.

Secondary: loadBlockTimestamps (:628-632) issues one sequential eth_getBlockByNumber per distinct block. A node returning maxLogs (50,000) logs across 50,000 distinct blocks forces 50,000 more requests (×4 with retries) before any decoding happens.

Fix. Lower MAX_CHUNK_ITERATIONS to something a human would accept (a few thousand) and make it a documented, configurable budget. Track a total-request counter for the whole listEvents call and abort with a clear problems[] entry. Refuse to halve below a floor (say 16 blocks) and fail rather than grind. Batch or concurrency-limit loadBlockTimestamps.

A-11 — Medium — `offlineVerifyChain` is O(n²)

Location: src/verify/offline.ts:379-390.

Mechanism. ordered.map((record, idx) => offlineVerify({ record, chain: ordered.slice(0, idx+1), ... })). Each of the n calls re-enters verifyChain, which re-sorts the slice (:443) and re-canonicalises and re-SHA-256s every record in it (:474-487). Total: n(n+1)/2 canonicalisations plus n sorts → O(n² log n).

Measured: n=250 → 309 ms; 500 → 1,014 ms; 1,000 → 3,702 ms; 2,000 → 14,565 ms (clean 4× per doubling). Extrapolated: n=10,000 ≈ 6 min; n=100,000 ≈ 10 hours. The chain is trivially forgeable because the caller also supplies the didDocument — an attacker generates their own Ed25519 key and signs a self-consistent chain; my PoC chain verifies to SIGNED_PENDING.

Scope, stated fairly: OfflineVerifyBundle.chain (:78) has no length cap and the public API (src/verify/index.ts:12) is exposed, but the MCP path is not affected — src/mcp/server.ts:448 calls single-record offlineVerify, which is linear (chain length 20,000 → 144 ms). requireChain is not a mitigation: :273 enters verifyChain whenever chain.length > 0 regardless, and with requireChain: true the attacker just starts the chain at seq 0. Cycles cannot hang it (a repeated record yields equal seq, caught at :492 → INSUFFICIENT_PROOF).

Fix. Compute each record's canonical hash once and memoise it across slices — that alone turns O(n²) into O(n). Add a maxChainLength cap (10,000 is generous) on OfflineVerifyBundle.

A-12 — Medium — Deep nesting → `RangeError` reported as verdict `BROKEN` ("tampering")

Location: src/core/canonical.ts:30-93; consequence at src/verify/offline.ts:154.

Mechanism. serialize() recurses per object/array level with no depth counter. Measured stack-overflow threshold: objects at ~1,671 deep (ok) / ~1,687 (throws); arrays ~4,543 / ~4,562. JSON.parse in V8 accepts far deeper — a 900 KB body nested 150,000 deep parses fine, then canonicalize throws RangeError: Maximum call stack size exceeded.

Why this is a security finding and not just a crash. In offlineVerify the RangeError is caught at :154 and converted to verdict BROKEN / body_not_canonicalisable. The module's own header (:14-20) defines BROKEN as integrity actually failed: tampering, and both the MCP tool description and the CLI exit-code contract (EXIT.VERIFICATION_FAILED = 3) treat it as a tampering allegation that must never be conflated with INSUFFICIENT_PROOF. So a merely deeply-nested record — including a legitimate record whose payload came from a hostile upstream — produces a false accusation of tampering in an evidence tool, which is exactly the cry-wolf failure the design explicitly forbids. I confirmed this: offlineVerify deep payload -> BROKEN.

Other canonicalize/canonicalHash call sites are not wrapped in try/catch — src/report/render.ts:71, src/seal/sealer.ts:321, src/binding/provider.ts:334, src/anchor/rekor.ts:545,859,872 — so a stack overflow there escapes as a bare RangeError, violating the library's "every failure is a WitnessDaoError with a code" contract. Reachability from hostile HTTP is limited today because the providers extract detail fields explicitly rather than splatting raw API JSON; treat those as THEORETICAL.

Fix. Add an explicit depth cap (256 is far beyond any legitimate record) in canonicalize, throwing WitnessDaoError('CANONICAL_TOO_DEEP'). In offline.ts:154, map a canonicalisation capability failure to INSUFFICIENT_PROOF with a distinct reason, reserving BROKEN for a hash or signature that genuinely mismatches.

A-13 — Medium — Unauthenticated MCP tools perform privileged actions

Location: src/mcp/server.ts:234-367 (seal_governance_events), :477-539 (report tools).

Mechanism (design, not a bug). MCP over stdio has no authentication by design, which is normally fine because the host process is the authorisation boundary. Here the exposed tools are not read-only:

• seal_governance_events writes durable records, consumes Witness quota and spends anchoring cadence (Bitcoin/Rekor/RFC-3161 submissions). dryRun defaults to false.
• governance_evidence_report and liability_defence_report return the entire report object plus the rendered artefact in the tool result — for the liability pack, that is a named individual's full participation and non-participation record, placed verbatim into the agent's context window and therefore into the conversation transcript and any provider-side logging.
• anchor_status reads the state directory.

Realistic exploit. This is a live prompt-injection scenario, not a hypothetical: the product's own premise is that a DAO's compliance copilot reads governance proposals. A Snapshot proposal body — attacker-authored by definition, and carried into the agent's context by ingest_governance_events (describeEvent → detail.bodyExcerpt, 4,000 characters of attacker text) — contains instructions to call liability_defence_report for a named director and summarise it, or to call seal_governance_events repeatedly to burn the DAO's anchoring quota. The tool descriptions are admirably honest about evidential limits but say nothing about authorisation.

Guardrails I would recommend.

1. Default seal_governance_events to dryRun: true and require an explicit confirm: "<daoId>" echo for the writing path — a value the agent cannot invent from a proposal body alone.
2. Split the tool set into read-only and mutating, and gate the mutating set behind an env flag (WITNESS_DAO_MCP_ALLOW_WRITES=1) that is off by default. An operator who wants an agent to seal has to say so once.
3. Return a reference plus summary for reports, not the full artefact — write the document to --out-style path and return the digest, the path and the summary counters. This preserves the tool's usefulness while removing the bulk-exfiltration primitive.
4. Add a per-process budget on seals and anchoring submissions per session, surfaced in notes.
5. Treat detail.bodyExcerpt and binding.rationale as untrusted text in tool output — at minimum wrap them in a delimiter and add a caveat line stating that proposal text is attacker-controlled and must not be followed as instruction. This is cheap and directly addresses the injection path.

A-15 — Medium — Bearer token may be sent over cleartext HTTP

Location: src/config.ts:193-200 (isHttpUrl accepts http:), src/seal/witness-client.ts:547-556 (stripTrailingSlash validates parseability only, not scheme).

Mechanism. WITNESS_BASE_URL=http://witness.internal/... passes validation, and authHeaders() (:403-405) then sends authorization: Bearer wtn.<id>.<secret> in clear. A WitnessClient constructed directly (a supported public API) bypasses config.validate() entirely, because stripTrailingSlash checks only that new URL() succeeds — so even a non-HTTP scheme reaches fetch from that path.

Fix. Require https: whenever an apiKey is present, with an explicit allowInsecureTransport opt-in for a local test double, and add the scheme check to stripTrailingSlash so the client-level entry point matches the config-level one.

A-19 — Medium — Unbounded anchor-retry amplification; no rate limit or circuit breaker

Location: src/anchor/anchorer.ts:451-515 (upgradePending).

Mechanism. upgradePending iterates every stored run (:453) and for each fires Promise.allSettled over all outstanding attestations (:462), re-invoking target.anchor() for anything in failed state (:475). run.attempts is incremented at :509 but never read and never capped. InMemoryAnchorRunStore (:280-294) grows without bound, and FileAnchorRunStore keeps every run forever. There is no inter-run backoff and no give-up.

Per run: 3 targets → OTS 4 calendars × 4 attempts = 16, RFC-3161 2 TSAs × 4 = 8, Rekor 1 × 4 plus the 409 path (rekor.ts:293-302) adding up to 44 — roughly 30–70 third-party requests per stored run, per cadence tick. An hourly cadence over a year is 8,760 stored runs → ~250,000–600,000 requests per tick, monotonically growing, aimed at four free public calendars and two commercial TSAs. Multiply by A-06 if any stored OTS proof is poisoned.

I confirmed by grep that there is no rate limiter, no circuit breaker, no global concurrency cap and no request budget anywhere in src/ — the only concurrency control in the codebase is src/seal/sealer.ts:131 (seal concurrency 1–16).

Fix. Cap run.attempts (e.g. 24) and move a run to a terminal abandoned state with a loud problems[] entry — an outstanding anchor that will never succeed is information the operator needs, and the codebase's own philosophy says a gap must never be invisible. Add exponential backoff keyed on attempts. Add a per-target circuit breaker that opens after N consecutive failures. Cap the number of runs upgradePending processes per invocation.

A-14 — Low — `verify_record`'s `bundle` is a schema-less pass-through

src/mcp/server.ts:387-394 declares bundle as { type: 'object', description: ... } with no properties, so validateSchema performs only the object-vs-not check; the handler then adds one manual guard (candidate.record must be an object, :427) and casts. Any shape reaches offlineVerify. Combined with the 16 MiB line limit this is the vehicle for A-11 and A-12 over MCP. Recommend declaring the bundle's real shape, including a maxItems-equivalent cap on chain and anchors (the hand-rolled validator would need maxItems added — a ~10-line change).

Related, in the same validator: validateSchema correctly uses Object.hasOwn at :329, and JSON.parse makes __proto__ an own data property, so a __proto__ key in tool arguments is rejected under additionalProperties: false. But no test pins that (grep for __proto__ across test/ returns nothing), and any schema that omits additionalProperties: false or uses the untyped pass-through branch (:382-384) passes such a key straight to the handler. Add the test.

A-16 — Low — `assertNoSecretsInFile` gaps

FORBIDDEN_FILE_KEYS (src/config.ts:38) covers apiKey, api_key, apikey, secret, privateKey, password — matched case-insensitively at any depth, with good test coverage including an 8-deep nesting case (test/config.test.ts:142-179). It omits authorization, bearer, credential, accessToken, refreshToken, pat, sessionKey. So {"witness":{"authorization":"Bearer wtn.a.b"}} in a committed config file is accepted silently. (token is deliberately excluded and there is a test asserting so at :183-189 — "in a DAO config it usually means an asset" — which is a defensible judgement call; the others look like oversights.) The check is defence-in-depth only, since apiKey is never read from a file, hence Low.

Secondary: the comment at :157-161 justifies having no depth cap on the ground that the input is always a JSON.parse result. But resolveConfig is exported public API and accepts fileConfig from any caller, so a cyclic or 200k-deep object causes unbounded recursion / RangeError outside the WitnessDaoError contract. Add a depth cap (64) and a WeakSet cycle guard — both are compatible with the stated goal, since no legitimate config is 64 levels deep.

A-17 — Low — `LineFramer` bound is looser than documented

src/mcp/jsonrpc.ts:105-123: this.buffer += chunk happens at :106, and this.buffer.length > this.maxBytes is only checked at :118, after the whole chunk has been concatenated — so the peak is maxBytes + chunkSize and a single very large chunk is fully materialised (twice, during concatenation) before the throw. Also maxBytes counts UTF-16 code units, not bytes, so a 16 M-character buffer of multi-byte characters is 32–64 MB of memory despite the name. In practice stdin chunks are 64 KB, so this is hardening rather than an exploit. Check the incoming chunk length against the remaining budget before concatenating, and rename to maxChars or measure with Buffer.byteLength. The existing test (test/mcp/jsonrpc.test.ts:76-81) uses a 32-byte limit and a 64-byte chunk, which cannot detect this.

A-18 — Low — `specs[name]` without `Object.hasOwn`

src/cli/args.ts:102: const spec = specs[name]. Because specs is a plain object literal, specs['__proto__'], specs['toString'], specs['constructor'], specs['valueOf'] all return inherited values rather than undefined, so the "Unknown option" branch at :104 is skipped and the token is silently treated as a string flag. No prototype pollution results (assigning a string to __proto__ invokes the setter, which ignores non-objects, and applyDefaults iterates own entries only), so this is a correctness/robustness gap rather than a vulnerability. Use Object.hasOwn(specs, name), and build the alias index and the values object with Object.create(null) or a Map.

A-20 — Low — `--out` has no traversal or overwrite check

src/cli/run.ts:802-803 does mkdir(dirname(writtenTo), { recursive: true }) then writeFile(writtenTo, ...), with resolveFrom at :1177 resolving relative paths against cwd. Any path the process can write is overwritten silently and parent directories are created. This is arguably the intended semantics of --out for a hand-driven CLI, and I would not change it for that use. It becomes a finding if the CLI is ever invoked with arguments from an untrusted source (a webhook, a job queue, an agent). If so: require the resolved path to stay under an allowed output root, refuse to follow a symlink, and refuse to overwrite without --force.

A-21 — Info — Untested security properties

Properties asserted in comments or docs but not covered by any test:

• src/cli/state.ts has no test file at all, and no file under test/ imports it (grep for FileSealJournal, FileAnchorRunStore, writeSecretFile, STATE_FILES returns only src/ hits). That covers the 0600 claim (which is wrong — A-07), the temp-file mode, the atomic-write guarantee and the corruption-refusal behaviour.
• The git argv and the -- option terminator: present at rules.ts:358,398, but every fake runner in test/binding/rules.test.ts inspects only args[0]. Nothing would fail if -- were dropped or the array replaced by a shell string. Add a deepEqual on the exact argv.
• Redirect policy: no test anywhere references a 30x status or Location.
• Backtick escaping in escapeCell; a newline in ruleVersion.
• __proto__ in MCP tool arguments.
• encodeURIComponent(chainKey) in getChainProof (witness-client.ts:234) — the recordId equivalent is well tested (test/seal/witness-client.test.ts:404-408 asserts /records/rec%2F1), chainKey is not.
• Any bound on HTTP response size — because there is none.

Well covered, for balance: rules-path traversal in all five variants including a real symlink (test/binding/rules.test.ts:170-208), secret scrubbing end-to-end through both the CLI (test/cli/run.test.ts:352-380) and MCP (test/mcp/server.test.ts:459-493), redact() including the unparseable-URL fallback (test/core/http.test.ts:292-326), additionalProperties: false rejection, and the LineFramer bound.

A-22 — Info — `--experimental-strip-types` vs the declared Node floor

package.json declares engines.node >= 20.11.0, but the test, test:integration, cli and mcp scripts all pass --experimental-strip-types, which did not exist before Node 22.6.0. Those scripts therefore cannot run on Node 20 or 21 despite the declared floor. Relying on an unstable flag is acceptable for a dev/test loop, and the published artefact is dist/ compiled by tsc, so the runtime does not need it — that separation is correct and worth keeping. Recommend raising engines to >=22.6.0 (or adding a separate engines note for development), and pinning a Node version in CI so a runner upgrade cannot silently change type-stripping semantics.

A-23 — Info — Supply chain: clean (positive finding)

Verified rather than assumed:

• dependencies: {} is truthful. pnpm-lock.yaml contains exactly three packages — typescript@5.9.3, @types/node@22.20.1 and its transitive undici-types@6.21.0 — all dev-only, all with SHA-512 integrity hashes.
• .npmrc contains ignore-scripts=true with a comment naming the org policy. Compliant.
• Nothing is fetched from the network at build or test time.
• files: ["dist", "README.md", "LICENSE"] is tight: no test/, no src/, no state directory, no .env. declarationMap/sourceMap are on, so dist will carry .map files referencing ../src/*.ts, but inlineSources is off so no source content is embedded — broken source maps for consumers, no information leak. .gitignore correctly excludes dist/, .env* and .witness-dao-state/.
• One nit: LIMITATIONS.md is referenced by the project's own honesty framing but is not in files, so it does not ship to npm consumers.

What is genuinely well done

Calibrated, and I mean these — several are better than the norm for code of this size:

• The RFC-3161 ASN.1 framing is properly hardened. I attacked it directly and it held: MAX_DER_DEPTH = 32 enforced at :321 (depth 33, 100 and 100,000 all return DER_TOO_DEEP with no stack overflow), MAX_DER_NODES = 10_000 at :325, length-vs-remaining checked on every TLV at :391 before the subarray, DER_BAD_LENGTH on >6 length octets, DER_INDEFINITE_LENGTH on 30 80, non-minimal long form rejected at :383, and no infinite loop in the constructed-children walk. A hand-rolled DER parser that survives that battery is rare. The only defect (A-04) is in the integer value decoder, not the framing.
• Rules-path traversal is closed properly and tested properly. Lookup goes through a pre-built index rather than path concatenation, containment is re-checked after realpath (rules.ts:212), the recursive readdir re-checks containment per entry (:276), NUL and absolute paths are rejected, and there are real tests for every case including an actual symlink.
• git is invoked correctly at the argument level — execFile with an array, never a shell, -- present at both call sites, timeout and maxBuffer set, windowsHide. The A-01 defect is a git configuration problem, not an argument-injection one; the argument handling itself is right.
• The Safe pagination cursor SSRF guard (safe.ts:502-521) and the OTS calendar origin allowlist (ots.ts:1265-1331) show the right instinct applied at exactly the two places where remote data becomes a URL. My criticism is that it was not generalised, not that it is absent.
• Secret scrubbing is layered and end-to-end tested. redact() on URLs including a textual fallback for unparseable ones, collectSecrets deliberately also scrubbing the wtn.<id>.<secret> tail component, scrubSecrets applied to every byte the CLI and the MCP transport emit, ctx.protect() called on a freshly minted key before anything can interpolate it, and tests that plant a real key in a thrown error and assert it is absent from both stdout and stderr on both surfaces. The one hole is the file on disk (A-07), not the output paths.
• The MCP transport gets the two things that actually break MCP servers right: a real line framer with tests for coalesced and split chunks and CRLF, and a hard architectural separation so the only stdout write is one JSON-RPC line (server.ts:1053) with diagnostics on stderr. Responses are queued in arrival order while handlers overlap — a thoughtful touch. A malformed line cannot desynchronise the stream.
• Bounded loops where it counts. listAllRecords has a page cap, a record cap and a repeated-cursor guard. verifyRfc6962InclusionProof is provably ≤ ~53 iterations even with treeSize = MAX_SAFE_INTEGER and a million supplied hashes (verified: returns ok:false in 0 ms). blockAtBoundary's binary search is ≤ 53. OTS varuint parsing enforces MAX_VARUINT_BYTES and re-checks Number.isSafeInteger per group. The Rekor UUID fan-out is capped at 10. These are the cases where a careless implementation hangs.
• The honesty architecture is the best thing in the codebase and it is enforced in code, not prose: ruleVersion withheld rather than falsified when the working tree is dirty; assertCompliancePosture refusing to let unanchored Sealed-tier records reach a report; assertDisclaimerIntact refusing to render a weakened disclaimer; INSUFFICIENT_PROOF kept structurally distinct from BROKEN all the way out to distinct exit codes; bindingConfidence never upgraded downstream; a refusal to emit an empty report a reader would misread as "nothing happened". This is the part of the design that would survive an adversarial audit by opposing counsel, and it is why A-08 and A-12 matter — they are the two places where the implementation undercuts that architecture.
• error.retryable and the HTTP error taxonomy are preserved correctly through retry exhaustion, with a comment explaining the earlier bug. Good instinct: the fix and the reason are both in the code.

Prioritised remediation

Must fix before an enterprise pilot

1. A-01 — git config RCE. Add -c core.fsmonitor=false -c core.hooksPath=/dev/null -c core.pager=cat -c diff.external= -c protocol.allow=never to both git invocations, and stop passing the full process.env to the subprocess. Verified to block the PoC. Half a day, including a test that asserts the exact argv (which is missing anyway — A-21).
2. A-07 — credential file permissions. Rewrite writeSecretFile and writeJsonFile with O_CREAT|O_EXCL|O_NOFOLLOW and an explicit fchmod, create the state directory 0700, and add the three tests (fresh / pre-existing / symlink). This is the only live credential on disk and it currently has no test at all. Half a day.
3. A-03 — HTTP response-size cap. One change in httpRequest; blunts A-04, A-05 and A-06 simultaneously. Half a day.
4. A-04 and A-05 — the two confirmed process-killers. Cap decodeInteger content length (one line); add a total-bytes budget plus duplicate-offset rejection to decodeAbiParameters. One to two days. Move the default TSAs to HTTPS while you are in that file.
5. A-02 — SSRF guard. redirect: 'error' plus a private-IP/link-local denylist applied at both request time and config-validation time, with a documented allowPrivateEgress escape hatch. One to two days. Non-negotiable for a deployment where the process holds a cloud service identity.
6. A-08 — Markdown injection. Escape backticks; route ruleVersion and liability caveats through escapeCell; validate ruleVersion as 40-hex in normaliseEvidence. Half a day, and it protects the artefact that is the product's whole point.
7. A-06 — OTS fan-out. Branch counting, a MAX_BRANCHES_PER_NODE, a concurrency limiter, and a target cap. One day. Ship this before enabling the anchoring cadence against the public calendars, or you will be rate-limited and lose the anchoring that makes records compliance-grade.

Fix before the pilot handles a real liability pack

8. A-12 — stop reporting a canonicalisation failure as BROKEN, and add a depth cap to canonicalize. Small change, disproportionate importance: a false tampering allegation in an evidence tool is a reputational event.
9. A-13 — MCP authorisation. At minimum: default seal_governance_events to dryRun: true, gate the mutating tools behind an off-by-default env flag, and stop returning full liability packs in tool output. This needs a product decision, so start it early even though the code change is modest.
10. A-15 — require HTTPS whenever an API key is present. Trivial.
11. A-19 — cap run.attempts, add backoff and a circuit breaker, and surface abandoned runs as a problems[] entry rather than retrying forever.

Can wait, but schedule it

12. A-09 (firstHeading regex — a one-character fix, do it opportunistically), A-10 (lower MAX_CHUNK_ITERATIONS, add a per-call request budget), A-11 (memoise body hashes; add a chain-length cap), A-14 (declare the bundle schema, add maxItems), A-16 (extend FORBIDDEN_FILE_KEYS; add a depth and cycle guard to walk), A-17 (check chunk size before concatenating), A-18 (Object.hasOwn), A-20 (document --out semantics, or constrain it if the CLI is ever driven programmatically).
13. A-21 — close the test gaps, particularly src/cli/state.ts (currently zero coverage on the credential path), the git argv assertion, and the redirect policy. Several properties in this codebase are asserted only in a comment, and in the writeSecretFile case the comment turned out to be factually wrong — which is precisely why the test matters.
14. A-22 — align engines with --experimental-strip-types and pin the Node version in CI.

No action needed

A-23 — the supply chain is clean and org-policy compliant. Also confirmed clean and worth not re-auditing: the Rekor inclusion-proof verifier, the DER framing (as distinct from decodeInteger), OTS varuint and depth handling, the governor.ts bounds-checked word readers, encodeURIComponent coverage on every interpolated URL path segment, and the absence of eval, new Function or any dynamic import() of a computed path anywhere in src/.