Agent Code Execution — bash/code in chat via Fly Sprites (epic)

[2witstudios]

Agent Code Execution (epic) — bash/code in chat via Fly Sprites

Lets PageSpace AI agents run bash and code in the existing AI chat, executed in isolated Fly Sprites microVMs, behind a default-OFF feature flag. Ships dark — unreachable by agents until the flag is enabled AND the Fly environment is provisioned.

Composed PRs (all merged into pu/flash-sandbox, each individually CI-gated)

• feat(sandbox): PR1 — safety foundation & policy (Agent Code Execution) #1465 PR1 — Safety foundation: canRunCode (drive/role, fail-closed), per-tenant/drive/user quota + concurrency (checkDistributedRateLimit), env kill-switch, resolveExecutionPolicy, buildSandboxEnv (no host secrets), audit (ActivityOperation/logActivity + securityAuditLog).
• feat(sandbox): PR2 — conversation sandbox lifecycle (Agent Code Execution) #1469 PR2 — Conversation lifecycle: unguessable HMAC session key (tenant+drive+conversation), create/resume/teardown, resume re-authz (cross-tenant bleed guard), sandbox_sessions table.
• feat(sandbox): PR3 — bash/writeFile/readFile tools over @fly/sprites (Agent Code Execution) #1472 PR3 — Tools/runners: bash/writeFile/readFile, command policy, default-deny egress, output truncation, path validation (resolvePathWithinSync).
• feat(sandbox): PR4 — register flag-gated bash/writeFile/readFile tools in agent chat #1477 PR4 — Flag-gated registration in the chat loop + call-time tool-gate (kill-switch → authz → quota), default-OFF.
• feat(sandbox): PR5 — switch execution driver to @fly/sprites (Agent Code Execution) #1481 PR5 — Execution driver = Fly Sprites (sandbox-client/sprites.ts); @vercel/sandbox fully removed.
• PR6 — Harden Agent Code Execution: purity, @fly/sprites SDK reconcile, full tests #1489 PR6 — Hardening: purity audit (injected clock/ids, Result<T>), per-sprite resource caps from policy, execFile/cwd/env, hard client-side kill() timeout, domain network policy w/ defaults, pinned SDK, full test pass.
• Review-fix round (in progress) — addressing Codex/CodeRabbit threads (egress lockdown on reuse, Node-24 runtime isolation, atomic budget charge, resume policy reapply, empty-secret reject, stale-session reclaim, blocked-escape audit, readFile memory cap, CI push-gating).

Safety posture

microVM isolation + drive/role authz + per-tenant quotas/concurrency + kill-switch + secret-scrubbed env + default-deny egress + resume re-authz + command policy + output truncation + immutable audit. Gated on feature flag + authz (NOT deployment mode).

Validation

Full CI green against master (Lint & TypeScript, Unit Tests, Security Test Suite, CodeQL, Static Security Analysis, Dependency Audit, Secret Scanning). Unit/integration tests use injected fakes — no live Fly calls.

⚠️ Enablement gates before flag-on (tracked in the "Fly Sprites Enablement & Gap Verification" task list)

The @fly/sprites network policy is domain-based only (no IP/CIDR), so it cannot itself block IP-literal egress. G1 (hard gate): empirically verify a Sprite cannot reach the Fly 6PN / *.internal / metadata / Tigris / Postgres by IP — requires the authed Fly env (CLI/token), which is NOT available in this workspace, so it could not be run here. Plus: provision Fly + SPRITES_TOKEN (G8), orphan-sweep reaper (G5), staged per-tenant rollout (G11).

Rollout

Merge is safe while dark. Enable per-tenant only after G1 passes.

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

• New Features

  • Added sandboxed code execution capability with bash command execution and file read/write operations
  • Implemented code execution quotas and rate limiting with per-tier concurrency controls

• Security

  • Added comprehensive audit logging for code execution activities
  • Implemented authorization gates and command policy enforcement for sandbox operations
  • Added egress filtering and kill-switch controls for the code execution feature

• Infrastructure

  • Updated CI/CD workflows to support sandbox-related testing
  • Added environment configuration variables for code execution settings

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
pagespace-marketing	Ready	Preview, Comment	Jun 2, 2026 10:20pm

[coderabbitai]

Warning

Review limit reached

@2witstudios, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 47 minutes and 58 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 20f1b83d-fa91-4c72-893b-3098d10ecdfa

📥 Commits

Reviewing files that changed from the base of the PR and between a697221 and a614f9f.

⛔ Files ignored due to path filters (1)

• bun.lock is excluded by !**/*.lock

📒 Files selected for processing (8)

• .github/workflows/security.yml
• packages/db/drizzle/0145_flawless_living_mummy.sql
• packages/db/drizzle/meta/0145_snapshot.json
• packages/db/drizzle/meta/_journal.json
• packages/db/package.json
• packages/db/src/schema.ts
• packages/lib/package.json
• packages/lib/src/services/sandbox/egress.ts

📝 Walkthrough

Walkthrough

This PR adds a sandboxed code-execution platform: DB-backed session tracking, Sprites-based executable sandboxes, authorization and quota gating, lifecycle planning, audit/redaction pipelines, tool runners (bash/write/read), web integration, CI workflow triggers, and packaging/type exports.

Changes

Sandboxed Code Execution Platform

Layer / File(s)	Summary
CI, env, and package wiring .github/workflows/security.yml, .github/workflows/test.yml, packages/lib/package.json, packages/lib/src/config/env-validation.ts, packages/lib/src/config/__tests__/env-validation.test.ts, packages/lib/src/monitoring/activity-logger.ts, packages/lib/src/security/distributed-rate-limit.ts	Workflow triggers now include sandbox-related paths and pu/flash-sandbox branch; packages/lib adds ./services/sandbox/* exports, typesVersions, and @fly/sprites dependency; serverEnvSchema accepts CODE_EXECUTION_ENABLED, SANDBOX_SESSION_SECRET, SPRITES_API_TOKEN; activity op includes code_execution; distributed limits add CODE_EXECUTION.
Database schema and schema exports packages/db/drizzle/0144_zippy_kid_colt.sql, packages/db/drizzle/meta/_journal.json, packages/db/package.json, packages/db/src/schema.ts, packages/db/src/schema/sandbox-sessions.ts	Adds sandbox_sessions migration, journal entry, package export for ./schema/sandbox-sessions, Drizzle schema module and indexes for conversation/lastActiveAt.
Policies & pure utilities packages/lib/src/services/sandbox/execution-policy.ts, packages/lib/src/services/sandbox/command-policy.ts, packages/lib/src/services/sandbox/egress.ts, packages/lib/src/services/sandbox/output-limit.ts, packages/lib/src/services/sandbox/sandbox-paths.ts, packages/lib/src/services/sandbox/sandbox-env.ts, packages/lib/src/services/sandbox/sandbox-options.ts	Defines execution profiles (default/minimal), command pre-checks (empty/size/metadata IP block), egress allowlist sanitizer and Sprites policy builder, UTF‑8-safe truncation, sandbox path confinement under /workspace, environment allowlist (NODE_ENV), and mapping from policy→sandbox options.
Sandbox client & Sprites driver packages/lib/src/services/sandbox/sandbox-client/types.ts, packages/lib/src/services/sandbox/sandbox-client/sprites.ts, packages/lib/src/services/sandbox/sandbox-client/__tests__/sprites.test.ts	Introduces ExecutableSandbox and ExecSandboxClient contracts; Sprites client implements run/write/read with output caps, timeouts (SIGKILL), egress lockdown, and vanished-sprite detection.
Session key & store packages/lib/src/services/sandbox/session-key.ts, packages/lib/src/services/sandbox/session-store.ts	Pure HMAC-SHA3-256 session key derivation; DB-backed SandboxSessionStore with findBySessionKey, save (upsert), touch, and remove.
Lifecycle & session manager packages/lib/src/services/sandbox/lifecycle.ts, packages/lib/src/services/sandbox/session-manager.ts, packages/lib/src/services/sandbox/__tests__/session-manager.test.ts	Pure lifecycle planner and orchestrator implementing create/resume/teardown/deny/noop behaviors, idle expiration handling, best-effort cleanup, and idempotent non-throwing teardown.
Quota & concurrency packages/lib/src/services/sandbox/quota.ts, packages/lib/src/services/sandbox/__tests__/quota.test.ts, packages/lib/src/security/distributed-rate-limit.ts	Per-user concurrency slots scaled by subscription tier, advisory preflight reading distributed rate-limit status across user/drive/tenant scopes, and budget charging that increments per-scope windows once per run.
Authorization & gate packages/lib/src/services/sandbox/can-run-code.ts, packages/lib/src/services/sandbox/tool-gate.ts, tests	Kill-switch isCodeExecutionEnabled, canRunCode authorization with explicit denial reasons, and gateSandboxToolCall that enforces kill-switch → authorize → quota in sequence.
Tool runners (bash/write/read) packages/lib/src/services/sandbox/tool-runners.ts, packages/lib/src/services/sandbox/__tests__/tool-runners.test.ts	Implements runBashInSandbox, writeSandboxFile, readSandboxFile: policy validation, quota preflight + slot reservation, session open, sandbox delegation, output truncation, anomaly auditing, and guaranteed slot release.
Audit pipeline packages/lib/src/services/sandbox/audit.ts, packages/lib/src/services/sandbox/__tests__/audit.test.ts	Redaction and truncation of code, immutable audit record construction, activity log input mapping to code_execution, conditional security events for anomalies, and resilient sink writes via Promise.allSettled.
Web integration and tests apps/web/src/lib/ai/tools/sandbox-tools.ts, apps/web/src/lib/ai/tools/sandbox-tools-runtime.ts, apps/web/src/lib/ai/core/ai-tools.ts, tests	createSandboxTools() factory and runtime wiring (buildRealSandboxRunDeps, resolveSandboxActorContext) and buildPageSpaceTools() conditional registration of code-execution tools gated by CODE_EXECUTION_ENABLED.

🎯 4 (Complex) | ⏱️ ~60 minutes

• Possibly related issue:

  • #1491: Node>=24 runtime assertion and dynamic import for the Sprites driver are directly related to runtime wiring in sandbox-tools-runtime.ts.

• Possibly related PRs:

  • 2witstudios/PageSpace#241: Related CI workflow trigger changes for security workflows.
  • 2witstudios/PageSpace#271: Related env-validation changes and schema test updates.
  • 2witstudios/PageSpace#160: Related distributed-rate-limit entries used by sandbox quota charging.

"I nibble logs and hop on keys,
I guard the code with fuzzy knees,
Redact the secrets, cap the bytes,
Spin up VMs on moonlit nights,
A rabbit cheers: safe sandbox, please!"

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch pu/flash-sandbox

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 179a140807

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[coderabbitai]

Actionable comments posted: 17

🧹 Nitpick comments (5)

packages/lib/src/config/__tests__/env-validation.test.ts (1)

49-76: ⚡ Quick win

Add matching coverage for SPRITES_API_TOKEN.

env-validation.ts gives SPRITES_API_TOKEN the same empty-string/invalid-value contract, and resolveSpritesToken() depends on that fail-closed behavior. Right now this suite only locks down the session-secret half of the change.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/lib/src/config/__tests__/env-validation.test.ts` around lines 49 -
76, Add tests mirroring the SANDBOX_SESSION_SECRET cases for SPRITES_API_TOKEN
in env-validation.test.ts: create one test that supplies SPRITES_API_TOKEN as an
empty string and asserts serverEnvSchema.safeParse returns success, and another
that supplies a too-short non-empty SPRITES_API_TOKEN and asserts safeParse
fails and the validation error issues include 'SPRITES_API_TOKEN'; this ensures
the schema behavior matches resolveSpritesToken and the empty-string fail-closed
contract.

apps/web/src/lib/ai/core/__tests__/ai-tools.test.ts (1)

226-257: ⚡ Quick win

Don’t erase the sandbox tool contract with as never.

Those casts let this test keep compiling even if buildPageSpaceTools or the sandbox tool shape changes, which weakens the registration guard you just added. Prefer a typed stub helper or satisfies against the actual factory return type here. As per coding guidelines, "Always use proper TypeScript types. Never use any types".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/lib/ai/core/__tests__/ai-tools.test.ts` around lines 226 - 257,
The tests use unsafe `as never` casts in the sandboxToolsFactory stubs which
breaks the type contract; update the two tests using buildPageSpaceTools to
return a properly typed stub instead of `as never` by either creating a small
typed helper object that matches the sandbox factory's return type (the expected
sandbox tools shape used by buildPageSpaceTools) or by using TypeScript's
`satisfies` operator against that factory return type for the literal objects
(e.g., replace `... }) as never` with `} satisfies ReturnType<typeof
sandboxToolsFactory>` or an explicit typed variable), and remove the `as never`
casts so the compiler will enforce the sandbox tool shape for `bash`,
`writeFile`, and `readFile`.

packages/lib/src/services/sandbox/execution-policy.ts (1)

16-35: ⚡ Quick win

Make ExecutionPolicy readonly to match the frozen runtime contract.

These objects are shared singletons and frozen before being returned, but the exported interface still permits policy.timeoutMs = ... at compile time. Marking the fields readonly keeps consumers from writing code that only fails at runtime. As per coding guidelines, "Always use proper TypeScript types. Never use any types".

♻️ Suggested typing change

 export interface ExecutionPolicy {
   /** Profile this policy represents (echoed back for audit/logging). */
-  profile: ExecutionProfile;
+  readonly profile: ExecutionProfile;
   /** Hard wall-clock cap for a single run, in milliseconds. */
-  timeoutMs: number;
+  readonly timeoutMs: number;
   /** vCPU allocation. */
-  vcpus: number;
+  readonly vcpus: number;
   /** Memory allocation, in megabytes. */
-  memoryMb: number;
+  readonly memoryMb: number;
   /** Disk allocation, in gigabytes. An explicit per-sprite cap, not the quota default. */
-  storageGb: number;
+  readonly storageGb: number;
   /** Maximum stdout/stderr bytes retained before truncation. */
-  maxOutputBytes: number;
+  readonly maxOutputBytes: number;
   /** Egress firewall allowlist. Empty means default-deny (no outbound). */
-  egressAllowlist: readonly string[];
+  readonly egressAllowlist: readonly string[];
   /** Whether the sandbox survives between runs. Always false in v1. */
-  persistent: boolean;
+  readonly persistent: boolean;
   /** Explicit deployment region. */
-  region: string;
+  readonly region: string;
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/lib/src/services/sandbox/execution-policy.ts` around lines 16 - 35,
The ExecutionPolicy interface currently allows mutation at compile time; update
the ExecutionPolicy declaration so every property is readonly (e.g., readonly
profile, readonly timeoutMs, readonly vcpus, readonly memoryMb, readonly
storageGb, readonly maxOutputBytes, readonly egressAllowlist, readonly
persistent, readonly region) to match the runtime frozen singleton contract used
by the sandbox execution policy; keep the existing readonly on egressAllowlist
if present and ensure the interface only exposes immutable fields so consumers
cannot assign to properties like policy.timeoutMs.

packages/lib/src/services/sandbox/__tests__/audit.test.ts (1)

90-103: ⚡ Quick win

Avoid a wall-clock SLA in this unit test.

performance.now() on shared CI runners measures scheduler noise as much as regex complexity, so this can flap even when the matcher stays linear. Keep the adversarial payload coverage, but move the strict timing bound to a benchmark/softer perf smoke test.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/lib/src/services/sandbox/__tests__/audit.test.ts` around lines 90 -
103, The test uses a strict wall-clock assertion with performance.now() and
expect(elapsedMs).toBeLessThan(1000), which flakes on CI; remove the timing
measurement and assertion in the adversarial-key test in audit.test.ts (keep the
payload, secret, buildAuditRecord call and the assertion that record.code does
not contain the secret) and move any strict performance check into a separate
benchmark or softer perf smoke test; update the test that references
buildAuditRecord and record.code accordingly so it only asserts correct
redaction, not elapsed milliseconds.

packages/lib/src/services/sandbox/sandbox-env.ts (1)

31-41: ⚡ Quick win

The return type promises keys this function may omit.

result starts empty and only gets a property when env[key] is a string, but the signature says Record<AllowlistedKey, string>. For partial inputs this can return {} while callers are told every allowlisted key exists. Return Partial<Record<AllowlistedKey, string>> instead, or materialize defaults for all allowlisted keys. As per coding guidelines, "Always use proper TypeScript types. Never use any types".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/lib/src/services/sandbox/sandbox-env.ts` around lines 31 - 41, The
function buildSandboxEnv currently returns Record<AllowlistedKey, string> but
only populates entries when env[key] is a string, which can violate the return
type; change its signature to return Partial<Record<AllowlistedKey, string>>
(or, if you prefer to keep the full Record type, ensure you materialize defaults
for every key in SANDBOX_ENV_ALLOWLIST) and update the variable declaration of
result and any callers to expect the partial type; locate buildSandboxEnv, the
result variable, SANDBOX_ENV_ALLOWLIST, AllowlistedKey and ServerEnv to apply
the type change (or implement default population) so the TypeScript types
accurately reflect possible missing keys.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/security.yml:
- Around line 29-30: The workflow currently lists pu/flash-sandbox only under
pull_request which leaves direct pushes/merges into that branch ungated; update
the workflow triggers so pu/flash-sandbox is included for push events as well
(or remove the exclusion that omits pu/flash-sandbox on push) so Security/CodeQL
runs on direct pushes and merges into pu/flash-sandbox; look for the
pull_request and push sections and ensure the branch name pu/flash-sandbox
appears under push.branches (or eliminate the exclusion in the push trigger) to
enforce the gate.
- Around line 6-9: Update the paths array in .github/workflows/security.yml to
include the web-layer registration files and generated migration artifacts so
the security workflow covers the full sandbox boundary: add patterns for
apps/web/src/lib/ai/tools/sandbox-tools.ts,
apps/web/src/lib/ai/tools/sandbox-tools-runtime.ts,
apps/web/src/lib/ai/core/ai-tools.ts and packages/db/drizzle/** (and mirror the
same additions where lines 31-34 repeat) so PRs changing tool exposure or
emitted migrations trigger the workflow.

In @.github/workflows/test.yml:
- Line 7: The workflow defines the pu/flash-sandbox branch under the
pull_request trigger but not under the push trigger, so direct pushes to that
branch skip the checks; update the .github/workflows/test.yml workflow to add
pu/flash-sandbox to the branches list under the push trigger (mirror the same
branches array used for pull_request) so both push and pull_request triggers
include pu/flash-sandbox and enforce the same gating.

In `@apps/web/src/lib/ai/tools/sandbox-tools-runtime.ts`:
- Around line 44-62: The memoized sandboxClientPromise in getSandboxClient
currently preserves a rejected promise forever; modify getSandboxClient so that
the dynamic import chain (the Promise assigned to sandboxClientPromise) has a
.catch handler that clears sandboxClientPromise = null before rethrowing the
error, ensuring subsequent calls retry; target sandboxClientPromise and
getSandboxClient (and the createSpritesSandboxClient import chain) when adding
the catch-and-rethrow logic.

In `@packages/lib/package.json`:
- Around line 190-279: The package.json's typesVersions mapping is missing
entries for several sandbox subpath exports listed under "exports" (e.g.,
"./services/sandbox/command-policy", "./services/sandbox/egress",
"./services/sandbox/output-limit", "./services/sandbox/sandbox-paths",
"./services/sandbox/sandbox-client/types",
"./services/sandbox/sandbox-client/sprites", "./services/sandbox/tool-runners",
"./services/sandbox/tool-gate"); update the "typesVersions" block to include
each of these subpaths mapping to their respective "./dist/... .d.ts" files
(matching the pattern used for the other sandbox entries) so TypeScript resolves
the correct declaration files for those exports. Ensure keys match the export
names and values point to the corresponding dist/*.d.ts paths used elsewhere in
the file.

In `@packages/lib/src/security/distributed-rate-limit.ts`:
- Around line 585-588: Update the comment block that begins "Agent code
execution daily run budget" to reference the correct provider: replace the
mention of "Vercel account-level Spend Management" with the sandbox driver's
provider ("Fly Sprites") and the appropriate control-plane term (e.g., "Fly
Sprites control plane" or "Fly Sprites cost management"). Locate this comment in
distributed-rate-limit.ts (the block preceding the daily run budget logic) and
update the wording so breadcrumbs point to the Fly Sprites control plane instead
of Vercel.

In `@packages/lib/src/services/sandbox/egress.ts`:
- Around line 45-55: The egress allowlist currently turns any non-empty string
into a Sprites domain allow rule, which allows '*' or non-host values to
short-circuit the deny-all; update buildSpriteNetworkPolicy to validate and
canonicalize egressAllowlist entries before mapping to rules: trim and lowercase
each entry, reject wildcard '*' and any value that is not a literal hostname
(reject IP literals, values containing ':' '/', or URL schemes/paths), and only
map the remaining valid hostnames to { domain, action: 'allow' } (drop invalid
entries and consider emitting a debug/log message for dropped values).

In `@packages/lib/src/services/sandbox/lifecycle.ts`:
- Around line 71-83: Move the idle-expired session reclamation ahead of the
unconditional deny: if an existingSession exists and (now.getTime() -
existingSession.lastActiveAt.getTime()) >= idleTimeoutMs then return the
teardown action (sandboxId and reason 'idle') regardless of authorization,
before checking authorization.ok; keep the original deny return for non-expired
sessions (authorization.ok false) so unauthorized actors still don't receive a
live session. Update the logic around authorization, existingSession,
idleTimeoutMs, idleFor, now and the return branches to reflect this reordered
check.

In `@packages/lib/src/services/sandbox/output-limit.ts`:
- Around line 30-34: truncateToBytes currently slices bytes to maxBytes then
decodes leniently, but the replacement char can produce more bytes than
maxBytes; update truncateToBytes so the returned text's UTF-8 byte length never
exceeds maxBytes by either trimming the byte buffer to a valid UTF-8 boundary
before decoding or by decoding and then re-checking Buffer.byteLength(decoded,
'utf8') and iteratively removing the last Unicode codepoint from decoded until
byte length <= maxBytes; reference the existing symbols cut, decoded, maxBytes,
and originalBytes and ensure truncated=true when trimmed.

In `@packages/lib/src/services/sandbox/quota.ts`:
- Around line 166-185: The current chargeCodeExecutionBudget function uses
Promise.all over budgetScopeIds with deps.charge and can partially apply charges
if some promises fail; fix by making multi-scope charging atomic: either add an
atomic bulk method to ChargeBudgetDeps (e.g., deps.bulkCharge(scopeIds, runId))
and call that from chargeCodeExecutionBudget, or implement sequential charging
with compensating rollback—charge scopes one-by-one, and on any failure call
deps.refund (or deps.uncharge) for all successfully charged scopes and surface
the original error; ensure the ChargeBudgetDeps interface is updated to include
the new bulkCharge or refund method and include an idempotency/run identifier so
retries are safe.

In `@packages/lib/src/services/sandbox/sandbox-client/sprites.ts`:
- Around line 313-320: The getOrCreate function currently returns wrap(await
sdk.getSprite(name)) without reapplying policy options; update getOrCreate to,
when sdk.getSprite(name) succeeds, compare the resumed sprite's effective
policy/caps with the incoming options and then either reapply
lockdownFreshSprite (or call a new helper like lockdownSpriteOnResume) to
enforce mutable limits or invalidate/rotate the session key when immutable caps
differ; use the existing lockdownFreshSprite signature and sdk/session-rotation
logic to ensure resumed sprites are re-locked to the provided options or a new
session is issued when policy versioning changes.
- Around line 316-331: The current catch blocks around sdk.getSprite in
createSpritesSandboxClient.getOrCreate and createSpritesSandboxClient.get
swallow all errors; change each catch to capture the error (e.g., catch (err))
and only handle the not-found condition (inspect err.code / err.status / use the
SDK's NotFoundError predicate) — on 404/not-found proceed to createSprite or
return null, otherwise rethrow the error so auth, rate-limit, and control-plane
errors surface. Ensure you update both the getOrCreate wrapper that calls
sdk.getSprite(name) and the get wrapper that calls sdk.getSprite(sandboxId).

In `@packages/lib/src/services/sandbox/session-key.ts`:
- Around line 44-53: deriveSessionKey currently accepts an empty secret which
yields a predictable HMAC; update deriveSessionKey to validate the secret input
(e.g., ensure it's a non-empty string) and throw a clear error when secret is
missing/empty before calling createHmac, so the function fails closed rather
than producing a predictable digest; reference the deriveSessionKey function and
the createHmac call when adding this guard.

In `@packages/lib/src/services/sandbox/session-manager.ts`:
- Around line 84-102: safeStop currently swallows failures which lets callers
(like provisionFresh and teardownConversationSandbox) remove the DB row even if
SandboxClient.stop failed; change safeStop to surface success/failure (e.g.,
return a boolean or throw a specific error) instead of always swallowing, and
update callers to only call SandboxSessionStore.remove (or safeRemove) when
safeStop indicates a confirmed stop; for failed stops hand the sandboxId to a
durable retry/reaper path rather than deleting the session row. Update all uses
(safeStop and safeRemove spots including the other occurrences referenced around
the provisionFresh and teardownConversationSandbox call sites) to follow this
pattern.

In `@packages/lib/src/services/sandbox/session-store.ts`:
- Around line 88-90: The conflict update on sandboxSessions using
onConflictDoUpdate currently sets { sandboxId, lastActiveAt: now, updatedAt: now
} but omits refreshing userId, so when a row is reused via sessionKey the
actor's userId remains stale; update the onConflictDoUpdate set map to also
assign userId (e.g., userId: newUserId or the function parameter name used where
sandboxId is coming from) so the row's user identity is updated when a session
row is reused, referencing sandboxSessions, sessionKey, userId, and the
onConflictDoUpdate call to locate the change.

In `@packages/lib/src/services/sandbox/tool-runners.ts`:
- Around line 284-289: The branch in the tool runner that resolves cwd (use of
resolvedCwd, resolveSandboxPath and SANDBOX_ROOT) returns fail('path_escape')
when resolveSandboxPath returns falsy but does not create an audit entry, unlike
writeSandboxFile/readSandboxFile which log 'blocked_command'; add an audit/log
call for the blocked sandbox-escape attempt (using the same 'blocked_command'
audit semantics) immediately before returning fail('path_escape') so all denied
cwd escapes are consistently audited.
- Around line 427-457: The current flow reads the entire file into a Buffer via
session.sandbox.readFileToBuffer and only then applies truncateToBytes, which
can OOM; change the sandbox read to enforce policy.maxOutputBytes at the API
boundary (e.g. add/use a readFileToLimitedBuffer or a maxBytes option on
session.sandbox.readFileToBuffer) so the client never returns more than
policy.maxOutputBytes, and ensure decoding respects UTF‑8 boundaries
(truncateToBytes or equivalent should operate on the limited Buffer, not after
full decode); keep the existing safeAudit calls (profile, code, exitCode,
durationMs, anomaly) and error paths but replace the raw full-buffer read in
this block with the bounded read call and then decode/truncate only that bounded
data.

---

Nitpick comments:
In `@apps/web/src/lib/ai/core/__tests__/ai-tools.test.ts`:
- Around line 226-257: The tests use unsafe `as never` casts in the
sandboxToolsFactory stubs which breaks the type contract; update the two tests
using buildPageSpaceTools to return a properly typed stub instead of `as never`
by either creating a small typed helper object that matches the sandbox
factory's return type (the expected sandbox tools shape used by
buildPageSpaceTools) or by using TypeScript's `satisfies` operator against that
factory return type for the literal objects (e.g., replace `... }) as never`
with `} satisfies ReturnType<typeof sandboxToolsFactory>` or an explicit typed
variable), and remove the `as never` casts so the compiler will enforce the
sandbox tool shape for `bash`, `writeFile`, and `readFile`.

In `@packages/lib/src/config/__tests__/env-validation.test.ts`:
- Around line 49-76: Add tests mirroring the SANDBOX_SESSION_SECRET cases for
SPRITES_API_TOKEN in env-validation.test.ts: create one test that supplies
SPRITES_API_TOKEN as an empty string and asserts serverEnvSchema.safeParse
returns success, and another that supplies a too-short non-empty
SPRITES_API_TOKEN and asserts safeParse fails and the validation error issues
include 'SPRITES_API_TOKEN'; this ensures the schema behavior matches
resolveSpritesToken and the empty-string fail-closed contract.

In `@packages/lib/src/services/sandbox/__tests__/audit.test.ts`:
- Around line 90-103: The test uses a strict wall-clock assertion with
performance.now() and expect(elapsedMs).toBeLessThan(1000), which flakes on CI;
remove the timing measurement and assertion in the adversarial-key test in
audit.test.ts (keep the payload, secret, buildAuditRecord call and the assertion
that record.code does not contain the secret) and move any strict performance
check into a separate benchmark or softer perf smoke test; update the test that
references buildAuditRecord and record.code accordingly so it only asserts
correct redaction, not elapsed milliseconds.

In `@packages/lib/src/services/sandbox/execution-policy.ts`:
- Around line 16-35: The ExecutionPolicy interface currently allows mutation at
compile time; update the ExecutionPolicy declaration so every property is
readonly (e.g., readonly profile, readonly timeoutMs, readonly vcpus, readonly
memoryMb, readonly storageGb, readonly maxOutputBytes, readonly egressAllowlist,
readonly persistent, readonly region) to match the runtime frozen singleton
contract used by the sandbox execution policy; keep the existing readonly on
egressAllowlist if present and ensure the interface only exposes immutable
fields so consumers cannot assign to properties like policy.timeoutMs.

In `@packages/lib/src/services/sandbox/sandbox-env.ts`:
- Around line 31-41: The function buildSandboxEnv currently returns
Record<AllowlistedKey, string> but only populates entries when env[key] is a
string, which can violate the return type; change its signature to return
Partial<Record<AllowlistedKey, string>> (or, if you prefer to keep the full
Record type, ensure you materialize defaults for every key in
SANDBOX_ENV_ALLOWLIST) and update the variable declaration of result and any
callers to expect the partial type; locate buildSandboxEnv, the result variable,
SANDBOX_ENV_ALLOWLIST, AllowlistedKey and ServerEnv to apply the type change (or
implement default population) so the TypeScript types accurately reflect
possible missing keys.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 19100e3b-95f1-4f20-9109-4dbdc5869dbb

📥 Commits

Reviewing files that changed from the base of the PR and between 6a823cc and 50ac066.

⛔ Files ignored due to path filters (1)

• bun.lock is excluded by !**/*.lock

📒 Files selected for processing (52)

• .github/workflows/security.yml
• .github/workflows/test.yml
• apps/web/src/lib/ai/core/__tests__/ai-tools.test.ts
• apps/web/src/lib/ai/core/ai-tools.ts
• apps/web/src/lib/ai/tools/__tests__/sandbox-tools.test.ts
• apps/web/src/lib/ai/tools/sandbox-tools-runtime.ts
• apps/web/src/lib/ai/tools/sandbox-tools.ts
• packages/db/drizzle/0144_zippy_kid_colt.sql
• packages/db/drizzle/meta/0144_snapshot.json
• packages/db/drizzle/meta/_journal.json
• packages/db/package.json
• packages/db/src/schema.ts
• packages/db/src/schema/sandbox-sessions.ts
• packages/lib/package.json
• packages/lib/src/config/__tests__/env-validation.test.ts
• packages/lib/src/config/env-validation.ts
• packages/lib/src/monitoring/activity-logger.ts
• packages/lib/src/security/distributed-rate-limit.ts
• packages/lib/src/services/sandbox/__tests__/audit.test.ts
• packages/lib/src/services/sandbox/__tests__/can-run-code.test.ts
• packages/lib/src/services/sandbox/__tests__/command-policy.test.ts
• packages/lib/src/services/sandbox/__tests__/egress.test.ts
• packages/lib/src/services/sandbox/__tests__/execution-policy.test.ts
• packages/lib/src/services/sandbox/__tests__/lifecycle.test.ts
• packages/lib/src/services/sandbox/__tests__/output-limit.test.ts
• packages/lib/src/services/sandbox/__tests__/quota.test.ts
• packages/lib/src/services/sandbox/__tests__/sandbox-env.test.ts
• packages/lib/src/services/sandbox/__tests__/sandbox-options.test.ts
• packages/lib/src/services/sandbox/__tests__/sandbox-paths.test.ts
• packages/lib/src/services/sandbox/__tests__/session-key.test.ts
• packages/lib/src/services/sandbox/__tests__/session-manager.test.ts
• packages/lib/src/services/sandbox/__tests__/tool-gate.test.ts
• packages/lib/src/services/sandbox/__tests__/tool-runners.test.ts
• packages/lib/src/services/sandbox/audit.ts
• packages/lib/src/services/sandbox/can-run-code.ts
• packages/lib/src/services/sandbox/command-policy.ts
• packages/lib/src/services/sandbox/egress.ts
• packages/lib/src/services/sandbox/execution-policy.ts
• packages/lib/src/services/sandbox/lifecycle.ts
• packages/lib/src/services/sandbox/output-limit.ts
• packages/lib/src/services/sandbox/quota.ts
• packages/lib/src/services/sandbox/sandbox-client/__tests__/sprites.test.ts
• packages/lib/src/services/sandbox/sandbox-client/sprites.ts
• packages/lib/src/services/sandbox/sandbox-client/types.ts
• packages/lib/src/services/sandbox/sandbox-env.ts
• packages/lib/src/services/sandbox/sandbox-options.ts
• packages/lib/src/services/sandbox/sandbox-paths.ts
• packages/lib/src/services/sandbox/session-key.ts
• packages/lib/src/services/sandbox/session-manager.ts
• packages/lib/src/services/sandbox/session-store.ts
• packages/lib/src/services/sandbox/tool-gate.ts
• packages/lib/src/services/sandbox/tool-runners.ts

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

packages/lib/src/services/sandbox/quota.ts (1)

166-195: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Restore atomic-looking charging instead of documenting partial charges as acceptable.

Promise.all still lets earlier scope increments stick when a later scope rejects, so a run can return an error after consuming user/drive/tenant budget anyway. That breaks the “denied run never charges” contract in this function and can unfairly exhaust quotas until the window rolls over.

Compensating-rollback sketch

 export interface ChargeBudgetDeps {
   /** Increment the daily budget for one scoped identifier (consumes the window). */
   charge: (id: string) => Promise<void>;
+  /** Undo one previously applied charge. */
+  refund: (id: string) => Promise<void>;
 }
@@
 export async function chargeCodeExecutionBudget({
   userId,
   driveId,
   tenantId,
   deps = defaultChargeDeps,
 }: {
@@
 }): Promise<void> {
-  await Promise.all(budgetScopeIds({ userId, driveId, tenantId }).map((id) => deps.charge(id)));
+  const charged: string[] = [];
+  try {
+    for (const id of budgetScopeIds({ userId, driveId, tenantId })) {
+      await deps.charge(id);
+      charged.push(id);
+    }
+  } catch (error) {
+    await Promise.allSettled(charged.map((id) => deps.refund(id)));
+    throw error;
+  }
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/lib/src/services/sandbox/quota.ts` around lines 166 - 195,
chargeCodeExecutionBudget currently uses Promise.all which can leave earlier
scope charges applied if a later charge fails; change it to perform sequential
charges using budgetScopeIds({ userId, driveId, tenantId }) and deps.charge so
you can track successful ids, and on any failure run a compensating rollback
calling a new/extended refund/uncharge method on ChargeBudgetDeps (e.g.,
deps.refund or deps.uncharge) for each already-charged id; ensure rollback is
best-effort (log rollback errors but do not mask the original charge error) and
rethrow the original error so callers see the denial, and update
ChargeBudgetDeps/defaultChargeDeps accordingly to expose the refund/uncharge
operation.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/security.yml:
- Around line 10-13: security.yml currently gates sandbox-related paths but
doesn't execute the sandbox Vitest tests; add a job/step in the workflow to
install deps and run the sandbox test suite (e.g., run vitest or the repo's test
script) limited to src/services/sandbox/** tests so changes to
packages/lib/src/services/sandbox/** will actually run
src/services/sandbox/__tests__/egress.test.ts and
src/services/sandbox/__tests__/session-manager.test.ts; ensure the step runs
after checkout and setup-node (or equivalent), uses the repo package manager
(pnpm/npm/yarn) to install, and invokes the test command targeting the sandbox
test patterns.

---

Duplicate comments:
In `@packages/lib/src/services/sandbox/quota.ts`:
- Around line 166-195: chargeCodeExecutionBudget currently uses Promise.all
which can leave earlier scope charges applied if a later charge fails; change it
to perform sequential charges using budgetScopeIds({ userId, driveId, tenantId
}) and deps.charge so you can track successful ids, and on any failure run a
compensating rollback calling a new/extended refund/uncharge method on
ChargeBudgetDeps (e.g., deps.refund or deps.uncharge) for each already-charged
id; ensure rollback is best-effort (log rollback errors but do not mask the
original charge error) and rethrow the original error so callers see the denial,
and update ChargeBudgetDeps/defaultChargeDeps accordingly to expose the
refund/uncharge operation.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cc1b5b9c-5512-4790-8cac-e6895978d479

📥 Commits

Reviewing files that changed from the base of the PR and between 50ac066 and a697221.

📒 Files selected for processing (20)

• .github/workflows/security.yml
• .github/workflows/test.yml
• apps/web/src/lib/ai/tools/sandbox-tools-runtime.ts
• packages/lib/package.json
• packages/lib/src/security/distributed-rate-limit.ts
• packages/lib/src/services/sandbox/__tests__/egress.test.ts
• packages/lib/src/services/sandbox/__tests__/output-limit.test.ts
• packages/lib/src/services/sandbox/__tests__/session-key.test.ts
• packages/lib/src/services/sandbox/__tests__/session-manager.test.ts
• packages/lib/src/services/sandbox/__tests__/tool-runners.test.ts
• packages/lib/src/services/sandbox/egress.ts
• packages/lib/src/services/sandbox/lifecycle.ts
• packages/lib/src/services/sandbox/output-limit.ts
• packages/lib/src/services/sandbox/quota.ts
• packages/lib/src/services/sandbox/sandbox-client/__tests__/sprites.test.ts
• packages/lib/src/services/sandbox/sandbox-client/sprites.ts
• packages/lib/src/services/sandbox/session-key.ts
• packages/lib/src/services/sandbox/session-manager.ts
• packages/lib/src/services/sandbox/session-store.ts
• packages/lib/src/services/sandbox/tool-runners.ts

🚧 Files skipped from review as they are similar to previous changes (15)

• .github/workflows/test.yml
• packages/lib/src/services/sandbox/tests/session-key.test.ts
• packages/lib/src/security/distributed-rate-limit.ts
• packages/lib/src/services/sandbox/output-limit.ts
• packages/lib/src/services/sandbox/session-key.ts
• packages/lib/package.json
• packages/lib/src/services/sandbox/sandbox-client/tests/sprites.test.ts
• packages/lib/src/services/sandbox/tests/output-limit.test.ts
• apps/web/src/lib/ai/tools/sandbox-tools-runtime.ts
• packages/lib/src/services/sandbox/lifecycle.ts
• packages/lib/src/services/sandbox/session-store.ts
• packages/lib/src/services/sandbox/tests/tool-runners.test.ts
• packages/lib/src/services/sandbox/session-manager.ts
• packages/lib/src/services/sandbox/sandbox-client/sprites.ts
• packages/lib/src/services/sandbox/tool-runners.ts