Repository navigation
[APPS-2945] Add: warn on process.env reads in backend functions - #565
Conversation
481364f to
e574841
Compare
1139d7b to
1c99e18
Compare
e574841 to
34a9189
Compare
1c99e18 to
f47f7f2
Compare
34a9189 to
abf34ad
Compare
cd6a25a to
cf3c8c3
Compare
abf34ad to
892bda9
Compare
cf3c8c3 to
47069c8
Compare
bd21790 to
2352fe9
Compare
47069c8 to
7c4b7c7
Compare
e7d5434 to
1fa97d9
Compare
7c4b7c7 to
2f3af66
Compare
e44fe4a to
141d960
Compare
2f3af66 to
10e3990
Compare
141d960 to
04671dd
Compare
10e3990 to
6941e61
Compare
04671dd to
696d422
Compare
6941e61 to
7c83f70
Compare
cd32453 to
a779bfd
Compare
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟡 Changes recommended
Optional-chain deletes incorrectly emit environment-read warnings instead of being excluded like plain deletes.
1 open issue
What changed in this PR
Adds warning-only checks for backend process.env reads, extending the apps plugin’s Node runtime checks to flag values unavailable in production.
Changes:
- Detects direct reads, destructuring, supported imports, and first-level
constaliases, with per-file warning deduplication. - Integrates warnings into shared backend static checks.
- Documents supported forms and adds unit and module-graph coverage.
| File | Description |
|---|---|
packages/plugins/apps/src/vite/dev-server-module-graph.test.ts |
Tests local-run warnings, deduplication, and dependency skips. |
packages/plugins/apps/src/backend/ast-parsing/warn-env-reads.ts |
Implements environment-read detection and warning reporting. |
packages/plugins/apps/src/backend/ast-parsing/warn-env-reads.test.ts |
Tests detection, exclusions, deduplication, and failure handling. |
packages/plugins/apps/src/backend/ast-parsing/run-backend-static-checks.ts |
Invokes the new warning check. |
packages/plugins/apps/src/backend/ast-parsing/run-backend-static-checks.test.ts |
Tests shared-check warning integration. |
packages/plugins/apps/src/backend/ast-parsing/ambient-global-access.ts |
Exports helpers reused by the detector. |
packages/plugins/apps/README.md |
Documents warnings, supported forms, and module exclusions. |
Review effort: Balanced
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
a779bfd to
9e703c8
Compare
9e703c8 to
e55c10f
Compare
b71e121 to
368c9ee
Compare
e55c10f to
c6047fd
Compare
368c9ee to
67fb3ed
Compare
c6047fd to
c910814
Compare
67fb3ed to
539234b
Compare
c910814 to
e02b793
Compare
539234b to
2c6b514
Compare
e02b793 to
44d6ae4
Compare
2c6b514 to
acb744e
Compare
44d6ae4 to
fe3b73b
Compare
The Node backend runtime doesn't pass the developer's environment
variables or Custom Credentials to backend functions, so a
process.env.X read finds nothing in production, and nothing flagged it
before upload.
The static checks now warn, never fail, on direct process.env reads in
app-local backend modules, once per file and use for each plugin
instance, naming the variable when the key is static: process.env.X
and process.env[key], destructures of process.env, env or default
process imports from process or node:process, and a const bound to
process.env or to an env destructured from process. Writes, including
destructuring and for...in/of targets, and deletes don't warn, and
neither does calling an inherited Object.prototype method such as
process.env.hasOwnProperty('X'); any other use of such a name, like
process.env.constructor, reads a variable and warns. Modules a backend
file imports from node_modules or from outside the Vite root are
skipped, so an SDK's own env reads don't warn.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
acb744e to
2f4f385
Compare
fe3b73b to
fa8a098
Compare
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|

Motivation
process.env.Xfinds nothing in production, and nothing flags the gap before upload. (APPS-2945, design)Architecture
flowchart TD T[".backend.* transform"] --> R["createBackendStaticChecks, one per plugin instance (app-local modules only)"] L["local run (module-graph walk)"] --> R V["vite build (nested backend build)"] --> R R --> M["rejectGlobalsMissingFromNode"] R --> W["warnAboutEnvReads"] W -->|"process.env.X, a destructure, an env import, a const alias"| K["warn: process.env.X"] W -->|"process.env[key], a rest destructure"| A["warn: process.env"] W -->|"writes, deletes, indirect forms"| N["no warning"]Changes
7 changes across warn-env-reads.ts, ambient-global-access.ts, run-backend-static-checks.ts, the Vite call sites, README.md and tests
warnAboutEnvReads, which warns on directprocess.envreads, naming static keys, but not on a call of an inheritedObject.prototypemethod such asprocess.env.hasOwnProperty('X').envdestructures, destructured parameter defaults andprocess.env ?? {}fallbacks.warn-env-reads.tsambient-global-access.tsrun-backend-static-checks.tsindex.ts,dev-server.ts,dev-server-module-graph.ts,build-backend-functions.ts,backend-static-checks-plugin.tsREADME.mdwarn-env-reads.test.ts,dev-server-module-graph.test.ts,index.test.tsQA Instructions
44d6ae49, as do the two local-run checks against a Datadog staging org, using the helper scripts from [APPS-2945] Update: switch the apps plugin's static checks to the Node backend runtime #557 and a Vite 8 scaffold.vite buildimport.meta.env,hasOwnProperty,globalThis.processor anode_modulesSDK;config.backend.tswarns on two linesvite dev, each module requested twice, then editeduseFetch; reads return nullSTRIPE_KEYset, local anddev:verifyRuns 1 and 2: fixtures, run commands and output
src/useEnv.backend.ts:src/useRestEnv.backend.ts:Run 3:
vite buildandvite devfixture and outputvite.config.mjsisexport default { plugins: [datadogVitePlugin({ apps: { enable: true } })] }, importing the built plugin from this branch'spackages/published/vite-plugin/dist/src/index.mjs;src/main.tsimports both functions,src/helper.tsisexport const helperKey = () => process.env.HELPER_KEY ?? null;, andnode_modules/fake-sdk/index.jsisexport const sdkKey = () => process.env.SDK_KEY ?? null;.src/billing.backend.ts:src/config.backend.ts:Blast Radius
process.envdirectly (NODE_ENVincluded) get newwarn-level log lines in builds and the dev server.Out of Scope / Follow-ups
3 items deferred
letaliases, dynamic imports, spreads andglobalThis.processNODE_ENVand keys the file assigns itself still warnNODE_ENVunset toovite build, Vite'sdefinerewritesprocess.envbefore the transform-hook check, so a file mixingprocess.env.Xwith forms likeproc.env.Xwarns on two linesDocumentation
🤖 Generated with Claude Code