feat: add Netlify function env guard plugin#78
Conversation
Add root package.json and compare Infisical staging /tuj keys against Netlify production env via API instead of the GHA runner process.env. Co-authored-by: Cursor <cursoragent@cursor.com>
Prevent Lambda deploy failures by trimming non-function env vars before the 4KB payload limit. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Want AI-powered code review for this PR? Get instant analysis, interactive visualizations, and actionable insights. |
✅ Deploy Preview for corporate-games-tech-app-production-target-primary ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis PR rewrites the drift-check script to compare Infisical secrets against Netlify environment variables using a new parity manifest, updates the CI workflow and package/tsconfig files to run it, and adds a Netlify build plugin that trims environment variables to avoid exceeding Lambda payload limits, plus netlify.toml/gitignore updates. ChangesDrift-check tooling
Estimated code review effort: 4 (Complex) | ~55 minutes Netlify config and function env guard plugin
Sequence Diagram(s)sequenceDiagram
participant Workflow as GitHub Actions
participant Script as runDriftCheck
participant Infisical
participant Netlify
Workflow->>Script: npm run drift-check
Script->>Infisical: fetchInfisicalKeys()
Infisical-->>Script: secret keys
Script->>Netlify: fetchNetlifyKeys()
Netlify-->>Script: env var keys
Script->>Script: compute missingOnNetlify / missingInInfisical
Script->>Workflow: reportDrift() -> exit code or warning
sequenceDiagram
participant Netlify as Netlify Build
participant Plugin as env-guard onPostBuild
participant Env as process.env
Netlify->>Plugin: onPostBuild({ utils })
Plugin->>Env: enumerate entries
Plugin->>Env: delete build-only/platform vars
Plugin->>Plugin: estimate function payload size
alt payload above target
Plugin->>Env: delete largest trim candidates
end
alt payload still above hard limit
Plugin->>Netlify: utils.build.failBuild()
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
plugins/netlify-plugin-function-env-guard/index.js (1)
4-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded allowlist duplicates the drift-check parity manifest — risk of the two lists diverging.
Per the stack context,
scripts/parity-manifest.tsalready defines the canonical set of expected Netlify env var keys (P0_KEYS,P1_KEYS,ALL_MANIFEST_KEYS) used by the drift-check tooling. This file re-declares a nearly-parallel, independently-maintained list (FUNCTION_ENV_NAMES). Any future key addition/removal now has to be kept in sync in two places by hand, which is exactly the kind of drift this PR's drift-check tooling is meant to catch — but this list sits outside that system entirely. Consider extracting the shared key list into a plain JS/JSON module both the TypeScript parity manifest and this CommonJS plugin can import, to keep a single source of truth.🤖 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 `@plugins/netlify-plugin-function-env-guard/index.js` around lines 4 - 93, The hardcoded allowlist in FUNCTION_ENV_NAMES duplicates the canonical Netlify env key set already defined in scripts/parity-manifest.ts, so the two sources can drift apart. Refactor the plugin to consume a shared single source of truth for the env keys, ideally by extracting the manifest keys into a plain JS/JSON module that both the parity-manifest logic and the Netlify plugin can import. Keep the plugin’s validation behavior the same, but remove the independently maintained list and reference the shared keys from the plugin entrypoint in index.js.
🤖 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 `@netlify.toml`:
- Around line 33-39: The redirect rule is using an invalid hostname field and
therefore won’t match only the www origin. Update the redirect entry in
netlify.toml to scope the rule via the full origin in the from field, and keep
the existing destination and 301 force behavior intact. Use the [[redirects]]
block to locate the rule and remove the hostname-based scoping from that
redirect.
In `@package.json`:
- Line 2: The package name in package.json looks like a placeholder and should
be replaced with the real project name. Update the "name" field from
"thispagedoesnotexist12345" to the intended package identifier used for this
repository, keeping it consistent with the project’s published or internal
naming.
- Around line 9-15: The Node runtime and typings are out of sync with the
supported target: update the drift-check workflow to use a supported LTS runtime
(22 or 24), align package.json’s `@types/node` version to that same major, and add
an engines field to make the supported Node version explicit. Use the existing
drift-check workflow and package.json devDependencies as the places to update,
and keep the runtime, types, and declared engine version consistent.
In `@plugins/netlify-plugin-function-env-guard/index.js`:
- Around line 95-133: The payload size guard is misclassifying allowlisted
function env vars as platform vars because `isPlatformEnv` uses `startsWith` for
broad prefixes like `NETLIFY` and `SITE_`. Update
`shouldExcludeFromFunctionPayload` and/or `isPlatformEnv` so
`FUNCTION_ENV_NAMES` entries such as `NETLIFY_API_KEY`, `NETLIFY_SITE_ID`, and
`SITE_URL` are counted in `envByteSize` instead of being excluded. Keep
`shouldDeleteFromEnvironment` behavior aligned so these allowlisted names are
neither deleted nor skipped from size accounting.
- Around line 143-151: The size accounting in the environment pruning logic is
using a stale snapshot of process.env, which makes initialSize and trimming
decisions inaccurate. Recompute the entries from the current process.env after
the deletion loop in the guard flow, then derive functionEntries from that fresh
snapshot before calling envByteSize and the trim logic. Update the code around
Object.entries(process.env), shouldDeleteFromEnvironment,
shouldExcludeFromFunctionPayload, and envByteSize so the byte estimate only
reflects variables that still exist.
- Around line 141-186: The environment trimming logic in onPostBuild is too late
to affect the packaged function payload, since process.env is already consumed
by Functions bundling. Move the logic into onBuild in module.exports and mutate
netlifyConfig.build.environment instead of deleting from process.env, using the
same shouldDeleteFromEnvironment, shouldExcludeFromFunctionPayload, canTrim, and
envByteSize flow to identify which variables should be removed before bundling.
In `@scripts/drift-check.ts`:
- Line 2: The drift-check script is recomputing the manifest key list inline
instead of reusing the shared ALL_MANIFEST_KEYS export, which can drift from
parity-manifest.ts over time. Update the imports in scripts/drift-check.ts to
use ALL_MANIFEST_KEYS from parity-manifest.ts and replace the local
deduplication logic in the drift comparison code with that shared symbol so both
paths stay consistent.
- Around line 62-89: `runDriftCheck`/`reportDrift` currently treat all manifest
keys with one global `STRICTNESS`, so P0 keys can be downgraded to warnings on
non-tracked branches. Update the drift-check flow to preserve severity per key
group: keep `P0_KEYS` as always-fail checks and apply branch-based strictness
only to `P1_KEYS`, or otherwise split the reporting so `reportDrift` can emit a
fatal error whenever any P0 key is missing. Use the existing `P0_KEYS`,
`P1_KEYS`, `runDriftCheck`, and `reportDrift` symbols to wire the two-tier logic
through the manifest-key aggregation and error handling.
---
Nitpick comments:
In `@plugins/netlify-plugin-function-env-guard/index.js`:
- Around line 4-93: The hardcoded allowlist in FUNCTION_ENV_NAMES duplicates the
canonical Netlify env key set already defined in scripts/parity-manifest.ts, so
the two sources can drift apart. Refactor the plugin to consume a shared single
source of truth for the env keys, ideally by extracting the manifest keys into a
plain JS/JSON module that both the parity-manifest logic and the Netlify plugin
can import. Keep the plugin’s validation behavior the same, but remove the
independently maintained list and reference the shared keys from the plugin
entrypoint in index.js.
🪄 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: 0dd0033c-acef-46f3-aeda-75cf6cc54e98
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
.github/workflows/drift-check.yml.gitignorenetlify.tomlpackage.jsonplugins/netlify-plugin-function-env-guard/index.jsplugins/netlify-plugin-function-env-guard/manifest.ymlplugins/netlify-plugin-function-env-guard/package.jsonscripts/drift-check.tsscripts/parity-manifest.tstsconfig.json
| # Bare-domain → www canonical redirects (HTTP + HTTPS) | ||
| [[redirects]] | ||
| from = "http://thispagedoesnotexist12345.com/*" | ||
| to = "https://www.thispagedoesnotexist12345.com/:splat" | ||
| status = 301 | ||
| force = true | ||
|
|
||
| [[redirects]] | ||
| from = "https://thispagedoesnotexist12345.com/*" | ||
| to = "https://www.thispagedoesnotexist12345.com/:splat" | ||
| hostname = "www.corporate-games.tech" | ||
| from = "/*" | ||
| to = "https://corporate-games.tech/:splat" | ||
| status = 301 | ||
| force = true |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## netlify.toml excerpt\n'
nl -ba netlify.toml | sed -n '25,45p'
printf '\n## search for redirect-related keys\n'
rg -n 'hostname|redirects|from =|to =' netlify.tomlRepository: palmtr3man/thispagedoesnotexist12345
Length of output: 239
🌐 Web query:
Netlify netlify.toml redirects hostname key documented from full origin hostname ignored
💡 Result:
There is no "hostname" key in the Netlify netlify.toml redirect configuration syntax [1][2]. Netlify's official documentation for [[redirects]] does not support a hostname property to filter or match traffic based on the origin hostname [1][2]. If you are attempting to redirect based on a specific hostname (domain), you should include the full URL (including the protocol and hostname) within the from property of your redirect rule [3]. For example: [[redirects]] from = "https://example.com/*" to = "https://destination.com/:splat" status = 301 force = true If your redirects are being ignored, it is typically due to one of the following reasons: 1. Precedence and File Shadowing: Redirects in a _redirects file (located in your publish directory) are processed before those in netlify.toml [4]. If a rule in _redirects or a system-generated rule (e.g., from a framework like Next.js or Nuxt) matches the path first, subsequent rules may never be evaluated [5][4]. 2. Framework Runtime Overrides: If you are using a framework with a specialized runtime (like Next.js), the framework may generate its own internal redirects and rewrites that take precedence over your netlify.toml rules [5][3]. 3. Syntax Issues: Ensure you are using force = true if you need the redirect to override existing files (preventing file shadowing) [2][6]. If you suspect your rules are being ignored due to host-based matching, ensure you are specifying the full origin in the from field as shown above, rather than attempting to use an unsupported hostname key [3].
Citations:
- 1: https://docs.netlify.com/manage/routing/redirects/redirect-options/
- 2: https://docs.netlify.com/build/configure-builds/file-based-configuration/
- 3: https://answers.netlify.com/t/issues-with-conditional-redirects-in-netlify-toml-and-next-js-runtime/126014
- 4: https://answers.netlify.com/t/headers-and-or-redirects-not-working-in-netlify-toml/88379
- 5: https://docs.netlify.com/build/frameworks/framework-setup-guides/nextjs/legacy-runtime/redirects-and-rewrites
- 6: https://www.netlify.com/blog/2020/04/07/creating-better-more-predictable-redirect-rules-for-spas/
Use the full origin in from
hostname isn’t a valid [[redirects]] field here, so this rule won’t scope to www.corporate-games.tech. Use the full URL in from instead.
Proposed fix
[[redirects]]
- hostname = "www.corporate-games.tech"
- from = "/*"
+ from = "https://www.corporate-games.tech/*"
to = "https://corporate-games.tech/:splat"
status = 301
force = true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Bare-domain → www canonical redirects (HTTP + HTTPS) | |
| [[redirects]] | |
| from = "http://thispagedoesnotexist12345.com/*" | |
| to = "https://www.thispagedoesnotexist12345.com/:splat" | |
| status = 301 | |
| force = true | |
| [[redirects]] | |
| from = "https://thispagedoesnotexist12345.com/*" | |
| to = "https://www.thispagedoesnotexist12345.com/:splat" | |
| hostname = "www.corporate-games.tech" | |
| from = "/*" | |
| to = "https://corporate-games.tech/:splat" | |
| status = 301 | |
| force = true | |
| [[redirects]] | |
| from = "https://www.corporate-games.tech/*" | |
| to = "https://corporate-games.tech/:splat" | |
| status = 301 | |
| force = true |
🤖 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 `@netlify.toml` around lines 33 - 39, The redirect rule is using an invalid
hostname field and therefore won’t match only the www origin. Update the
redirect entry in netlify.toml to scope the rule via the full origin in the from
field, and keep the existing destination and 301 force behavior intact. Use the
[[redirects]] block to locate the rule and remove the hostname-based scoping
from that redirect.
| @@ -0,0 +1,16 @@ | |||
| { | |||
| "name": "thispagedoesnotexist12345", | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Placeholder-looking package name.
"thispagedoesnotexist12345" reads like a leftover placeholder rather than an intentional project name.
🤖 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 `@package.json` at line 2, The package name in package.json looks like a
placeholder and should be replaced with the real project name. Update the "name"
field from "thispagedoesnotexist12345" to the intended package identifier used
for this repository, keeping it consistent with the project’s published or
internal naming.
| "devDependencies": { | ||
| "@infisical/sdk": "^5.0.2", | ||
| "@types/node": "^26.0.1", | ||
| "tsx": "^4.22.4", | ||
| "typescript": "^5.9.3", | ||
| "vitest": "^3.2.4" | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## package.json\n'
cat -n package.json
printf '\n## .github/workflows/drift-check.yml\n'
cat -n .github/workflows/drift-check.ymlRepository: palmtr3man/thispagedoesnotexist12345
Length of output: 1865
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## scripts/drift-check.ts\n'
cat -n scripts/drift-check.ts
printf '\n## tsconfig.json\n'
cat -n tsconfig.json 2>/dev/null || true
printf '\n## package lock references to `@types/node`\n'
rg -n '"`@types/node`"|node-version|engines' -S package.json .github/workflows scripts tsconfig.json README.md 2>/dev/null || trueRepository: palmtr3man/thispagedoesnotexist12345
Length of output: 5423
🌐 Web query:
Node.js release schedule Node 20 end of life official schedule July 2026
💡 Result:
Node.js 20 reached its official End-of-Life (EOL) on April 30, 2026 [1][2][3]. As of today, July 2, 2026, the Node.js project no longer provides security patches, bug fixes, or any other support for the v20 release line [1][3]. Users are encouraged to migrate to a supported version of Node.js [1].
Citations:
- 1: https://nodejs.org/en/about/eol
- 2: https://github.com/nodejs/Release/blob/main/README.md
- 3: https://www.herodevs.com/blog-posts/node-js-end-of-life-dates-you-should-be-aware-of
drift-check is pinned to EOL Node 20. Bump .github/workflows/drift-check.yml to a supported LTS (22 or 24) and align package.json’s @types/node to the chosen runtime; add engines to make the target explicit.
🤖 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 `@package.json` around lines 9 - 15, The Node runtime and typings are out of
sync with the supported target: update the drift-check workflow to use a
supported LTS runtime (22 or 24), align package.json’s `@types/node` version to
that same major, and add an engines field to make the supported Node version
explicit. Use the existing drift-check workflow and package.json devDependencies
as the places to update, and keep the runtime, types, and declared engine
version consistent.
| const PLATFORM_PREFIXES = [ | ||
| 'AWS_', | ||
| 'BRANCH', | ||
| 'BUILD_', | ||
| 'COMMIT_', | ||
| 'CONTEXT', | ||
| 'DEPLOY_', | ||
| 'NETLIFY', | ||
| 'NODE_', | ||
| 'NPM_', | ||
| 'PATH', | ||
| 'PWD', | ||
| 'REPOSITORY_', | ||
| 'SITE_', | ||
| 'URL', | ||
| ]; | ||
|
|
||
| const BUILD_ONLY_ENV_NAMES = new Set([ | ||
| 'SECRETS_SCAN_OMIT_KEYS', | ||
| 'SECRETS_SCAN_OMIT_PATHS', | ||
| ]); | ||
|
|
||
| function envByteSize(entries) { | ||
| return entries.reduce((total, [key, value]) => total + Buffer.byteLength(`${key}=${value || ''}`), 0); | ||
| } | ||
|
|
||
| function isPlatformEnv(name) { | ||
| return PLATFORM_PREFIXES.some((prefix) => name === prefix || name.startsWith(prefix)); | ||
| } | ||
|
|
||
| function shouldExcludeFromFunctionPayload(name) { | ||
| return BUILD_ONLY_ENV_NAMES.has(name) || isPlatformEnv(name); | ||
| } | ||
|
|
||
| function shouldDeleteFromEnvironment(name) { | ||
| if (BUILD_ONLY_ENV_NAMES.has(name)) return true; | ||
| if (isPlatformEnv(name)) return false; | ||
| return !FUNCTION_ENV_NAMES.has(name); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
PLATFORM_PREFIXES collides with allowlisted FUNCTION_ENV_NAMES entries, silently excluding required function vars from size accounting.
isPlatformEnv matches by startsWith, and prefixes 'NETLIFY' and 'SITE_' also match NETLIFY_API_KEY, NETLIFY_SITE_ID (lines 36-37), and SITE_URL (line 89) — all explicitly required function env vars. Because shouldExcludeFromFunctionPayload checks isPlatformEnv without first checking FUNCTION_ENV_NAMES membership, these three vars are excluded from the payload size calculation (undercounting the real byte total) while also not being deleted (shouldDeleteFromEnvironment returns false for platform-classified names) — so they still ship to the function, uncounted. This further weakens the accuracy of the guard's threshold check.
🐛 Proposed fix
function isPlatformEnv(name) {
+ if (FUNCTION_ENV_NAMES.has(name)) return false;
return PLATFORM_PREFIXES.some((prefix) => name === prefix || name.startsWith(prefix));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const PLATFORM_PREFIXES = [ | |
| 'AWS_', | |
| 'BRANCH', | |
| 'BUILD_', | |
| 'COMMIT_', | |
| 'CONTEXT', | |
| 'DEPLOY_', | |
| 'NETLIFY', | |
| 'NODE_', | |
| 'NPM_', | |
| 'PATH', | |
| 'PWD', | |
| 'REPOSITORY_', | |
| 'SITE_', | |
| 'URL', | |
| ]; | |
| const BUILD_ONLY_ENV_NAMES = new Set([ | |
| 'SECRETS_SCAN_OMIT_KEYS', | |
| 'SECRETS_SCAN_OMIT_PATHS', | |
| ]); | |
| function envByteSize(entries) { | |
| return entries.reduce((total, [key, value]) => total + Buffer.byteLength(`${key}=${value || ''}`), 0); | |
| } | |
| function isPlatformEnv(name) { | |
| return PLATFORM_PREFIXES.some((prefix) => name === prefix || name.startsWith(prefix)); | |
| } | |
| function shouldExcludeFromFunctionPayload(name) { | |
| return BUILD_ONLY_ENV_NAMES.has(name) || isPlatformEnv(name); | |
| } | |
| function shouldDeleteFromEnvironment(name) { | |
| if (BUILD_ONLY_ENV_NAMES.has(name)) return true; | |
| if (isPlatformEnv(name)) return false; | |
| return !FUNCTION_ENV_NAMES.has(name); | |
| } | |
| const PLATFORM_PREFIXES = [ | |
| 'AWS_', | |
| 'BRANCH', | |
| 'BUILD_', | |
| 'COMMIT_', | |
| 'CONTEXT', | |
| 'DEPLOY_', | |
| 'NETLIFY', | |
| 'NODE_', | |
| 'NPM_', | |
| 'PATH', | |
| 'PWD', | |
| 'REPOSITORY_', | |
| 'SITE_', | |
| 'URL', | |
| ]; | |
| const BUILD_ONLY_ENV_NAMES = new Set([ | |
| 'SECRETS_SCAN_OMIT_KEYS', | |
| 'SECRETS_SCAN_OMIT_PATHS', | |
| ]); | |
| function envByteSize(entries) { | |
| return entries.reduce((total, [key, value]) => total + Buffer.byteLength(`${key}=${value || ''}`), 0); | |
| } | |
| function isPlatformEnv(name) { | |
| if (FUNCTION_ENV_NAMES.has(name)) return false; | |
| return PLATFORM_PREFIXES.some((prefix) => name === prefix || name.startsWith(prefix)); | |
| } | |
| function shouldExcludeFromFunctionPayload(name) { | |
| return BUILD_ONLY_ENV_NAMES.has(name) || isPlatformEnv(name); | |
| } | |
| function shouldDeleteFromEnvironment(name) { | |
| if (BUILD_ONLY_ENV_NAMES.has(name)) return true; | |
| if (isPlatformEnv(name)) return false; | |
| return !FUNCTION_ENV_NAMES.has(name); | |
| } |
🤖 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 `@plugins/netlify-plugin-function-env-guard/index.js` around lines 95 - 133,
The payload size guard is misclassifying allowlisted function env vars as
platform vars because `isPlatformEnv` uses `startsWith` for broad prefixes like
`NETLIFY` and `SITE_`. Update `shouldExcludeFromFunctionPayload` and/or
`isPlatformEnv` so `FUNCTION_ENV_NAMES` entries such as `NETLIFY_API_KEY`,
`NETLIFY_SITE_ID`, and `SITE_URL` are counted in `envByteSize` instead of being
excluded. Keep `shouldDeleteFromEnvironment` behavior aligned so these
allowlisted names are neither deleted nor skipped from size accounting.
| module.exports = { | ||
| onPostBuild({ utils }) { | ||
| const entries = Object.entries(process.env); | ||
| for (const [name] of entries) { | ||
| if (shouldDeleteFromEnvironment(name)) { | ||
| delete process.env[name]; | ||
| } | ||
| } | ||
|
|
||
| const functionEntries = entries.filter(([name]) => !shouldExcludeFromFunctionPayload(name)); | ||
| const initialSize = envByteSize(functionEntries); | ||
|
|
||
| if (initialSize <= TARGET_MAX_BYTES) { | ||
| console.log(`[function-env-guard] Environment payload is ${initialSize} bytes; no trimming needed.`); | ||
| return; | ||
| } | ||
|
|
||
| const candidates = functionEntries | ||
| .filter(([name]) => canTrim(name)) | ||
| .map(([name, value]) => ({ | ||
| name, | ||
| size: Buffer.byteLength(`${name}=${value || ''}`), | ||
| })) | ||
| .sort((a, b) => b.size - a.size); | ||
|
|
||
| let currentSize = initialSize; | ||
| let trimmedCount = 0; | ||
|
|
||
| for (const candidate of candidates) { | ||
| if (currentSize <= TARGET_MAX_BYTES) break; | ||
| currentSize -= candidate.size; | ||
| delete process.env[candidate.name]; | ||
| trimmedCount += 1; | ||
| } | ||
|
|
||
| console.log( | ||
| `[function-env-guard] Trimmed ${trimmedCount} non-function environment variable(s); estimated payload ${currentSize} bytes.` | ||
| ); | ||
|
|
||
| if (currentSize > BYTE_LIMIT) { | ||
| utils.build.failBuild( | ||
| `Function environment is still above AWS Lambda's ${BYTE_LIMIT} byte limit after trimming non-function variables. Move large required function values to function-scoped Netlify environment variables or an external secret store.` | ||
| ); | ||
| } | ||
| }, | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file with line numbers and surrounding context.
wc -l plugins/netlify-plugin-function-env-guard/index.js
sed -n '1,260p' plugins/netlify-plugin-function-env-guard/index.js
# Search for related Netlify build-plugin or environment usage in the repo.
rg -n "netlifyConfig\.build\.environment|onPostBuild|onBuild|function-env-guard|process\.env" .
# List likely docs/config files.
git ls-files | rg '(^|/)(netlify\.toml|README\.md|docs/|plugins/|package\.json$)'Repository: palmtr3man/thispagedoesnotexist12345
Length of output: 31738
🌐 Web query:
Netlify build plugin lifecycle onBuild onPostBuild Functions bundling process.env netlifyConfig.build.environment
💡 Result:
The Netlify Build plugin lifecycle is a sequence of events triggered during a site's build process [1][2]. Understanding the interaction between lifecycle stages and functions is essential for tasks like dynamic function generation [3][4]. Lifecycle Events The relevant lifecycle events occur in the following order [1]: 1. onPreBuild: Runs before the build command [1][5]. 2. onBuild: Runs after the build command completes but before the bundling of Functions and Edge Handlers [1]. 3. Functions Bundling: This internal stage processes and packages your site's functions (typically using tools like esbuild) [1][6]. 4. onPostBuild: Runs after the build command, after Functions bundling, and after Edge Handlers bundling are finished, but before the final deploy stage [7][1]. Accessing Environment Variables You can access environment variables within your build plugins using two primary methods [8][9]: - process.env: This is the recommended approach for reading any environment variables (including those from the Netlify UI or netlify.toml) during the build process [9]. It behaves like a standard Node.js environment [10]. - netlifyConfig.build.environment: This object contains a subset of a project’s environment variables, specifically those declared by the user in the Netlify UI or netlify.toml [9]. It is recommended when you need to programmatically modify these values during the build [9][11]. Functions Bundling and Utilities Functions bundling is an automated step in the Netlify build lifecycle [6]. If you need to dynamically inject functions into your build, you can use the built-in functions utility available in your plugin's event handlers [3][4]. By using utils.functions.add(path), you can add function files or directories during the build, which will then be picked up and bundled by the Netlify build system [3][4]. These injected functions are generally processed alongside your project's other functions [12].
Citations:
- 1: https://www.netlify.com/blog/2021/12/27/do-more-at-build-time-with-build-plugins/
- 2: https://darn.es/building-a-netlify-build-plugin/
- 3: Add
functionscore utility netlify/build#644 - 4: https://www.npmjs.com/package/@netlify/functions-utils
- 5: https://www.netlify.com/blog/2019/10/16/creating-and-using-your-first-netlify-build-plugin/
- 6: https://www.netlify.com/blog/2021/04/02/modern-faster-netlify-functions/
- 7: https://docs.netlify.com/extend/develop-and-share/develop-build-plugins/
- 8: https://docs.netlify.com/build/configure-builds/environment-variables/
- 9: https://developers.netlify.com/sdk/build-event-handlers/values-and-methods/
- 10: Document how to use environment variables in Build plugins netlify/build#1094
- 11: https://docs.netlify.com/build/configure-builds/file-based-configuration/
- 12: feat: support individual function paths in
zipFunctionsnetlify/build#6481
🌐 Web query:
Netlify build plugins onBuild runs before Functions bundling onPostBuild after bundling official docs
💡 Result:
According to official Netlify documentation, the build process execution order is designed such that Functions bundling occurs after the onBuild event and before the onPostBuild event [1][2]. The sequence of events is as follows: 1. onPreBuild: Runs before the build command [3]. 2. build.command: The project's build command is executed [3]. 3. onBuild: Runs after the build command completes [1][2]. 4. Functions Bundling: Netlify's core process performs functions bundling [3]. 5. onPostBuild: Runs after the build command completes and after functions bundling has been executed [1][2]. Consequently, if you use an onBuild event handler, it will execute before your functions are bundled, while an onPostBuild event handler will execute after the bundling process is finished [1][2].
Citations:
- 1: https://docs.netlify.com/extend/develop-and-share/develop-build-plugins/
- 2: https://developers.netlify.com/sdk/build-event-handlers/hook-into-events/
- 3: Run core plugins last netlify/build#1796
🌐 Web query:
Netlify docs netlifyConfig.build.environment modify values during build process build plugins official
💡 Result:
To modify build environment variables during the Netlify build process using official build plugins, you should utilize the netlifyConfig.build.environment object [1][2]. Official documentation recommends modifying netlifyConfig.build.environment over process.env when you need to change variable values during the build process [1][2]. This approach ensures that the environment variables are persisted to the build configuration, which allows the Netlify backend to access and utilize these updated values [3]. Key Implementation Details: 1. Access: Within your build plugin's event handlers (e.g., onPreBuild), you can access the configuration via the netlifyConfig object passed as an argument [1][2][4]. 2. Modification: You can directly assign or update values within netlifyConfig.build.environment [1][2]. Example: onPreBuild({ netlifyConfig }) { netlifyConfig.build.environment.DATABASE_URI = 'your-updated-value'; } 3. process.env vs. netlifyConfig: While modifying process.env is still possible for backward compatibility and can be used to read environment variables during the build, it does not update the site's official build configuration [1][2][3]. Modifying netlifyConfig.build.environment is the preferred method for ensuring configuration consistency across the Netlify platform [3]. Note: netlifyConfig.build.environment contains only the variables declared via the Netlify UI or netlify.toml [1][2]. If you only need to read variables, accessing them via process.env is sufficient [1][2].
Citations:
- 1: https://docs.netlify.com/extend/develop-and-share/develop-build-plugins/
- 2: https://developers.netlify.com/sdk/build-event-handlers/values-and-methods/
- 3: feat: update environment variables with
netlifyConfig.build.environmentnetlify/build#3393 - 4: Transforming configuration during builds netlify/build#1193
🌐 Web query:
Netlify build plugins netlifyConfig.build.environment recommended when you need to modify values during build official docs
💡 Result:
When working with Netlify build plugins, there are two primary ways to interact with environment variables, each recommended for different purposes according to official documentation [1][2]. 1. process.env You should use process.env to access environment variables when you only need to retrieve their values during the build process [1]. This object includes all environment variables available to the build, including those set in the Netlify UI, netlify.toml, and other sources [1]. 2. netlifyConfig.build.environment You should use netlifyConfig.build.environment when you need to modify environment variable values during the build process [1]. This object contains a subset of a project’s environment variables—specifically those explicitly declared by a user in the Netlify UI or netlify.toml [1]. Modifying this object allows the build to utilize the updated configuration [1][3]. In summary, use process.env for reading and netlifyConfig.build.environment for modifying environment variables within your build plugins [1].
Citations:
- 1: https://developers.netlify.com/sdk/build-event-handlers/values-and-methods/
- 2: https://docs.netlify.com/build/configure-builds/environment-variables/
- 3: Transforming configuration during builds netlify/build#1193
Move this to onBuild and mutate netlifyConfig.build.environment
onPostBuild runs after Functions bundling, so deleting from process.env here won’t change the packaged Lambda env payload. Use onBuild and update netlifyConfig.build.environment for the values you want Netlify to carry into the function bundle.
🤖 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 `@plugins/netlify-plugin-function-env-guard/index.js` around lines 141 - 186,
The environment trimming logic in onPostBuild is too late to affect the packaged
function payload, since process.env is already consumed by Functions bundling.
Move the logic into onBuild in module.exports and mutate
netlifyConfig.build.environment instead of deleting from process.env, using the
same shouldDeleteFromEnvironment, shouldExcludeFromFunctionPayload, canTrim, and
envByteSize flow to identify which variables should be removed before bundling.
| const entries = Object.entries(process.env); | ||
| for (const [name] of entries) { | ||
| if (shouldDeleteFromEnvironment(name)) { | ||
| delete process.env[name]; | ||
| } | ||
| } | ||
|
|
||
| const functionEntries = entries.filter(([name]) => !shouldExcludeFromFunctionPayload(name)); | ||
| const initialSize = envByteSize(functionEntries); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Stale snapshot reused for size/trim accounting — distorts the byte estimate.
entries (line 143) is captured before the deletion loop (144-148) mutates process.env. functionEntries (line 150) is then filtered from that same stale entries array instead of re-reading Object.entries(process.env) after deletion. This means functionEntries/initialSize include variables that were already deleted moments earlier (any non-platform, non-build-only, non-allowlisted var). Consequences:
initialSizeis inflated with bytes for values no longer present inprocess.env.- Those same "phantom" entries are also
canTrim-eligible, so the trim loop can "consume" a fake budget from vars that don't exist, causingcurrentSizeto drop belowTARGET_MAX_BYTES/BYTE_LIMITeven though the real remaining payload (allowlistedFUNCTION_ENV_NAMESentries, which are never trimmable) could still exceed the limit — silently passing the guard'sfailBuildcheck when it shouldn't.
🐛 Proposed fix
const functionEntries = entries.filter(([name]) => !shouldExcludeFromFunctionPayload(name));
+ // Recompute from process.env after the deletion pass above, not the stale pre-deletion snapshot.
+ const functionEntries = Object.entries(process.env).filter(([name]) => !shouldExcludeFromFunctionPayload(name));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const entries = Object.entries(process.env); | |
| for (const [name] of entries) { | |
| if (shouldDeleteFromEnvironment(name)) { | |
| delete process.env[name]; | |
| } | |
| } | |
| const functionEntries = entries.filter(([name]) => !shouldExcludeFromFunctionPayload(name)); | |
| const initialSize = envByteSize(functionEntries); | |
| const entries = Object.entries(process.env); | |
| for (const [name] of entries) { | |
| if (shouldDeleteFromEnvironment(name)) { | |
| delete process.env[name]; | |
| } | |
| } | |
| // Recompute from process.env after the deletion pass above, not the stale pre-deletion snapshot. | |
| const functionEntries = Object.entries(process.env).filter(([name]) => !shouldExcludeFromFunctionPayload(name)); | |
| const initialSize = envByteSize(functionEntries); |
🤖 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 `@plugins/netlify-plugin-function-env-guard/index.js` around lines 143 - 151,
The size accounting in the environment pruning logic is using a stale snapshot
of process.env, which makes initialSize and trimming decisions inaccurate.
Recompute the entries from the current process.env after the deletion loop in
the guard flow, then derive functionEntries from that fresh snapshot before
calling envByteSize and the trim logic. Update the code around
Object.entries(process.env), shouldDeleteFromEnvironment,
shouldExcludeFromFunctionPayload, and envByteSize so the byte estimate only
reflects variables that still exist.
| @@ -1,45 +1,130 @@ | |||
| import { InfisicalClient } from "@infisical/sdk"; | |||
| import { InfisicalSDK } from "@infisical/sdk"; | |||
| import { ALIAS_GROUPS, P0_KEYS, P1_KEYS } from "./parity-manifest.js"; | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Duplicate manifest-key computation instead of reusing ALL_MANIFEST_KEYS.
parity-manifest.ts already exports ALL_MANIFEST_KEYS = [...new Set([...P0_KEYS, ...P1_KEYS])]. Line 114 recomputes the exact same set inline instead of importing it. If the manifest's dedup/derivation logic changes later, this local copy can silently drift out of sync.
♻️ Proposed fix
-import { ALIAS_GROUPS, P0_KEYS, P1_KEYS } from "./parity-manifest.js";
+import { ALIAS_GROUPS, ALL_MANIFEST_KEYS } from "./parity-manifest.js";- const manifestKeys = [...new Set([...P0_KEYS, ...P1_KEYS])];
+ const manifestKeys = ALL_MANIFEST_KEYS;Also applies to: 109-116
🤖 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 `@scripts/drift-check.ts` at line 2, The drift-check script is recomputing the
manifest key list inline instead of reusing the shared ALL_MANIFEST_KEYS export,
which can drift from parity-manifest.ts over time. Update the imports in
scripts/drift-check.ts to use ALL_MANIFEST_KEYS from parity-manifest.ts and
replace the local deduplication logic in the drift comparison code with that
shared symbol so both paths stay consistent.
| function reportDrift( | ||
| strictness: string, | ||
| missingOnNetlify: string[], | ||
| missingInInfisical: string[] | ||
| ): void { | ||
| const issues: string[] = []; | ||
|
|
||
| if (missingOnNetlify.length > 0) { | ||
| issues.push(`missing on Netlify: ${missingOnNetlify.join(", ")}`); | ||
| } | ||
| if (missingInInfisical.length > 0) { | ||
| issues.push(`missing in Infisical ${INFISICAL_PATH}: ${missingInInfisical.join(", ")}`); | ||
| } | ||
|
|
||
| if (issues.length === 0) { | ||
| console.log("✅ No drift detected. Netlify production matches Infisical staging manifest."); | ||
| return; | ||
| } | ||
|
|
||
| const message = `[Drift Detected] ${issues.join("; ")}`; | ||
|
|
||
| if (strictness === "fail") { | ||
| console.error(`❌ FATAL: ${message}`); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| console.warn(`⚠️ WARNING: ${message}`); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
P0 vs. P1 severity tiers are never actually enforced.
parity-manifest.ts documents P0_KEYS as "P0 — hard fail when missing on Netlify (staging/prod)" and P1_KEYS as "P1 — parity / namespace keys; hard fail on main/develop/PR", implying P0 should always be a hard failure while P1 is conditional on branch. However, runDriftCheck/reportDrift merge both sets into a single manifestKeys list (line 114) and apply one global strictness value (from STRICTNESS) uniformly — there is no per-key severity logic. On any branch other than main/develop/PR, STRICTNESS is 'warn', so even a missing P0 key (e.g. SUPABASE_SERVICE_ROLE_KEY) only produces a warning instead of the documented hard fail.
This defeats the purpose of having two severity tiers and could let a P0 key silently stay missing on Netlify production for any non-tracked branch build.
Also applies to: 109-125
🤖 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 `@scripts/drift-check.ts` around lines 62 - 89, `runDriftCheck`/`reportDrift`
currently treat all manifest keys with one global `STRICTNESS`, so P0 keys can
be downgraded to warnings on non-tracked branches. Update the drift-check flow
to preserve severity per key group: keep `P0_KEYS` as always-fail checks and
apply branch-based strictness only to `P1_KEYS`, or otherwise split the
reporting so `reportDrift` can emit a fatal error whenever any P0 key is
missing. Use the existing `P0_KEYS`, `P1_KEYS`, `runDriftCheck`, and
`reportDrift` symbols to wire the two-tier logic through the manifest-key
aggregation and error handling.
1 participant
This PR adds the Netlify function environment guard plugin including netlify.toml and the plugin directory files, leaving out the SendGrid integration changes as requested.
Summary by CodeRabbit
New Features
wwwdomain now redirects to the primary site.Bug Fixes