[APPS-2792] Add: scope process.env to an allowlist during local execution (Secret Store parity)

[tyffical]

Motivation

• Part of APPS-2792 — local Node execution for App Builder backend functions, closing the Secret Store parity gap described in the Kickoff doc. Stacked on the sandboxing runtime guard.
• Backend functions run in-process inside the Vite dev server rather than a fresh Deno subprocess, so a function reading process.env directly would otherwise see the dev server's entire real environment — its own DD_API_KEY/DD_APP_KEY credentials plus the developer's unrelated shell secrets. Production has no equivalent gap: each execution gets a fresh Deno subprocess with --allow-env scoped to the resolved credential names.

Architecture

runScriptLocally()
   │
   ├─ runWithScopedEnv(buildScopedEnv({}), () =>
   │     runBlocked(() => fn(...args))      ← network-guard, unchanged
   │  )                                      ← env-guard, new
   │
   │  buildScopedEnv({}) returns only { PATH, HOME, NODE_ENV, TMPDIR }
   │  (whichever are set in the real env) — never a copy of process.env.
   │
   ▼
customer function body executes with the scoped env only
   │
   ▼ (success, throw, or timeout)
real process.env restored — via runWithScopedEnv's own finally, or
forceResetEnv() in the timeout handler if fn never settles

env-guard.ts mirrors network-guard.ts's generation-counter design: a monotonic currentGeneration, bumped by every runWithScopedEnv call and by forceResetEnv, so a late restore from an abandoned (timed-out) execution can't clobber a newer execution's still-active scoped window.

Changes

6 changes across 4 files

What changed	File
New buildScopedEnv/runWithScopedEnv/forceResetEnv, scoping process.env to PATH/HOME/NODE_ENV/TMPDIR for the duration of a local execution.	env-guard.ts
Unit coverage: allowlist filtering, Custom Credentials merge point, restore-on-throw, and the abandoned/newer-execution race (mirrors network-guard.test.ts's equivalent test).	env-guard.test.ts
Wired runWithScopedEnv/buildScopedEnv around the customer function call in runScriptLocally, nested with the existing runBlocked network guard. Added forceResetEnv() alongside the existing forceReset() in the timeout handler.	local-execution.ts
Integration tests through executeScriptLocally: DD_API_KEY and an AWS-like credential are never visible to the customer function; PATH/HOME/NODE_ENV/TMPDIR remain visible; real process.env is restored after both success and throw.	local-execution.test.ts
Extended the /proc/.../environ guard to createReadStream/open/openSync/promises.open and normalized Buffer/URL path arguments — the original guard only covered readFile* and string paths.	env-guard.ts
process.report.getReport()/writeReport() strip environmentVariables at the JS level during an active scoped-env window, on every Node version this repo supports — process.report.excludeEnv (the native flag) is only wired up to Node's report generator from v22.0.0 onward, so relying on it alone leaks the real environment through on CI's pinned Node 20.19.4.	env-guard.ts, env-guard.test.ts

Two deliberate scope decisions, both documented in the RFC's Security section:

1. The safe allowlist is intentionally narrow (PATH/HOME/NODE_ENV/TMPDIR only) — it excludes build-plugins' own OVERRIDE_VARIABLES (DD_API_KEY, DD_APP_KEY, DD_SITE, etc.), which are the dev server's own credentials and must never reach customer code.
2. Custom Credentials resolution (e.g. a declared STRIPE_API_KEY) is an open, undecided question — buildScopedEnv is always called with {} for now, matching production's "skip on unresolvable, never fail the task" behavior. The parameter is a forward-looking extension point only; no local-override mechanism exists yet.

QA Instructions

yarn install

yarn test:unit packages/plugins/apps/src/vite/env-guard.test.ts packages/plugins/apps/src/vite/local-execution.test.ts
# Expected: Test Suites: 2 passed / Tests: 118 passed ✅ VERIFIED

yarn build:all && yarn test:unit
# Expected: Test Suites: 92 passed / Tests: 2259 passed, 1 skipped ✅ VERIFIED

yarn workspace @dd/apps-plugin run typecheck
# Expected: no output, clean exit ✅ VERIFIED

npx eslint 'packages/plugins/apps/**/*.ts' --quiet
# Expected: no output, clean exit ✅ VERIFIED

Real manual QA against a scaffolded app, per the Testing and QA Guide:

# npm create @datadog/apps@latest -- qa-test-app --template vite-react -y, npm link @datadog/vite-plugin,
# add a backend function reading process.env.DD_API_KEY/AWS_SECRET_ACCESS_KEY/PATH, npm run dev
curl -X POST http://localhost:5180/__dd/executeAction -d '{"functionName":"...checkEnv","args":[]}'
# {"success":true,"result":{"data":{"apiKeyVisible":false,"awsKeyVisible":false,"path":"visible"}}} ✅ VERIFIED

Confirmed with the real dev server process holding DD_API_KEY/AWS_SECRET_ACCESS_KEY in its actual environment (via DD_API_KEY=... AWS_SECRET_ACCESS_KEY=... npm run dev) — neither leaked to the customer function, PATH stayed visible.

Blast Radius

• No behavior change to npm run dev:verify's cloud path — this only affects the in-process local-execution branch.
• Risk: low-medium. This is a real security boundary (the only thing standing between customer code and the dev server's own credentials), but it's purely additive, well-isolated to runScriptLocally, and follows an already-reviewed pattern (network-guard.ts).

Out of Scope / Follow-ups

2 items deferred

Item	Status	Next step
Custom Credentials real resolution (e.g. proxying through $.Actions's own channel)	Open, undecided	Explicit design decision needed before treating Secret Store parity as fully closed — see RFC's "Open tension, not yet resolved" row
process.env scoping is a global swap, not chain-scoped like network-guard.ts's AsyncLocalStorage-based blocking — a fire-and-forget setTimeout in customer code can observe the real, unscoped environment once the outer call's finally has already restored it	Confirmed via code review, not yet fixed	Needs a Proxy-based process.env design (values resolved per active AsyncLocalStorage scope, not a single swapped object) — bigger lift than network-guard.ts's equivalent fix, since arbitrary code reads process.env directly rather than through an interceptable function

Documentation

• RFC: Confluence page 7012712651 — Security section, Secret Store parity
• Kickoff doc: Confluence page 7025394455 — Secret Store parity
• QA guide: Confluence page 7100498075 — exact local/staging QA commands for this repo

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2311dbe9e4

ℹ️ 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".

[chatgpt-codex-connector]

Scope the environment while evaluating customer modules

When a backend module reads an environment variable at module scope, loadModule(...) executes that customer code before runWithScopedEnv is entered, so code such as const key = process.env.DD_API_KEY can retain the dev server credential and return or use it later from the guarded function. The environment must be scoped during customer-module evaluation as well, even if Vite's own transform work needs a separate unscoped phase.

Useful? React with 👍 / 👎.

[tyffical]

Same underlying cause as the analogous, already-accepted network-guard gap in local-execution.ts (a customer module's own top-level code runs before any guard is entered, since loadModule needs real network/fs access for Vite's own transform pipeline — see that file's own doc comment). Confirmed real for env-guard too: a module-scope const key = process.env.DD_API_KEY would see the real value. Same fix shape as the network-guard case would need (scoping before loadModule, which previously broke the real dev-server integration test outright when tried for network-guard) — not attempting a different fix here without re-litigating that same tradeoff. Flagging alongside the post-timeout gap above for the #security review.

[chatgpt-codex-connector]

Do not expose the real environment after a timeout

When a customer function continues after exceeding timeoutMs (for example, it awaits a timer and then resumes), the timeout wins the race but does not cancel that function; this reset consequently exposes the real process.env to the still-running customer continuation. Such a function can simply wait past the timeout and then read DD_API_KEY, AWS credentials, or any other shell secret, defeating the new isolation guarantee.

Useful? React with 👍 / 👎.

[tyffical]

Confirmed real, and not something this PR can close on its own. It's the same root cause as the RFC's already-accepted 'abandoned, not cancelled' execution model (Decisions and Trade-Offs, and Milestone 7's own confirmed findings): a timed-out function isn't cancelled, just abandoned, so forceResetEnv() restoring the real env for the next execution necessarily also restores it underneath the still-running old one. Closing this fully needs either true per-execution process isolation or an AsyncLocalStorage-based proxy over process.env (replacing the whole-object-swap approach here with per-access-context resolution) — both bigger than this milestone's scope, and the latter has the same category of complexity the RFC already deferred for action-catalog/apps-backend's own registration scoping. Flagging this explicitly as a residual gap for the Secret Store parity #security review this milestone already requires, rather than leaving it implicit.

[Copilot]

Pull request overview

Friend, this PR scopes environment variables during local backend-function execution to reduce secret exposure.

Changes:

• Adds an environment allowlist guard and reset handling.
• Integrates the guard with local execution and timeouts.
• Adds unit and integration coverage.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
env-guard.ts	Implements environment scoping and restoration.
env-guard.test.ts	Tests filtering, restoration, and generation handling.
local-execution.ts	Applies and resets the environment guard.
local-execution.test.ts	Tests local-execution environment behavior.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[tyffical]

This consolidates three points already answered individually on other threads:

• Module-scope leak (customer init runs before this scope) → confirmed real, same root cause as the network-guard's analogous gap; see reply on the module-init-leak thread.
• Post-timeout continuation exposure → confirmed real, same "abandoned, not cancelled" execution model as the RFC's accepted trade-off; see reply on the post-timeout thread.
• Returned object's getter/toJSON running after this scope during serialization → not an open gap. assertJsonSerializable runs inside runWithScopedEnv's callback (local-execution.ts:701-706), so a malicious toJSON()/getter fires while the scoped env is still active, not after restoration.

No new action beyond the two already-tracked, already-accepted gaps above.

[tyffical]

Same root question as the analogous finding on PR #484's local-execution.ts (network-guard side) — traced through carefully and it doesn't currently reproduce, for the same reason: the abandoned check runs synchronously with no await before either guard is entered. See my reply on #484 for the regression test that proves it end-to-end through both runBlocked and runWithScopedEnv together (they're nested in the same call).

[datadog-prod-us1-4]

✅ All CI checks and tests passed.

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

_{This comment will be updated automatically if new data arrives.
🔗 Commit SHA: d0371c5 | Docs | View more details | Give us feedback!}

[tyffical]

@codex review

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-01T18:15:07.688927Z	b511d0d	Manual request

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b511d0d4dd

ℹ️ 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".

[Copilot]

Pull request overview

Copilot reviewed 34 out of 36 changed files in this pull request and generated 5 comments.

[yoannmoinet]

Not sure where you got this information, but excludeEnv was introduced in v22.13.0 ref

[tyffical]

Confirmed, thanks — verified via nodejs/node#55697: excludeEnv landed in v22.13.0. This file's comment and env-guard.ts's own matching comment (which said v22.0.0, also wrong) are fixed locally. Neither had any behavioral impact since the code already treats it as unavailable below its actual threshold via the CI-pinned-Node-20.19.4 fallback path — this was a comment-accuracy fix only.