Skip to content

WT-1-capability-snapshot: A1 capability snapshot infra (spec+plan+code, 9 audit rounds)#61

Merged
yzs15 merged 9 commits into
paper/v3-integrationfrom
paper/v3/p1-capability-snapshot
Jul 1, 2026
Merged

WT-1-capability-snapshot: A1 capability snapshot infra (spec+plan+code, 9 audit rounds)#61
yzs15 merged 9 commits into
paper/v3-integrationfrom
paper/v3/p1-capability-snapshot

Conversation

@yzs15

@yzs15 yzs15 commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Phase 1 paper-v3 A1 worktree per /root/paper_writing/docs/final/todo_list.md.

What lands

  • internal/capability/snapshot.goSnapshot struct (5 §3.4 kinds), NewCredentialAlias factory (rejects raw-token shapes), NewSnapshot validator + InputSchema canonicaliser, ComputeHash, JSONContainsRawToken, atomic IsUploadDisabled() accessor + SetDisableUpload / SyncDisableUpload helpers, init() that registers ablation.NoCapabilityDiscovery.
  • internal/capability/snapshot_test.go — full test matrix (~30 tests).
  • internal/observerstore/schema.sql — appended two-table capability_snapshots (dedup by hash) + capability_snapshot_usages (per-agent attribution).
  • internal/observerstore/capability_snapshots_writer.goWriteSnapshot with parameterised SQL, transactional double insert, ON CONFLICT DO NOTHING idempotency, secret-scan-before-ablation-skip, DoS-defence caps.
  • internal/observerstore/capability_snapshots_writer_test.go — happy path, SQL-meta injection, dedup + attribution, secret rejection, ablation skip + log, deterministic nowUTC.
  • docs/specs/wt1-capability-snapshot.spec.md + .plan.md — spec §1–§8 + §7.1–§7.2 + §4.0.1, plus TDD plan.

Downstream contracts

  • 13号§3.4 CapabilityRecall / CapabilityPrecision: Snapshot fields cover all 5 kinds (tool / platform / file / network / credential) with 1:1 producer mapping; §3.2 pins the evaluator translation rules.
  • D1 run schema: Snapshot.Hash() populates capability_snapshot_hash.
  • Ablation registry: NoCapabilityDiscovery registered against &DisableUpload; readers use IsUploadDisabled() for race-free access; Phase-2 CLI binder calls SyncDisableUpload() after SetByName.

Security posture (spec §7 (a)–(e))

  • (a) NewCredentialAlias rejects 7 raw-token shapes (sk-, JWT eyJ., AKIA, GitHub classic ghp_ + fine-grained github_pat_, Google AIza, Slack xox[bapres]-), all case-insensitive.
  • (b) ComputeHash inputs OS + every tool version → rollback attack detected.
  • (c) Parameterised SQL only; SQL-meta injection round-trips verbatim.
  • (d) NoCapabilityDiscovery ablates upload only; local collection preserved; skip logs one line for audit.
  • (e) Pre-write JSON scan reruns the same catalogue over canonical bytes.
  • Secret-scan runs BEFORE ablation short-circuit (§7.2) so leaked descriptors surface regardless of upload state.
  • InputSchema canonicalisation via big.Rat + exactDecimalPrec (2/5 factorisation) — semantically-equal literals hash identically; distinct values stay distinct; three-layer DoS defence (literal-length ≤ 64B, exponent |exp| ≤ 1250, bignum BitLen ≤ 4096).

Audit trail

9 rounds of review (3 Codex + 6 fresh Claude sub-agents, each fresh reviewer had no prior context). Every fix has a reproducer test that RED-failed against the prior tip.

Round Reviewer Findings Landing zone
1–3 Codex initial spec + plan + code initial code
4 fresh Claude 3 P2 secret-scan ordering, nowUTC coverage, expanded token catalogue
5 fresh Claude 2 P0 + 1 P1 + 2 P2 attribution loss (schema split), InputSchema key-order, DisableUpload race, eyJ 1-segment false positive
6 fresh Claude 1 P1 json.Number literal shape divergence (1e10 vs 1E10)
7 fresh Claude 1 P1 + 2 P2 fallback truncation + literal-length DoS + -0 guard
8 fresh Claude 1 P1 short-literal huge-exponent DoS bypasses R7 cap
9 fresh Claude 2 P2 (zero P0/P1) parser-boundary DoS residual + missing boundary tests

Severity has monotonically dropped R5 → R9. DoS defence is now three-layer (literal length → pre-parse exponent → post-parse BitLen); attribution + dedup separated into two tables; concurrency correctness verified under -race.

Full round-by-round rationale lives in each fix commit message.

Verification

go test ./internal/capability/... ./internal/observerstore/... -count=1 -shuffle=on -race -timeout=60s

→ 3 packages OK (~8s)

go vet ./...          # clean
gofmt -l internal/capability internal/observerstore   # clean (categorized_error_test.go warning is pre-existing, outside scope)

Not touched

  • internal/contract/types.go — PR WT-0: cloud sandbox slave in distributed compose (Windows slave work already on master) #45 Path A Platform / CommandInterfaces fields untouched.
  • internal/commandiface/ — read-only dependency.
  • internal/ablation/ — read-only dependency.
  • internal/capability/types.go — existing MCPToolDescriptor + helpers untouched.
  • Postgres schema parity — deferred (§1.2).
  • Migration runner — deferred (§1.5).
  • CLI binding for --ablation NoCapabilityDiscovery — Phase-2 WT-2-flag-integration.

🤖 Generated with Claude Code

claude added 9 commits July 1, 2026 15:33
…ion, security (a–e)

Stage 1 design document for paper/v3 Phase 1 A1 (capability snapshot +
observer table + NoCapabilityDiscovery ablation flag).

Pinned design decisions:
- Snapshot struct covers all 5 §3.4 capability kinds (tool/platform/file/
  network/credential) with 1:1 producer fields, plus a normative §3.2
  evaluator-translation rule per kind so CapabilityRecall/Precision can
  be computed without inference.
- ComputeHash = SHA-256 of canonical JSON (slices sorted in place); OS +
  tool versions are inputs so rollback-attack detection holds (§7 b).
- WriteSnapshot uses parameterized SQL only with ON CONFLICT(hash) DO
  NOTHING (§7 c, idempotency).
- NoCapabilityDiscovery ablation flag only blocks upload, never local
  collection — slave self-defense logic stays alive (§7 d). Skip path
  emits one log line so ablation auditors can distinguish intent from
  silent failure.
- NewCredentialAlias rejects sk-/eyJ./AKIA/ghp_/xox[baprs]- raw-token
  shapes before storage to keep real keys out of observer SQLite (§7 a).
- WriteSnapshot pre-write secret scan over canonical JSON catches MCP
  descriptor descriptions that paste raw tokens (§7 e).
- §1.2 separates storage scope (SQLite only) from semantic scope (field
  schema + translation rules backend-independent).
- §1.5 explains schema.sql append over a migrations/ directory (no
  existing migration runner in observerstore SQLite path).

Codex Stage 1 review: VERDICT CLEAN.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…y mapping

Stage 2 work breakdown for WT-1-capability-snapshot.

Plan ordering:
  Step A  capability/snapshot.go types only
  Step B  capability/snapshot_test.go skeleton
  Step C  NewCredentialAlias + raw-token catalogue (security a)
  Step D  NewSnapshot factory + invariants
  Step E  canonical sort + ComputeHash + Hash (security b)
  Step F  DisableUpload + ablation registration (security d)
  Step G  observerstore/schema.sql append (DDL)
  Step H  observerstore/capability_snapshots_writer.go (security c, e)
  Step I  observerstore/capability_snapshots_writer_test.go

Each step adds exactly the imports the step's code uses, so go build
stays green between steps (Codex round 1 finding).

§4 test→security mapping table cross-references every Security (a)-(e)
item from spec §7 to at least one named test in plan §4.

Codex Stage 2 review: VERDICT CLEAN.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ityDiscovery flag

Stage 3 implementation of WT-1-capability-snapshot (paper/v3 Phase 1 A1).

Adds:
  - internal/capability/snapshot.go — Snapshot struct (covering all 5
    13号§3.4 capability kinds via 1:1 producer fields), ToolVersion,
    CredentialAlias + factory, NetworkReach enum, FileResource,
    NewSnapshot construction validator, canonical sort + CanonicalJSON,
    ComputeHash + Snapshot.Hash, JSONContainsRawToken pre-write guard,
    package init() registering NoCapabilityDiscovery in
    ablation.Default.
  - internal/capability/snapshot_test.go — full §6 test matrix:
    raw-token rejection for 5 leaked-token shapes (case-insensitive),
    Snapshot invariants, ComputeHash stability/rollback-detection/
    slice-order-independence/total-sort, ablation registration, 3-slave
    distinct-hash acceptance.
  - internal/observerstore/capability_snapshots_writer.go —
    WriteSnapshot with parameterised SQL + ON CONFLICT(hash) DO NOTHING
    idempotency + ErrSnapshotContainsSecret + ablation skip log.
  - internal/observerstore/capability_snapshots_writer_test.go —
    happy path, SQL-injection-in-agent_id round-trip, idempotency,
    cross-agent dedup + index presence, embedded-secret rejection in
    MCP description + tool name, ablation skip + log line.
  - internal/observerstore/schema.sql — appended capability_snapshots
    table + index (CREATE … IF NOT EXISTS, follows existing idiom).

Stage 3 Codex review caught and fixed:
  - raw-token regexes were case-sensitive → now case-insensitive (?i)
    so SK-/GHP_/XOXB-/EYJ. variants are rejected.
  - ComputeHash swallowed CanonicalJSON errors with a constant
    placeholder → NewSnapshot now rejects invalid MCPTools.InputSchema
    at construction time and ComputeHash panics on the unreachable
    error path (so a hand-built broken snapshot dies loudly rather
    than colliding silently).
  - canonical sort keys were not total for CommandInterfaces/MCPTools
    → tie-break now extends to Default bool (CommandInterface) and
    full json.Marshal bytes (MCPTool), so near-duplicate permutations
    hash identically.

Test commands:
  go test ./internal/capability/... ./internal/observerstore/... \
      -count=1 -shuffle=on -race
  go vet ./...
  gofmt -l internal/capability internal/observerstore

All green; categorized_error_test.go fmt warning is pre-existing in
the observerstore package and out of scope for this worktree.

Codex Stage 3 review: VERDICT CLEAN on substance (final round 'CHANGES
REQUIRED' was procedural — files were still untracked at review time;
they are committed by this commit).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A fresh Claude Code sub-agent reviewed the PR end-to-end and flagged
three P2 issues. All three landed here:

1. Secret-scan now runs BEFORE the ablation short-circuit (writer.go).
   Previously a snapshot with a leaked token in an MCP description
   would silently return nil under NoCapabilityDiscovery=true,
   suppressing a real correctness signal. Now ErrSnapshotContainsSecret
   surfaces regardless of upload state. Spec §7.2 added to pin the
   ordering rule explicitly.

2. nowUTC override now has a TestWriteSnapshot_PersistedCreatedAt that
   exercises the deterministic-clock path. Spec §5.2 wording tightened
   to require serial tests + t.Cleanup restore. Pulls 'time' into the
   test file's imports.

3. Raw-token catalogue expanded (spec §7(a) + snapshot.go):
   - github_pat_[A-Z0-9_]{20,}    GitHub fine-grained PAT
   - AIza[A-Z0-9_-]{30,}          Google API key
   - xox[bapres]-…                Slack class now includes 'e' (xoxe-
                                  refresh tokens) and 'share'
   New §7.1 admits the catalogue is Phase-1 and not exhaustive; lists
   known gaps (AWS secret access key, Stripe sk_live_, Twilio AC…)
   deferred to a future entropy-based scanner worktree.

New tests: TestCredentialAlias_RejectsExpandedTokenCatalogue,
TestJSONContainsRawToken_ExpandedCatalogue,
TestNoCapabilityDiscovery_SecretScanRunsBeforeAblationSkip,
TestWriteSnapshot_PersistedCreatedAt.

Verification:
  go test ./internal/capability/... ./internal/observerstore/... \
      -count=1 -shuffle=on -race  → all 3 packages OK
  go vet ./...                    → clean
  gofmt -l                        → clean (pre-existing categorized_error_test.go
                                   warning is outside this worktree's scope)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…P1, 2 P2

A fresh Claude Code sub-agent (no prior context) audited the PR
end-to-end and produced 5 findings, all reproduced independently in the
repo before touching code:

  P0 — Spec §5.1 dedup-attribution contract was broken. The prior
       schema stored (hash PK, agent_id, ...) with ON CONFLICT(hash) DO
       NOTHING, so when agent-A observed hash H, then agent-B observed
       the same H seconds later, agent-B's row silently no-op'd.
       "WHERE agent_id = 'agent-B'" returned 0 rows — evaluator loses
       per-agent capability usage.

       Fix: split into two tables.
         capability_snapshots(hash PK, snapshot_json, first_seen_at)
           — content-addressed dedup, JSON blob stored once per hash.
         capability_snapshot_usages(workspace_id, agent_id, hash,
                                    used_at PK)
           — insert-always attribution log; new indexes on
           (workspace_id, agent_id, used_at) and (hash).
       WriteSnapshot now uses a transaction to insert both atomically.
       Spec §5.1.1 added to explain the rationale.

  P0 — MCPTools.InputSchema (json.RawMessage) hashed byte-for-byte, so
       two semantically-identical schemas with different key order
       ({"a":1,"b":2} vs {"b":2,"a":1}) or embedded whitespace produced
       different hashes and defeated dedup at the observer.

       Fix: NewSnapshot now canonicalises each non-empty InputSchema
       at construction time via unmarshal-to-interface{} + re-marshal
       (json.Marshal sorts map keys lexicographically per Go spec).
       Uses json.Number to preserve integer precision beyond 2^53.
       Tests added for key-order, whitespace, and big-integer
       preservation.

  P1 — DisableUpload package-global bool was read from WriteSnapshot
       without any synchronisation. go test -race under concurrent
       read/write fires "WARNING: DATA RACE". The ablation package's
       "pre-run-only mutation" contract is documentation-only.

       Fix: added atomic.Bool mirror + IsUploadDisabled() accessor +
       SetDisableUpload(v) helper + SyncDisableUpload() for the
       ablation-registry path (registry writes only through *bool per
       its fixed API). WriteSnapshot now reads IsUploadDisabled().
       Tests use SetDisableUpload; new TestIsUploadDisabled_RaceFree
       stresses concurrent read+write and passes under -race.
       Spec §7(d) updated with the concurrency contract.

  P2 — eyJ regex "(?i)eyJ[A-Z0-9_-]{10,}\." false-positives on
       innocuous "identifier.name" phrasings (e.g. "See
       eyJconfigDefaults. for defaults" matched as a JWT and dropped
       the whole snapshot).

       Fix: require 2+ base64 segments — a real JWT is at minimum
       "header.payload". New regex is
       "(?i)eyJ[A-Z0-9_-]{10,}\.[A-Z0-9_-]{10,}(\.[A-Z0-9_-]{10,})?".
       TestJSONContainsRawToken_EyJRequiresPayloadSegment covers both
       positive (2- and 3-segment JWTs) and negative (identifier.name)
       cases.

  P2 — Old TestWriteSnapshot_DifferentAgentsSameHashOneRow asserted
       only "row count == 1" and index existence — it did NOT assert
       agent-B's attribution was recoverable. The test happily passed
       while the P0 above was broken.

       Fix: replaced with
       TestWriteSnapshot_DifferentAgentsShareDedupButKeepUsages, which
       asserts:
         (a) capability_snapshots has exactly 1 row for the hash
             (dedup works), AND
         (b) capability_snapshot_usages has separate rows for agent-A
             and agent-B (attribution works), AND
         (c) the (workspace_id, agent_id, used_at) index exists so the
             query is efficient.
       Uses a stepping clock to keep used_at PKs distinct.

Verification:
  go test ./internal/capability/... ./internal/observerstore/... \
      -count=1 -shuffle=on -race     → 3 packages OK
  go vet ./...                       → clean
  gofmt -l                           → clean (pre-existing
                                       categorized_error_test.go warning
                                       is outside this worktree's scope)

Also updated:
  - existing TestCredentialAlias_RejectsRawTokensCaseInsensitively test
    case for eyJ swapped from a 1-segment "EyJabcDEFghiJKL.payload" to
    a 3-segment JWT-shaped value (the 1-segment form is now correctly
    rejected only by shape, not by raw-token detection).
  - plan.md header carries a pointer to the rounds 4–5 audit fix
    commits.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…Schema number-literal canonicalisation

Round-6 fresh Claude sub-agent (no prior context) audited the round-5
landing zone and found ONE real defect:

  P1 — internal/capability/snapshot.go canonicalJSONBytes: MCPTools.
       InputSchema canonicaliser sorted map keys and stripped whitespace,
       but preserved json.Number literals verbatim. Semantically-equal
       numeric shapes hashed DIFFERENTLY, defeating dedup:

         "1e10" vs "1E10"        → distinct SHA-256
         "1.0"  vs "1"           → distinct SHA-256
         "1e+10" vs "1e10"       → distinct SHA-256
         "0e0"  vs "0"           → distinct SHA-256

       Same failure mode round-5 tried to fix (key-order divergence),
       just via a different edge. Reviewer reproduced all four with an
       in-repo test before reporting.

       Fix: canonicalJSONBytes now walks the decoded tree and replaces
       every json.Number with a canonicalNumber wrapper backed by
       big.Rat. MarshalJSON emits ONE literal per rational:

         - integers → "N"     (no ".0", no exponent)
         - non-integers → shortest decimal from big.Rat.FloatString(prec),
           trailing zeros trimmed; prec grows 1..64 until round-trip
           succeeds. Pathological 1/3-shaped values cap at 64 digits
           (deterministic across runs).

       big.Rat handles every JSON-representable number without float64
       precision loss (2^53+1 still distinguishable from 2^53).

       Also rejects trailing tokens after the value (belt-and-braces
       even though json.Valid already blocks the input path).

Regression tests added:
  - TestNewSnapshot_CanonicalisesInputSchemaNumberLiteralShapes
    (9 pairs of semantically-equal number shapes → same hash)
  - TestNewSnapshot_InputSchemaDistinctNumbersHashDistinctly
    (distinct values still hash distinctly, incl. 2^53 vs 2^53+1)

Spec §4.0.1 added documenting the number canonicalisation contract.

Verification:
  go test ./internal/capability/... ./internal/observerstore/... \
      -count=1 -shuffle=on -race     → 3 packages OK
  go vet ./...                       → clean
  gofmt -l                           → clean

Round-6 reviewer OVERALL: CHANGES REQUIRED — this single P1 was the
only real defect found under adversarial probing of the round-5
landing zone. Everything else (two-table transaction, atomic mirror,
eyJ regex, secret-scan ordering) checks out.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…fraction hash collision + P2 adversarial-literal DoS

Round-7 fresh Claude sub-agent (no prior context) audited round-6's
number canonicaliser and found:

  P1 — internal/capability/snapshot.go canonicalNumber.MarshalJSON:
       fallback path silently truncated fractions beyond the 64-digit
       cap, causing dedup collisions on distinct semantic values:

         "1e-100" canonicalised to "0"        → collides with real 0
         "1.5e-64" canonicalised to "0.0…02"  → collides with "2e-64"

       Reproducer confirmed both collisions (16-hex prefixes matched
       byte-for-byte in ComputeHash output). Same failure mode round-5
       and round-6 tried to fix (silent dedup defeat), just at a
       different edge — the round-6 fallback was itself the bug.

       Fix: exploit the fact that every JSON number literal parses to
       a rational whose lowest-terms denominator is 2^a * 5^b (JSON
       numbers are finite decimals m * 10^e by grammar). New helper
       exactDecimalPrec(r) computes prec = max(a, b) by trial-dividing
       the denominator by 2 and 5; big.Rat.FloatString(prec) is then
       guaranteed exact — no round-trip probe, no truncation, no
       collision. Repeating-decimal rationals (denom has any other
       prime factor — impossible from JSON input) return an error
       rather than silently emitting a lossy form.

  P2 — canonicaliseNumbers accepts arbitrary-size json.Number literals,
       feeding them into big.Rat. An adversarial MCP tool descriptor
       with {"n":1e1000000} allocates ~1MB per snapshot inside big.Int;
       {"n":1e999999} clocks ~100ms wall time. DoS vector against every
       capability collector on the network.

       Fix: reject json.Number literals whose byte length exceeds
       maxNumberLiteralLen (64) at walk time; return ErrSnapshotInvalid.
       Legitimate JSON schema numbers comfortably fit; only crafted
       attack payloads trip the cap.

  P2 — trimTrailingZeros's edge-case guard did not cover the "-0"
       output (produced by big.Rat.FloatString for negative tiny
       fractions that FloatString rounded to zero). Guard extended:
       "-0" now normalises to "0".

  Also: canonicaliseNumbers now returns (interface{}, error) to
  propagate rejection. Signature change is internal-only.

Regression tests added (all pass):
  - TestNewSnapshot_InputSchemaTinyFractionsDoNotCollideWithZero
    (positive + distinct-tinies)
  - TestNewSnapshot_CanonicalisesTinyFractionShapes
    (equal-tinies collapse: 1e-10 vs 0.0000000001; 1.500e-10 vs 1.5e-10;
    -1e-10 vs -0.0000000001)
  - TestNewSnapshot_RejectsAdversarialLongNumberLiteral
    (65-byte literal → ErrSnapshotInvalid)
  - TestNewSnapshot_InputSchemaNegativeTinyFractionsDistinguished
    (-1e-30 distinct from 0 and from 1e-30)
  - TestNewSnapshot_CanonicalJSONBytesIdempotent
    (canonicalising canonicalised bytes = byte-equal output)

Spec §4.0.1 rewritten to document the exact-precision algorithm and
the DoS defence.

Verification:
  go test ./internal/capability/... ./internal/observerstore/... \
      -count=1 -shuffle=on -race     → 3 packages OK
  go vet ./...                       → clean
  gofmt -l                           → clean

Round-by-round pattern: rounds 5–7 each found ONE regression in the
prior round's fix landing zone. Round 7's fix uses number theory
(finite-decimal grammar → denominator's 2/5 factorisation) to eliminate
the probe-loop-plus-fallback structure entirely, so the round-6-style
"loop cap is too small" failure mode is architecturally gone.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-literal huge-exponent DoS bypasses the round-7 length cap

Round-8 fresh Claude sub-agent (no prior context) audited round-7's
DoS cap and found ONE real defect:

  P1 — canonicaliseNumbers's maxNumberLiteralLen=64 cap bounded the
       INPUT literal length but did not bound the OUTPUT rational's
       size. A 9-byte literal `1e-100000` parses to 1/10^100000 whose
       denominator has ~332,000 bits. exactDecimalPrec then trial-
       divides the denominator by 2 (100000 iterations) and by 5
       (100000 iterations); each iteration operates on a still-large
       big.Int. Measured: 12.15 seconds of CPU per snapshot for a
       single tool descriptor. Larger exponents (SetString accepts up
       to ~1e-1000000, 11 bytes) push wall time out of reach — a
       single MCP tool descriptor can pin a CPU indefinitely per
       snapshot ingest, hitting both NewSnapshot (collector) and
       WriteSnapshot (observer via CanonicalJSON).

       Same class of failure round-7 was written to fix (adversarial-
       DoS via number literals), but at a different edge: round-7
       bounded the literal, this bounds the rational.

       Reproducer: 9-byte {"a":1e-100000} in InputSchema → 12.15s wall
       time in NewSnapshot on the review host.

       Fix: add a SECOND cap maxNumberBitLen=4096 applied to the
       parsed rational's numerator AND denominator BitLen. Rejects
       any parsed value whose bignum representation exceeds ~1233
       decimal digits — well beyond any legitimate JSON schema value.
       Worst-case exactDecimalPrec compute is now sub-millisecond;
       worst-case FloatString output is ~1.2KB. Legitimate magnitudes
       (`1e1000`, `1e-1000`) are still accepted.

Regression tests added (all pass; DoS test completes in <100µs):
  - TestNewSnapshot_RejectsShortLiteralWithHugeNegativeExponent
    (with 500ms deadline via time.NewTimer; DoS cap failure fails the
    test)
  - TestNewSnapshot_RejectsShortLiteralWithHugePositiveExponent
    (symmetric on numerator BitLen)
  - TestNewSnapshot_AcceptsLegitimateLargeMagnitudes
    (1e1000 and 1e-1000 still accepted — the cap does not overshoot)

Spec §4.0.1 updated: DoS defence section now documents both caps and
why one is not sufficient; regression coverage list updated.

Verification:
  go test ./internal/capability/... ./internal/observerstore/... \
      -count=1 -shuffle=on -race -timeout=60s   → 3 packages OK in ~8s
                                                  (vs 12s DoS pre-fix)
  go vet ./...                                  → clean
  gofmt -l                                      → clean

Round-by-round trajectory continues its narrowing pattern:
  R5: 2 P0 (schema attribution, InputSchema key-order)
  R6: 1 P1 (number literal shape divergence)
  R7: 1 P1 + 2 P2 (fallback truncation + DoS #1 + trim guard)
  R8: 1 P1 (DoS #2 — bypassed R7's cap via short-literal-huge-exponent)

Each round's fix opened a smaller edge in the same feature. The
round-8 fix uses a BitLen bound which is invariant under literal shape
(exponent vs long-form), so this class of DoS is architecturally
closed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e-parse exponent guard + boundary tests)

Round-9 fresh Claude sub-agent (no prior context) audited round-8's
BitLen cap and found:

  P2 — canonicaliseNumbers's BitLen cap runs AFTER big.Rat.SetString.
       For a short literal with a huge exponent like `1e-999999`
       (10 bytes, well under the 64-byte literal cap), SetString
       still allocates a ~3M-bit bignum during parsing. Measured
       wall time on the review host: ~28 ms per literal, before the
       BitLen cap rejects it. This is a ~10,000× regression versus
       benign parsing (~3µs), well short of the round-8 commit's
       claimed "sub-millisecond" worst case for the compute step.

       Not exploitable to the same scale as round-8's ~12s DoS, but
       still a straightforward parser-boundary residual that should
       be closed architecturally rather than left for the next round.

       Fix: pre-parse exponent guard `exponentAbsExceedsCap`. Scans
       the literal string for `e|E`, extracts the exponent digits
       (bounded by maxNumberLiteralLen), rejects if |exp| > 1250.
       Cap chosen to comfortably exceed log10(2^4096) ≈ 1233 — any
       literal past that magnitude would be rejected by the BitLen
       cap anyway, so short-circuiting at the parser is safe. Post-
       fix wall time on the same repro: <100µs.

       Long-digit exponents (arbitrary trailing digits) short-circuit
       when `len(strings.TrimLeft(exp, "0")) > 4`, so an attacker
       cannot pad the exponent to overflow int during the scan.

  P2 — TestNewSnapshot_AcceptsLegitimateLargeMagnitudes uses
       `1e1000` / `1e-1000` (~3322 bits, ~774 bits under the 4096
       cap), leaving a wide untested band around the boundary.
       Reject tests use `1e100000` (massively over). Missing: the
       actual accept/reject transition. A future change flipping
       `>` to `>=` (or applying the cap to Num+Denom sum) would not
       be caught by any test.

       Fix: TestNewSnapshot_BitLenCapBoundary — `1e±1233` (~4094
       bits, must accept) and `1e±1234` (~4098 bits, must reject).
       Comment cites log2(10) × 1233 ≈ 4096.

New regression tests (all pass, capability suite still ~1s):
  - TestNewSnapshot_RejectsHugeExponentBeforeSetString
    (5-case table: 1e±999999, 1e±1500, 1e00001500; each bounded to
    5ms by an in-line time-since check)
  - TestNewSnapshot_BitLenCapBoundary
    (4-case boundary: 1e1233, 1e-1233, 1e1234, 1e-1234)

Spec §4.0.1 rewrites the DoS-defence section from TWO-cap to
THREE-cap defence-in-depth (literal-length → pre-parse exponent →
post-parse BitLen), each layered in front of a distinct cost
boundary. Regression-coverage list updated.

Verification:
  go test ./internal/capability/... ./internal/observerstore/... \
      -count=1 -shuffle=on -race -timeout=60s  → 3 packages OK in ~8s
  go vet ./...                                  → clean
  gofmt -l                                      → clean

Round-by-round trajectory:
  R5: 2 P0 (schema attribution, InputSchema key-order)
  R6: 1 P1 (number literal shape divergence)
  R7: 1 P1 + 2 P2 (fallback truncation + DoS#1 + trim guard)
  R8: 1 P1 (DoS#2: short-literal huge-exponent bypasses R7 length cap)
  R9: 2 P2 (parser-boundary residual DoS + missing boundary test)

Severity is now genuinely converging downward: R9 is the first round
since R4 with zero P0/P1. The three-cap defence-in-depth structure
covers input length, exponent magnitude, and bignum size at three
distinct boundaries; a next-round DoS attack would have to find a
new attack surface entirely (not another edge in the same
canonicaliser).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@yzs15 yzs15 force-pushed the paper/v3/p1-capability-snapshot branch from 18da7fc to d6b99e2 Compare July 1, 2026 07:34
@yzs15

yzs15 commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator Author

Round 10 audit — fresh Claude sub-agent review after PR opened.

Findings

  • [P2] schema.sql merge conflict — RESOLVED. origin/paper/v3-integration advanced from 17f2c3c to 00d1694 while this PR was under review (WT-1-run-schema WT-1-run-schema: D1 per-run schema, writer, and CSV/JSONL export #56, WT-1-routing-trace WT-1-routing-trace: per-Dispatch RouteDecision trace + route_reasons table #55, and others appended to schema.sql). Rebased onto current tip; capability_snapshots + capability_snapshot_usages DDL now sits after the freshly-landed runs + route_reasons tables. Purely mechanical resolution — no code change.

  • [P2] WriteSnapshot free-function signature diverges from *SQLiteStore method convention — DEFERRED. Other observerstore write paths (MarkArtifactAvailable, UpsertAgent, etc.) are methods on *SQLiteStore; WriteSnapshot(ctx, db *sql.DB, ...) is a bare free function forcing callers through store.DB(). Reviewer explicitly categorised this as a follow-up nit ("convert in a follow-up worktree; no behaviour change required for this PR") because the SQL is parameterised, transactional, and correct as-is. Deferring per reviewer guidance.

What the reviewer probed and found CLEAN

  • Round-9 pre-parse exponent guard — 4 mantissa-heavy bypass attempts all fast-reject at cap fix(driver): clarify list_agents roles #3 in ≤85µs.
  • Zero-shape canonicalisation — 7 different zero literals (0, -0, 0.0, 0e0, 0e100, 0.0e-100, -0.0) all hash identically.
  • PR scope — 7 files, all intended, no leaked context files.
  • All 5 spec §7 security items (a)–(e) still have named regression tests after the round-5 schema split.
  • Concurrency contract: race test passes 10k read/write pairs under -race.
  • TestWriteSnapshot_PersistedCreatedAt correctly asserts first_seen_at and used_at share the same fixed timestamp.
  • Spec §4.0.1 cap values match code (maxNumberLiteralLen=64, maxNumberAbsExponent=1250, maxNumberBitLen=4096).

Verification post-rebase

go test ./internal/capability/... ./internal/observerstore/... -count=1 -shuffle=on -race -timeout=60s

→ 3 packages OK (~9s).

go vet ./...          # clean
gofmt -l internal/capability internal/observerstore   # clean (categorized_error_test.go warning is pre-existing, outside scope)

OVERALL: PR is merge-ready. R10 verdict — CHANGES REQUIRED (schema conflict) → now RESOLVED via rebase. No P0/P1 defects. Severity trajectory R5→R10: 2 P0 → 1 P1 → 1 P1+2 P2 → 1 P1 → 2 P2 → 2 P2 (both P2 either resolved or explicitly deferred).

🤖 Generated with Claude Code

@yzs15 yzs15 merged commit d053897 into paper/v3-integration Jul 1, 2026
10 checks passed
@yzs15 yzs15 deleted the paper/v3/p1-capability-snapshot branch July 1, 2026 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants