Skip to content

fix(sessions): re-enable v2 shared session access - #5300

Merged
eshurakov merged 5 commits into
mainfrom
eshurakov/eager-estuary
Aug 17, 2026
Merged

fix(sessions): re-enable v2 shared session access#5300
eshurakov merged 5 commits into
mainfrom
eshurakov/eager-estuary

Conversation

@eshurakov

@eshurakov eshurakov commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Re-enable v2 public session sharing after fix(sessions): disable shared session access #5122 and after existing public_ids were nulled in production
  • Replace raw /s/{public_id} URLs with purpose-bound HS256 JWT share tokens (/s/{share_token})
  • Leave legacy /share/{shareId} 404s in place

How it works

  • Share still stores a UUID public_id on cli_sessions_v2. session-ingest signs a JWT with that UUID as jti, the session id as sub, issuer kilo-session-ingest, audience kilo-session-share
  • The public page and ingest lookup verify the token, then resolve the session only if public_id still matches jti (unshare / rotate invalidates old links)
  • Tokens with iat before SESSION_SHARE_TOKEN_MIN_IAT are rejected, so we can globally invalidate every share issued before a given unix timestamp without rotating the signing secret
  • Shared pages load metadata from the worker; they never look up by UUID in the URL
  • PostHog skips $pageview on /s/*; PostHog and Sentry redact path tails to /s/:share_token

Verification

  • pnpm --filter cloudflare-session-ingest test
  • pnpm --filter kilo-app session share / analytics tests

Restore /s/{public_id} and the public ingest lookup after existing
public_ids were exported and nulled. Leave legacy /share pages disabled.
@kilo-code-bot

kilo-code-bot Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

Reviewed the incremental commits converting v2 session sharing from plain public_id URLs to purpose-bound HS256 JWT share tokens (/s/{share_token}), including the new session-share-token sign/verify/resolve service, the metadata endpoint, analytics/Sentry referrer redaction, and all updated tRPC consumers — token verification, generation-bound resolution, no-store caching, and secret handling are sound with strong test coverage.

Files Reviewed (26 files)
  • apps/web/next.config.mjs
  • apps/web/sentry.server.config.ts
  • apps/web/src/app/(app)/cloud/webhooks/[triggerId]/requests/WebhookRequestsContent.tsx
  • apps/web/src/app/s/[sessionId]/page.test.tsx
  • apps/web/src/app/s/[sessionId]/page.tsx
  • apps/web/src/components/PostHogProvider.tsx
  • apps/web/src/components/cloud-agent-next/SessionActionsDialog.tsx
  • apps/web/src/components/cloud-agent-next/ShareSessionDialog.tsx
  • apps/web/src/lib/sanitize-analytics-url.test.ts
  • apps/web/src/lib/sanitize-analytics-url.ts
  • apps/web/src/lib/session-ingest-client.test.ts
  • apps/web/src/lib/session-ingest-client.ts
  • apps/web/src/routers/cli-sessions-v2-router.test.ts
  • apps/web/src/routers/cli-sessions-v2-router.ts
  • apps/web/src/sentry.server.config.test.ts
  • pnpm-lock.yaml
  • services/session-ingest/package.json
  • services/session-ingest/src/app.ts
  • services/session-ingest/src/env.ts
  • services/session-ingest/src/index.test.ts
  • services/session-ingest/src/routes/api.test.ts
  • services/session-ingest/src/routes/api.ts
  • services/session-ingest/src/services/session-share-token.test.ts
  • services/session-ingest/src/services/session-share-token.ts
  • services/session-ingest/worker-configuration.d.ts
  • services/session-ingest/wrangler.jsonc
Previous Review Summaries (2 snapshots, latest commit cae27fa)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit cae27fa)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Reviewed the incremental commits converting v2 session sharing from plain public_id URLs to purpose-bound HS256 JWT share tokens (/s/{share_token}), including the new session-share-token sign/verify/resolve service, the metadata endpoint, analytics/Sentry referrer redaction, and all updated tRPC consumers — token verification, generation-bound resolution, no-store caching, and secret handling are sound with strong test coverage.

Files Reviewed (26 files)
  • apps/web/next.config.mjs
  • apps/web/sentry.server.config.ts
  • apps/web/src/app/(app)/cloud/webhooks/[triggerId]/requests/WebhookRequestsContent.tsx
  • apps/web/src/app/s/[sessionId]/page.test.tsx
  • apps/web/src/app/s/[sessionId]/page.tsx
  • apps/web/src/components/PostHogProvider.tsx
  • apps/web/src/components/cloud-agent-next/SessionActionsDialog.tsx
  • apps/web/src/components/cloud-agent-next/ShareSessionDialog.tsx
  • apps/web/src/lib/sanitize-analytics-url.test.ts
  • apps/web/src/lib/sanitize-analytics-url.ts
  • apps/web/src/lib/session-ingest-client.test.ts
  • apps/web/src/lib/session-ingest-client.ts
  • apps/web/src/routers/cli-sessions-v2-router.test.ts
  • apps/web/src/routers/cli-sessions-v2-router.ts
  • apps/web/src/sentry.server.config.test.ts
  • pnpm-lock.yaml
  • services/session-ingest/package.json
  • services/session-ingest/src/app.ts
  • services/session-ingest/src/env.ts
  • services/session-ingest/src/index.test.ts
  • services/session-ingest/src/routes/api.test.ts
  • services/session-ingest/src/routes/api.ts
  • services/session-ingest/src/services/session-share-token.test.ts
  • services/session-ingest/src/services/session-share-token.ts
  • services/session-ingest/worker-configuration.d.ts
  • services/session-ingest/wrangler.jsonc

Previous review (commit e8d95aa)

Status: No Issues Found | Recommendation: Merge

Reviewed the re-enablement of v2 shared session access: the /s/{public_id} page and the unauthenticated GET /session/:public_id ingest lookup are restored cleanly, with UUID validation, indexed public_id lookup, NOT NULL schema guarantees on the DO lookup keys, and tests covering 400/404/200 paths.

Files Reviewed (3 files)
  • apps/web/src/app/s/[sessionId]/page.tsx
  • services/session-ingest/src/app.ts
  • services/session-ingest/src/index.test.ts

Reviewed by kimi-k3 · Input: 91.3K · Output: 7.7K · Cached: 600.6K

Review guidance: REVIEW.md from base branch main

Replace public_id lookups with purpose-bound share tokens issued by
session-ingest. Shared pages resolve metadata through the worker, and
PostHog/Sentry redact /s/{token} paths.
Fail closed before the database lookup when the JWT jti is missing or empty.
Stop recording on /s/* and block share-URL nodes so JWT credentials are not captured in DOM snapshots.
@eshurakov
eshurakov merged commit b0997ec into main Aug 17, 2026
69 checks passed
@eshurakov
eshurakov deleted the eshurakov/eager-estuary branch August 17, 2026 19:46
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