test(queue): verify concurrent cross-tenant execution isolation - #6299
Conversation
…Job (#4793) Rent-a-Loop's #4793 asked for a deliberately adversarial test proving one tenant's execution can't reach another tenant's data. Audited every module-level mutable state candidate across src/ (github, queue, review, selfhost, signals, db -- ~96 declarations) plus packages/loopover-miner's execution runtime: everything is either a static read-only allowlist, a correctly tenant/token-keyed cache (verified via existing key-derivation code, e.g. the GitHub response cache's auth-identity+URL key), a deployment-wide singleton by design (SDK clients, health counters, kill-switch cache), or -- for the miner's own execution runtime -- safe by construction since each rented loop run is a fresh CLI process per the architecture #4783 already established, not a shared long-lived instance. Adds a stronger end-to-end proof than #4794's narrower credential-cache test: two different installations' real pull_request webhooks driven through the actual processJob entry point CONCURRENTLY, sharing one Worker isolate's module-level state, with every GitHub write checked against both tenants' identifying data. No code changes were needed -- the isolation already held.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-16 00:43:25 UTC
Review summary Nits — 6 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionNot yet addressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://gittensory.aethereal.dev/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Summary
Closes #4793. Per its acceptance criterion — "a deliberately adversarial test run (attempting to access another tenant's data or gittensory's own infrastructure) fails to escape the sandbox" — this required two things: an audit, and proof.
The audit. Systematically reviewed every module-level mutable-state candidate:
let/var/Map/Setmodule-level declarations acrosssrc/github/**,src/queue/**,src/review/**,src/selfhost/**,src/signals/**,src/db/**. The vast majority are static read-only allowlists (hardcoded string vocab, never mutated). The real candidates — the installation-token cache, the GitHub REST response cache,inFlightGraphQlPostssingle-flight map — are all correctly keyed by installation/auth-identity + URL (verified by reading each key-derivation function directly, e.g.responseCacheKeyinsrc/github/client.tsembeds both the auth identity and the full URL). The rest are deployment-wide singletons by design (Sentry/OTel SDK clients, the AI-provider health streak, the global kill-switch's process-local cache) — correctly global, not tenant data.packages/loopover-miner's own module-level state (thedefault*Storesingleton pattern used throughout its lib files) is safe by construction: per Multi-tenant hosted architecture spec #4783's architecture, each rented loop run is an independent CLI process invocation, never a shared long-lived instance serving multiple tenants — module state can't leak across runs that don't share a process.No code changes were needed anywhere — every candidate was already either correctly scoped or safely global.
The proof. Added a stronger, more direct test than #4794's narrower credential-cache-only coverage: two different installations' real
pull_requestwebhooks driven through the actualprocessJobentry point, running truly concurrently (Promise.all), sharing one Worker isolate's module-level state end-to-end. Every GitHub write (the reviewing-placeholder post and the final comment) is checked against both tenants' identifying data — repo full name and PR title never cross-contaminate, and each write carries only its own installation's minted token.Test plan
npx tsc --noEmitcleannpm run test:ci(full local gate incl. drift checks, coverage, engine parity, MCP/miner packs, UI build): green, verified via literal captured exit codetest/unit/queue.test.tsin isolation: 166/166 passedorigin/main, no conflicts