fix(scripts): detect env reads inside a for-of over a literal-name array (#8652) - #8694
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
gen-selfhost-env-reference.ts recognized computed env[X] access only via a string literal, the
envString(env, "X") helper, or a helper whose name argument is a literal at the call site. It had
no case for a name sourced from iterating a local array of string literals, so the four critical
secret tokens src/selfhost/preflight.ts reads only through
for (const name of CRITICAL_SECRET_VARS) { const value = nonBlank(env[name]); ... }
(GITHUB_WEBHOOK_SECRET, LOOPOVER_API_TOKEN, LOOPOVER_MCP_TOKEN, INTERNAL_JOB_TOKEN) were absent from
the operator-facing self-host env reference -- exactly the high-risk secrets that table exists to warn
operators about.
Extend the scan: a pre-pass collects every locally-declared const array of only string literals
(unwrapping a trailing 'as const'), and a new for-of branch surfaces the array's names when the loop
iterates it with a single identifier loop variable whose body reads env[loopVar]. Generalizes to any
such array -- no var name is special-cased. Regenerated reference adds exactly the four tokens.
Test: a fixture-driven regression test drives the new array-loop path directly, with positive and
negative shapes (non-env loop, destructured loop var, non-identifier iterable, unknown identifier,
numeric/empty arrays) exercising every new branch.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8694 +/- ##
=======================================
Coverage 93.76% 93.76%
=======================================
Files 797 797
Lines 79454 79454
Branches 24070 24070
=======================================
Hits 74504 74504
Misses 3565 3565
Partials 1385 1385
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-25 23:55:23 UTC
Review summary Nits — 5 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Problem
Closes #8652.
scripts/gen-selfhost-env-reference.tsrecognized computedenv[X]access only via a string literal, theenvString(env, "X")helper, or a helper whose name argument is a literal at the call site. It had no case for a var name sourced from iterating a local array of string literals.src/selfhost/preflight.tsreads four critical secret tokens only through such a loop:env[name]is a computed access whose argument is the loop variable, not a string literal, so it was invisible to the element-access branch. The four tokens above (SELFHOST_SETUP_TOKENis separately read literally elsewhere, so it was already covered) were missing from the operator-facing self-host env reference — exactly the high-risk secrets preflight’s own comment says bypass real checks silently if left weak.Fix
Extend the scan generically:
collectLiteralStringArrays) collects every locally-declaredconst NAME = ["A", "B", ...]whose initializer is an array of only string literals, unwrapping a trailingas const.ForOfStatementbranch surfaces the array’s names when the loop iterates such an array with a single identifier loop variable whose body readsenv[loopVar].Generalizes to any such array — no var name is special-cased. The regenerated
apps/loopover-ui/src/lib/selfhost-env-reference.tsadds exactly the four tokens (GITHUB_WEBHOOK_SECRET,INTERNAL_JOB_TOKEN,LOOPOVER_API_TOKEN,LOOPOVER_MCP_TOKEN) and nothing else.Tests
A fixture-driven regression test in
test/unit/selfhost-env-reference-script.test.tsdrives the new array-loop path directly, asserting the two positive names are detected, plus negative shapes that exercise every new branch: a literal-name array whose loop never readsenv, a destructured loop variable, an assignment-target loop (noconst), a non-identifier iterable (Object.keys(env)), an unknown iterable identifier, and numeric/empty arrays.npm run selfhost:env-reference -- --checkpasses;git diff --checkclean.