Skip to content

test(runtime-logs): pin cursor codec rules surviving mutants - #2176

Merged
freeqaz merged 3 commits into
mainfrom
mutant-sweep-1011b
Oct 11, 2026
Merged

freeqaz merged 3 commits into
mainfrom
mutant-sweep-1011b

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Problem

packages/occ/src/runtime-logs/cursor.ts signs runtime log cursors and binds them to one principal, Agent, revision and source. It also validates the signed position and the kubelet frontier times the reader orders by. A mutation sweep found that the existing suites killed only 11 of 51 mutants of these rules. The suites were runtime-logs-content, runtime-logs-sandbox, runtime-logs-replay, occ-api-security and http-errors. For example, a cursor that ignored the Agent, revision or source binding still passed every test, as did a cursor carrying a v2. prefix or an extra segment, or one issued an hour in the future. A truncated MAC (where timingSafeEqual would throw) also passed, and so did a Driver time of 2026-02-30T12:00:00Z. The product code is correct; the tests did not pin it.

Change

Test only. tests/conformance/runtime-logs-content.test.mjs gets a small codec fixture and four cases, placed before the PEM route-polling case:

  • binding and envelope: a cursor is refused for another Agent, revision or source, and when text moves across the field delimiter. It is also refused with an extra part, a non-v1 prefix or a short MAC; the short MAC returns invalid instead of throwing. Expiry starts strictly after the TTL, and up to 60 s of clock skew is tolerated (60.001 s is refused).
  • codec limits: encoding keeps the newest 48 frontier hashes. Line hashes are exact, so trailing whitespace changes the hash. A cursor secret needs at least 32 characters.
  • signed schema: the unchanged fixture decodes as valid, and each of 11 malformed position fields, signed with the real secret, decodes as invalid. This follows the existing PEM-pair case.
  • frontier times: validRuntimeLogFrontierTime accepts kubelet UTC only. It refuses 10-digit fractions, +00:00 offsets, extended years, Feb 30 and month 13. compareRuntimeLogTime treats a trimmed fraction as equal to its zero-padded form and orders .5 after .123.

Verification

  • Mutation sweep: 51 mutants of cursor.ts, run one at a time in a separate worktree.
    • Before: the five suites killed 11.
    • After: the content file alone kills 46. T1 (the TTL widened to a day) is still killed only by occ-api-security's two-hour expiry case, so 47 of 51 are killed overall.
    • Each of the 36 newly killed mutants is killed by one of the new cases, and every new case passes on unchanged product code.
  • 4 mutants survive:
    • Equivalent: P1, the up-front array refusal. An array has no v, so the next check refuses it.
    • Equivalent: P15, the lastTime check inside the PEM pair. A valid pemAfterTime whose time key equals lastTime's key forces lastTime to be valid.
    • Unreachable without the secret (defence in depth): M1, the MAC purpose prefix. To exploit it, an attacker would need another auth-secret signer's MAC over a string that also decodes to a cursor carrying the right binding hash. The other auth-secret signers MAC strings that start with their own fixed purpose label.
    • Unreachable without the secret (defence in depth): P6, the decode-side 48-hash cap. encode never signs more than 48 hashes.
    • Pinning M1 or P6 would mean re-implementing the HMAC in the test.
  • runtime-logs-content.test.mjs: 71/71 tests. Two sets of 20/20 full-file runs, each pinned with taskset -c 2,3: one on c023521 and one on 8a3618e, at 4.9-6.8 s.
  • prettier --check (shown to fail on a misformatted copy) and eslint are clean on the file.
  • An opus review returned LAND WITH NITS. All nits are applied, including the case split, the positive control and these body corrections.

Risks

Test only; no product change. The boundary values (60 s skew, 48 hashes, 32-character secret, and the TTL edge via the exported constant) are pinned exactly, so changing one of them deliberately needs a test update. The cases sit away from the lines open PR #1981 edits in this file; a trial merge-tree with its head merges the file cleanly.

A mutation sweep of packages/occ/src/runtime-logs/cursor.ts left 40 of 51
mutants alive against the runtime log, API security and HTTP error suites.
Three codec-level cases kill the 36 that are real gaps: binding fields
(agent, revision, source, delimiters), the v1 envelope and short MACs,
expiry and clock-skew boundaries, the 48-hash cap, the secret length, signed
schema validation of position fields, and the kubelet time validator and
fraction ordering. The other four are equivalent. Test only.
@freeqaz
freeqaz requested a review from a team as a code owner October 11, 2026 13:11
@clawsweeper

clawsweeper Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Oct 11, 2026
@clawsweeper

clawsweeper Bot commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

This PR adds three conformance cases covering runtime-log cursor bindings, signed position validation, and timestamp rules.

Example: A cursor issued for agent-test is decoded for agent-other.

  • Before: Production rejects it, but the existing tests did not explicitly protect the Agent binding.
  • After: A focused assertion fails if decoding accepts the other Agent.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Useful, focused test hardening with no identified defect; execution confidence rests on reported unit and mutation results rather than independently inspected run output.
Proof confidence 🦐 gold shrimp (3/6) Only conformance tests change; no shipped runtime behavior requires new live proof, and reported test execution remains distinct from independently observed results.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Test only · Worth it: Yes · Fix scope: Complete
User problem: Regressions could weaken cursor isolation or resume-time validation without failing the existing suites.
Reason: The tests protect meaningful existing contracts with real production functions and leave runtime behavior unchanged.

Merge readiness

✅ Ready for maintainer review

Keep this PR open: it adds useful coverage for existing cursor security and ordering contracts, with no concrete introduced defect found.

Priority: P3
Reviewed head: 17ee86463ee16c8f5eb3e14a49204b8d18e9528d

Before merge

None.

Findings

None.

Agent review details

How this fits together

OCC's runtime-log readers validate signed cursors before Driver log reads; the cursors carry identity-bound resume positions and bounded frontier state.

flowchart LR
  A[Runtime log request] --> B[OCC authorization]
  B --> C[Cursor validation]
  C --> D[Driver log read]
  D --> E[Sanitized page and cursor]
  F[New conformance cases] --> C
Loading

Technical review

Best possible solution:

Retain the focused codec coverage alongside the existing reader and route tests.

Do we have a high-confidence way to reproduce the issue?

No production bug is claimed; source inspection confirms that the new assertions target existing behavior, while the reported mutation results were not independently executed.

Is this the best way to solve the issue?

Direct codec tests are appropriate for these signed-format and identity invariants, and they introduce no test-only production seam.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 220cc9572400.

Testing

Proof path: unit tests only.

Security

None.

Evidence

What I checked:

  • Pinned test-only introduction: The introduced diff adds a codec fixture and three cases without changing production code or existing assertions. (tests/conformance/runtime-logs-content.test.mjs:1831, 17ee86463ee1)
  • Existing production contract: The codec already enforces delimited identity bindings, exact v1 envelopes, MAC length safety, signed schema validation, expiry, and future-clock limits; timestamp validation preserves kubelet precision. (packages/occ/src/runtime-logs/cursor.ts:64, 220cc9572400)
  • Real production callers: OCC constructs the binding from the requesting principal, Agent, revision, and source; the container reader rejects invalid cursors before invoking its Driver read. (packages/occ/src/index.ts:2862, 220cc9572400)
  • Discussion and reported validation: The full PR body reports 70 passing tests and mutation coverage improving from 11 to 47 of 51 mutants; these execution claims were not independently rerun. Both discussion comments are bot status messages. The supplied body identity is a09301875b17b7d35b13ecbcb4b192a4e7114498b86c3b4af1adbdfe7d4aed5a. (17ee86463ee1)
  • Adjacent work preserves a distinct contribution: fix(logs): bound complete runtime log responses #1981 owns response-byte bounds and polling progress, whereas this PR protects existing codec contracts.
  • Policy and inspection limits: The complete root AGENTS.md was read; no nested tests instructions or maintainer-notes directory were present. Local follow/blame encountered unavailable historical objects, so commit patches from GitHub supplemented the ownership hunt without establishing the original codec introduction. No tests or review helper ran, and the checkout remained clean. (AGENTS.md:81, 17ee86463ee1)

Review metrics

Metric Value Why it matters
Production versus test changes Production: 0; tests: +110/-1 lines Growth is confined to three focused contract cases and their fixture.

Labels

Label changes:

  • add P3: This is test hardening for behavior already implemented correctly, with no demonstrated current user failure.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

  • P3: This is test hardening for behavior already implemented correctly, with no demonstrated current user failure.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only; repair and merge need explicit maintainer commands such as @clawsweeper autofix or @clawsweeper automerge.

Reviewed October 11, 2026, 9:13 AM ET / 13:13 UTC.

Review nits: the hash cap, exact line hash and secret length get their own
case title, and the malformed-fields case first checks that its unchanged
fixture position decodes as valid.
@freeqaz
freeqaz deployed to integration-qa-pr October 11, 2026 13:25 — with GitHub Actions Active
@freeqaz
freeqaz merged commit 77f8ebd into main Oct 11, 2026
24 of 27 checks passed
@freeqaz
freeqaz deleted the mutant-sweep-1011b branch October 11, 2026 13:28

This branch was successfully deployed

1 active deployment
integration-qa-pr — 8a3618e7 Deployed Oct 11, 2026 by freeqaz via QA advisory (kubernetes) #338
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant