feat: per-browser session isolation via X-Client-Id (fixes shared-state privacy leak)#17
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 36 minutes and 54 seconds.Comment |
Deploying chainshield with
|
| Latest commit: |
83fdbcb
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f35e8191.chainshield.pages.dev |
| Branch Preview URL: | https://feat-per-client-isolation.chainshield.pages.dev |
…onger leaks one user's policies and timeline rows to other users; the server has one in-memory store and prior to this every browser hitting the same URL saw the same /policies and /timeline; the fix tags every row at write time with the X-Client-Id header sent by the frontend (a stable UUID stored in localStorage on first load) and filters every read by the same id, so Browser A's clientId=abc only sees rows it wrote and getPolicy on a foreign id returns 404 without leaking existence; the Store interface now accepts an optional clientId on every CRUD method, InMemoryStore + ZeroGStore tag rows internally without changing the public Policy/Decision schema, PolicyService and DecisionEngine thread the id end-to-end (the daily-outflow rule reads the timeline for its own clientId so Browser A's transactions don't bleed into Browser B's daily cap), Fastify clientIdOf reads x-client-id with bound checks (1-128 chars, trimmed, falls back to admin view on missing/oversized), and the Astro fetch wrapper persists getClientId() in localStorage falling back to in-memory uuid when localStorage throws (Safari private mode); 8 new specs cover the API path (browser A's policy invisible to B, getPolicy 404 not 200-empty for foreign id, evaluate 404s for foreign policy, timeline isolation, admin view via no-header, oversized header rejection) plus two store-level unit tests
AnkanMisra
force-pushed
the
feat/per-client-isolation
branch
from
49c96f1 to
83fdbcb
Compare
1 participant
Why
User report: opened the deployed
chainshield.pages.devin two browsers, the second browser showed the first browser's policies and decisions. The deployed server has one in-memory store and every browser hitting the same URL was reading the same data — privacy violation for any non-trivial deployment.What changes
A stable per-browser UUID is generated on first load, sent as the
X-Client-Idheader on every API call, and used as a filter on every store read + a tag on every store write. Browser A's clientId never sees Browser B's data; foreign id lookups return 404 instead of leaking existence.Frontend — one stable session id per browser
web/src/lib/api.tsnow exportsgetClientId():localStorageunder keychainshield:client-idcrypto.randomUUID()on first load and persists itlocalStoragethrows (Safari private mode)api()wrapper setsX-Client-Idon every fetch automatically — no caller-side change neededServer — clientId threads through the whole request
Storeinterface: every CRUD method takes an optionalclientId. Writes tag the row; reads filter to matching tag.undefinedclientId = global admin view (legacy curl/CLI path).InMemoryStore,ZeroGStore: store rows inMap<id, { row, clientId }>shape. Filter on read.PolicyService.create / update / get / list: threadclientId.DecisionEngine.evaluate(intent, policy, clientId?): persists the decision under the caller's clientId, and the daily-outflow timeline read uses the same id so Browser A's history doesn't bleed into Browser B'smaxDailyOutflowEthrule.FastifyclientIdOf(req): readsx-client-id, trims, validates (1-128 chars), returnsundefinedon missing/oversized to fall back to admin view safely.Tests — 8 new specs
tests/clientIsolation.test.ts:/policieslistGET /policies/:idreturns 404 for a foreign-owned id (no existence leak)POST /evaluatereturns 404 (PolicyNotFound) for a foreign-owned policy/timelineis isolated — Browser A's decisions don't appear in Browser B's viewX-Client-Idsee the global admin view (curl / CLI / tests)Plus two store-level unit tests verifying the tagging behaviour directly.
Verification
bun test— 101 specs across 12 files (was 93; +8 isolation specs)bun run typecheck— server (tsc --noEmit) + web (astro check: 0/0/0) cleanbun run build:web— Astro production build succeedsBackwards compatibility
bun run demo) doesn't send the header — falls into the admin view, sees everything as before.clientIdstill pass —undefinedeverywhere = legacy global behaviour.What this PR does NOT do
Need help on this PR? Tag
@codesmithwith what you need.