fix(#707): scope session GC retention #720

Merged

jason.woltje merged 3 commits from fix/tess-session-gc-scope into main 2026-07-13 00:44:20 +00:00

Conversation 2 Commits 3 Files Changed 10

+163 -175

jason.woltje commented 2026-07-12 21:35:20 +00:00

Owner

Copy Link

Copy Source

Refs #707

Task: TESS-M1-SEC-006

Scopes session cleanup and log retention to a single session, disables global GC paths, and retires legacy global schedules.

Refs #707 Task: TESS-M1-SEC-006 Scopes session cleanup and log retention to a single session, disables global GC paths, and retires legacy global schedules.

jason.woltje added 1 commit 2026-07-12 21:35:20 +00:00

fix(#707): scope session GC retention 5ad29926d9

jason.woltje force-pushed fix/tess-session-gc-scope from 5ad29926d9 to f17e70c145 2026-07-12 23:21:35 +00:00 Compare

jason.woltje added 1 commit 2026-07-12 23:48:40 +00:00

test(#707): preserve approval boundary during session GC a4b6665bd7

jason.woltje commented 2026-07-13 00:17:15 +00:00

Author

Owner

Copy Link

Copy Source

REQUEST CHANGES reviewer-of-record [W-jarvis:reviewer] head a4b6665bd7

CI is green on exact head (ci/woodpecker/pr/ci pipeline 1713), and the direction is right: global /gc, cold-start full GC, and scheduled global GC are disabled; log demotion now uses promoteSessionToWarm(sessionId, cutoff); SEC-001 command approval tests are present and pass in CI.

Blockers before I can attest SEC-006:

1. SessionGCService.collect() still builds a Redis SCAN MATCH glob from the raw session id: mosaic:session:${sessionId}:*. WebSocket ingress accepts any string conversationId, and that value can become the session id. A session id containing Redis glob metacharacters (for example *) broadens the scan/delete outside the single session namespace, violating the per-session retention boundary. Please escape/canonicalize session ids before using them in Redis glob patterns, or avoid glob matching for session cleanup, and add a regression test with glob metacharacters proving only the requested session's keys are deleted.

2. The requested durable-approval regression is not proven yet. The current test asserts the scan pattern is mosaic:session:abc:* and that del was not called with a command-approval string, but it does not create a valid SEC-001 approval, run a GC pass, and prove the approval still authorizes afterward. Please add that regression so a valid tess:command-approval:<id> survives session GC.

Once those are covered, I can re-review the new head.

REQUEST CHANGES reviewer-of-record [W-jarvis:reviewer] head a4b6665bd76a23b75fbebee5535a7cc1a420dd9f CI is green on exact head (`ci/woodpecker/pr/ci` pipeline 1713), and the direction is right: global `/gc`, cold-start full GC, and scheduled global GC are disabled; log demotion now uses `promoteSessionToWarm(sessionId, cutoff)`; SEC-001 command approval tests are present and pass in CI. Blockers before I can attest SEC-006: 1. `SessionGCService.collect()` still builds a Redis `SCAN MATCH` glob from the raw session id: `mosaic:session:${sessionId}:*`. WebSocket ingress accepts any string `conversationId`, and that value can become the session id. A session id containing Redis glob metacharacters (for example `*`) broadens the scan/delete outside the single session namespace, violating the per-session retention boundary. Please escape/canonicalize session ids before using them in Redis glob patterns, or avoid glob matching for session cleanup, and add a regression test with glob metacharacters proving only the requested session's keys are deleted. 2. The requested durable-approval regression is not proven yet. The current test asserts the scan pattern is `mosaic:session:abc:*` and that `del` was not called with a `command-approval` string, but it does not create a valid SEC-001 approval, run a GC pass, and prove the approval still authorizes afterward. Please add that regression so a valid `tess:command-approval:<id>` survives session GC. Once those are covered, I can re-review the new head.

jason.woltje added 1 commit 2026-07-13 00:20:53 +00:00

fix(#707): escape session GC scan scope 37be090e7b

jason.woltje commented 2026-07-13 00:31:02 +00:00

Author

Owner

Copy Link

Copy Source

VERIFIED APPROVE reviewer-of-record [W-jarvis:reviewer] head 37be090e7b

Evidence:

• Exact PR/branch head verified: 37be090e7bcbe2666535f1b3f2a53658535d599e.
• PR CI green: ci/woodpecker/pr/ci pipeline 1715 success.
• Re-reviewed remediation for prior blockers: SessionGCService.collect() now escapes Redis glob metacharacters before building the SCAN MATCH pattern, with a regression for abc*?[tenant]\\escape; this would fail if the raw session id were used.
• Durable approval survival is covered by creating a real CommandAuthorizationService approval, running a session GC pass, confirming the tess:command-approval:<id> entry remains, then authorizing with that approval successfully.
• SEC-001 abuse coverage remains: valid durable approval is still required/consumed, but /gc no longer executes an unaudited global sweep; it returns disabled and does not call sweepOrphans.
• SEC-006 retention boundary remains scoped: session GC deletes only the requested session namespace, demotes logs through promoteSessionToWarm(sessionId, cutoff), exposes no fullCollect/sweepOrphans, and startup removes legacy repeatable global GC schedules.
• git diff --check clean; no secrets in diff.

VERIFIED APPROVE reviewer-of-record [W-jarvis:reviewer] head 37be090e7bcbe2666535f1b3f2a53658535d599e Evidence: - Exact PR/branch head verified: `37be090e7bcbe2666535f1b3f2a53658535d599e`. - PR CI green: `ci/woodpecker/pr/ci` pipeline 1715 success. - Re-reviewed remediation for prior blockers: `SessionGCService.collect()` now escapes Redis glob metacharacters before building the `SCAN MATCH` pattern, with a regression for `abc*?[tenant]\\escape`; this would fail if the raw session id were used. - Durable approval survival is covered by creating a real `CommandAuthorizationService` approval, running a session GC pass, confirming the `tess:command-approval:<id>` entry remains, then authorizing with that approval successfully. - SEC-001 abuse coverage remains: valid durable approval is still required/consumed, but `/gc` no longer executes an unaudited global sweep; it returns disabled and does not call `sweepOrphans`. - SEC-006 retention boundary remains scoped: session GC deletes only the requested session namespace, demotes logs through `promoteSessionToWarm(sessionId, cutoff)`, exposes no `fullCollect`/`sweepOrphans`, and startup removes legacy repeatable global GC schedules. - `git diff --check` clean; no secrets in diff.

jason.woltje merged commit 753a360517 into main 2026-07-13 00:44:20 +00:00

jason.woltje deleted branch fix/tess-session-gc-scope 2026-07-13 00:44:20 +00:00

jason.woltje referenced this issue from a commit 2026-07-13 00:44:21 +00:00

fix(#707): scope session GC retention (#720)

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

bug

Defect in framework tooling or delivered code

fleet-enhancement

Fleet-wide reliability/speed/cost improvements surfaced by the nightly enhance pass

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

code-be-01 (Mosaic fleet seat code-be-01) code-be-02 (Mosaic fleet seat code-be-02) code-infra-01 (Mosaic fleet seat code-infra-01) fargo fred gate-merge-01 (Mosaic fleet seat gate-merge-01) happy jason.woltje (Jason Woltje) merge-gate ops-01 (Mosaic fleet seat ops-01) ops-02 (Mosaic fleet seat ops-02) ops-03 (Mosaic fleet seat ops-03) ops-ci-01 (Mosaic fleet seat ops-ci-01) ops-deploy-01 (Mosaic fleet seat ops-deploy-01) orch-01 (Mosaic fleet seat orch-01) pepper resume rev-code-01 rev-code-02 rev-security-01 rev-security-02 rev-security-03 (Mosaic fleet seat rev-security-03) sanity scooby (Scooby) scrappy shaggy tiny velma veronica (Mosaic fleet seat veronica) vision woodpecker

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: mosaicstack/stack#720