Post-v0.0.15 audit remediation (2 High, 2 Medium, Lows)#22
Merged
Conversation
The DELETE /:id/suspensions/:suspensionId route only checked sessionId consistency, not ownership — on hosted tiers anyone knowing the session and suspension ids could delete another session's suspension state. Run resolveSessionParam first, matching its sibling resume routes (H-1).
fetchWithRetry validated only the initial URL and followed redirects, so a plugin hitting an endpoint that returns 302 Location: http://169.254.169.254/… was dialed straight into cloud metadata (SSRF). Force redirect: "manual" (after ...rest so a caller can't override) and throw on any 3xx, matching the core provider HTTP path (H-2).
A cross-pod receiver only detected a seq gap when a later frame arrived out of order. A lost *tail* frame (last NOTIFY dropped, no successor) left the receiver silently stale until the next event or reconnect. Each origin now publishes a periodic heartbeat carrying its high-water transportSeq through the same per-session outbox as real frames, so the receiver treats senderSeq >= expected as a hole and resets. seq/epoch are captured at enqueue time so a concurrent emit cannot make the heartbeat overtake a real frame and cause a false reset (M-1).
The orphaned-job sweep ran only once at boot and skipped jobs younger than the staleness threshold, so a job pending <15min before a crash hung forever. Re-run it on an unref'd interval (drained on shutdown) and anchor freshness on max(startedAt, updatedAt) so a job that writes progress is never force-failed by the now-periodic sweep (M-2).
Session create/get/patch/list echoed metadata.ownerTokenHash back to the caller; strip it via sanitizeSessionForResponse (L-2). POST /events/emit was gated only on NODE_ENV, so a non-production hosted boot accepted arbitrary cross-session events; add a session-owner check (no-op on self) (L-3).
retry-startup, open-{logs,config,data}-dir, restart-server, pick-data-dir and
import:pick-{plugin,world} handled IPC without the isTrustedSender check the
secret channels use. Add it as defense-in-depth behind nav-pinning so an
untrusted frame can't drive restart / dir / import actions (L-4).
Resolve getStreamingText(id) at export time so a mid-stream export keeps the partial assistant text instead of a blank row (L-10). Clear the live buffer on any story-runtime completion, including empty content, so stale partial text can't linger (L-11). Remove the scroll listener via a stored handler ref instead of the no-op onscroll=null (L-12).
Graceful shutdown closed the DataStore without waiting for an in-flight turn, so a turn mid-commit could have the store torn out from under it. Add a time-boxed drain phase (awaitActiveTurnsDrained) before store close; the existing 2s drainPhase timeout guarantees a stuck turn can't block shutdown (L-6).
The compactor tagged its trace event with the summaryId, so context.compacted never showed under the turn's traceId in /api/traces. Thread the turn traceId through CompactorRunner.run via the existing options bag and use it for the trace event, falling back to summaryId when absent (L-8).
updatedAt starts equal to startedAt and only moves forward, so the sweep's freshness is just Date.parse(row.updatedAt) — drop the redundant max() over startedAt. Also drop the test-only activeTurnCount export; the drain test uses hasActiveTurn instead. (ponytail-review shrink, no behavior change.)
…ctive) PR #22 review found all three reliability patches unsound; each needs infrastructure beyond a Low/Medium remediation and is reverted to backlog: - L-6 (turn drain): awaited the wrong signal. actions.ts releases the W4 turn-control registry in executeTurn's finally, BEFORE proposal/snapshot commit; plugin-rpc/resume turns never register. So the store could still close mid-commit — false confidence. A correct fix drains the commit pipeline / held session locks, not activeTurns. - M-2 (periodic job sweep): a regression. background-jobs writes updatedAt only at enqueue and completion, never during a run, so a queued/long (>15min) job is force-failed mid-flight and later overwritten to done — clients observe a spurious failed state. Closing the boot blind spot safely needs a real job lease. Reverted to boot-only sweep. - M-1 (transport heartbeat): the sender loop iterates the replay-session LRU (cap 256), so a high-fanout pod evicts sender state and skips heartbeats — weakest exactly when tail-loss is most likely. Needs LRU-independent high-water state; deferred to backlog per the domain review. Keeps the sound fixes (2 High + L-2/L-3/L-4/L-8/L-10/L-11/L-12).
ackness
marked this pull request as ready for review
July 16, 2026 12:41
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Full-repository audit of PR #21 (v0.0.15) found defects the initial audit missed; this branch fixes the ones that are cleanly fixable and reverts three reliability patches that turned out to need infrastructure.
High (kept)
fix(server): enforce the session-owner guard onDELETE /api/sessions/:id/suspensions/:suspensionId— it previously only checked sessionId consistency, so on hosted tiers anyone knowing the ids could delete another session's suspension state.fix(ai-provider): the pluginctx.http/fetchWithRetryhelper followed redirects with only the initial URL SSRF-checked, so a302 Location: http://169.254.169.254/…reached cloud metadata. Forceredirect: "manual"and fail closed on any 3xx.Low (kept)
fix(server): stripmetadata.ownerTokenHashfrom session responses; gatePOST /events/emiton the session-owner check.fix(desktop): addisTrustedSenderto the remaining IPC channels.fix(web): resolve live streaming text on export; clear the live buffer on any story-runtime completion; remove the scroll listener via a stored handler ref.fix(context): file thecontext.compactedtrace under the turn's traceId.Reverted to backlog after review (see commit
revert:)activeTurnsis released before the proposal/snapshot commit; plugin-rpc/resume turns never register), so it didn't protect the commit window.updatedAtonly at enqueue/completion, so a queued or long-running (>15min) job was force-failed mid-flight, giving clients a spuriousfailedstate. Reverted to boot-only sweep.Why
The v0.0.15 hardening was buildable and its own audit closed the tracked Critical/High items, but a fresh 7-reviewer pass found two new High gaps (a cross-tenant destructive write and a plugin-reachable SSRF) plus hygiene issues. Three reliability items looked fixable but a review of the remediation showed each needs real infrastructure (a commit-pipeline/session-lock drain, a job lease, LRU-independent transport high-water state); shipping the partial versions would give false confidence or regress, so they are documented and deferred.
How it was validated
pnpm lint21/21 ·pnpm test40/40.Known risks / follow-up
audits/2026-07-16-pr21-full-audit/REPORT.md): commit-window drain, job lease, LRU-independent heartbeat state./api/llm-confignon-secret metadata the UI needs), L-5 (by-design guard window), L-7/L-9/L-13/L-14 (observability/cosmetic).