fix(security): close final AppSec review gaps

[aviggiano]

Summary

• reject Modal handoff unless sealed governance proves the source is clean
• keep operator-allowlisted secret names and secret-looking values out of unrelated local model children and Modal task secrets
• fail closed when a cloud environment changes security classification between compilation and initial or lifecycle launch
• document the supported routing and clean-source behavior

This addresses the remaining P1 feedback in PR #635 (discussion_r3803978885) and PR #639 (discussion_r3803971550). It is the final follow-up for #527 and #628.

Validation

• pnpm typecheck
• pnpm lint
• pnpm format:check
• pnpm docs:check
• focused runtime regressions: 3 passed
• security package: 18 passed
• focused Modal secret-binding regression: passed
• full Modal package: 563 passed; one unrelated 5-second timeout passed in isolation in 3.46 seconds
• two independent reviews: no remaining actionable findings

Final feedback and CI stabilization

• sealed legacy lifecycle relaunches now fail closed only when custom sensitive allowlisted variables cannot be preserved; current controllers and marker-less empty-sensitive runs remain compatible;
• the Make OpenRouter cross-pipe quarantine regression deterministic #647 OpenRouter regression acknowledges the relay actual provisional state before adversarial stdout, with no cross-pipe timing sleep;
• the acknowledgement and mutation branch are positively asserted;
• focused race stress passed 50/50 sequential iterations; both OpenRouter regressions, both lifecycle regressions, controller-source coverage, test/production TypeScript checks, ESLint, Prettier, and diff checks pass;
• two independent follow-up reviews report no remaining findings.

Fixes #647.

Greptile Summary

This follow-up closes the remaining cloud handoff, provider-scoped environment, lifecycle-classification, OpenRouter relay, and operator npm advisory gaps.

• Requires sealed clean-source governance before Modal handoff.
• Scopes sensitive allowlisted environment entries to their owning provider route across local and Modal execution.
• Revalidates cloud credential classification at initial and lifecycle launch boundaries.
• Pins and verifies the private operator npm closure and its bundled production dependencies.
• Retains final OpenRouter rate-limit evidence when replacement construction crosses the recovery deadline.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
packages/modal/src/node-provider.ts	Enforces clean sealed Git governance for cloud handoffs and binds only task-scoped credentials into Modal secrets.
packages/runtime/src/data-governance.ts	Adds credential-like environment classification and longest-prefix provider-route ownership.
packages/runtime/src/start-run.ts	Carries provider-scoped environment metadata and validates compatible sealed-controller behavior across launch and lifecycle paths.
packages/runtime/src/smithers.ts	Compiles task-specific cloud credentials and fails closed when their live classification differs from the sealed workflow.
packages/runtime/src/templates/smithers/agents/environment.tsx	Blanks sensitive variables from unrelated model children and restores only values owned by the active provider route.
packages/runtime/src/templates/smithers/agents/openrouter.tsx	Retains the last real rate-limit relay when replacement construction crosses the retry deadline.
packages/runtime/src/operator-npm.ts	Introduces digest-pinned, read-only provisioning and validation for the private controller’s npm closure.
packages/security/src/sensitive-redaction.ts	Extends shared sensitive environment-name and secret-value classification used by routing controls.
scripts/check-production-dependency-advisories.mjs	Expands production advisory inventory to validate bundled dependency trees and exact bundled versions.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Compile workflow] --> B[Seal governance and task environment]
  B --> C{Governed source clean?}
  C -->|No| X[Reject Modal handoff]
  C -->|Yes| D[Initial or lifecycle launch]
  D --> E{Current credential classification matches seal?}
  E -->|No| Y[Fail closed and require a new run]
  E -->|Yes| F[Select task provider route]
  F --> G[Forward ordinary allowlisted inputs]
  F --> H[Forward only route-owned sensitive inputs]
  G --> I[Local model child or Modal task]
  H --> I

Loading

_{Reviews (10): Last reviewed commit: "fix(runtime): retain final OpenRouter ra..." | Re-trigger Greptile}

Context used (3)

• Knowledge Base — Runtime: Run Orchestration Lifecycle
• Knowledge Base — Modal remote benchmark execution
• Knowledge Base — Security

[socket-security]

All alerts resolved. Learn more about Socket for GitHub.

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report

[aviggiano]

Final bundled-npm follow-up

Authoritative head: cf2926ce2e7a475f2ad498075f01b17822e4cdec.

Independent review found four Moderate advisories hidden inside npm's bundled dependencies and a path-dependent closure hash caused by pnpm-generated node_modules/.bin shims. This head addresses both without adding a permanent fork:

• the patch now byte-matches the green npm v11 upstream changes in npm/cli#9842 and npm/cli#9872: brace-expansion@5.0.9, ip-address@10.5.0, tar@7.5.22, and undici@6.28.0;
• generated .bin shims are neither hashed nor copied because they contain installation-specific absolute paths and are not used by the controller's direct npm CLI invocation;
• the private snapshot rejects any injected .bin directory;
• the patch has an explicit removal condition when upstream npm ships these versions.

Validation on this head:

• full bundled-aware registry audit: 284 production package names / 291 exact versions, 0 advisories at any severity;
• pnpm audit: 0 vulnerabilities across 1,080 dependencies;
• fresh archive at a different absolute path: frozen install passed; closure metrics and SHA-256 reproduced; 3/3 operator-npm tests passed;
• real private-snapshot npm install --ignore-scripts smoke test passed;
• formatting, ESLint, docs checks, workspace typecheck, CI policy tests, and the production advisory gate passed locally.

Socket's duplicate “obfuscated code” warnings were reviewed and are not suppressed: they refer to npm's official bundled/minified distribution, whose registry integrity and complete closure digest are pinned.

[aviggiano]

Final CI and current-main integration update:

• The CLI lane failure was an existing fixture race: its synthetic controller lease expired after 30 seconds on a slow runner, parking the node as controller-loss while fake workflow status still reported one running node.
• ac227d7f gives that synthetic run a schema-valid active lease/deadline, asserts the exact running node, and keeps exact duration formatting in the pure formatter check. The focused workflow test passed twice at 116s and 121s, well beyond the old boundary.
• 31b2a3a0 integrates current main / fix(config): raise default agent timeout to one hour #646 while preserving the one-hour timeout changes, trusted npm test launcher, provider-environment isolation, and causal blocked-child cancellation coverage.
• Focused CLI, blocked-child, and pinned local/cloud closure regressions pass; runtime and CLI test TypeScript, ESLint, Prettier, and diff checks are green.

[aviggiano]

Final release-lane blocker fix is at a62d4853.

A current-main runtime-supporting failure exposed a real pre-spawn recovery-deadline race: the last bounded 429 relay could be discarded before replacement command construction discovered that no replacement child could start. The final error remained correct, but final caller stderr lost its request ID. This head retains one bounded prior relay until a replacement outcome is authoritative and releases it only when the replacement deadline wins before spawn.

Validation: forced race 20/20; compound deadline regression 12/12; generated OpenRouter suite 11/11; runtime production/test TypeScript, ESLint, Prettier, build, and diff checks passed. The separate 100 ms synthetic idle-fixture startup race was stabilized without weakening its multi-attempt or final-429 assertions. Independent review found no actionable issue.

Authoritative CI: https://github.com/monad-developers/ultrafuzz/actions/runs/32172764043.