Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions apps/loopover-ui/src/lib/selfhost-env-reference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,22 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [
name: "GITHUB_PUBLIC_TOKEN",
firstReference: "src/queue/ai-review-orchestration.ts",
},
{
name: "GITHUB_WEBHOOK_SECRET",
firstReference: "src/selfhost/preflight.ts",
},
{
name: "HOME",
firstReference: "src/selfhost/ai.ts",
},
{
name: "INTERNAL_JOB_TOKEN",
firstReference: "src/selfhost/preflight.ts",
},
{
name: "LOOPOVER_API_TOKEN",
firstReference: "src/selfhost/preflight.ts",
},
{
name: "LOOPOVER_ENABLE_PAGERDUTY",
firstReference: "src/services/notify-pagerduty.ts",
Expand All @@ -257,6 +269,10 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [
name: "LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER",
firstReference: "src/selfhost/ai.ts",
},
{
name: "LOOPOVER_MCP_TOKEN",
firstReference: "src/selfhost/preflight.ts",
},
{
name: "LOOPOVER_REPO_CONFIG_DIR",
firstReference: "src/server.ts",
Expand Down Expand Up @@ -626,9 +642,13 @@ export const SELFHOST_ENV_REFERENCE_MARKDOWN = [
"| `GITHUB_INSTALLATION_CONCURRENCY_ENABLED` | `src/selfhost/installation-concurrency-admission.ts` |",
"| `GITHUB_INSTALLATION_CONCURRENCY_LIMIT` | `src/selfhost/installation-concurrency-admission.ts` |",
"| `GITHUB_PUBLIC_TOKEN` | `src/queue/ai-review-orchestration.ts` |",
"| `GITHUB_WEBHOOK_SECRET` | `src/selfhost/preflight.ts` |",
"| `HOME` | `src/selfhost/ai.ts` |",
"| `INTERNAL_JOB_TOKEN` | `src/selfhost/preflight.ts` |",
"| `LOOPOVER_API_TOKEN` | `src/selfhost/preflight.ts` |",
"| `LOOPOVER_ENABLE_PAGERDUTY` | `src/services/notify-pagerduty.ts` |",
"| `LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER` | `src/selfhost/ai.ts` |",
"| `LOOPOVER_MCP_TOKEN` | `src/selfhost/preflight.ts` |",
"| `LOOPOVER_REPO_CONFIG_DIR` | `src/server.ts` |",
"| `LOOPOVER_REVIEW_CONTINUOUS` | `src/queue/processors.ts` |",
"| `LOOPOVER_VERSION` | `src/selfhost/otel.ts` |",
Expand Down
58 changes: 58 additions & 0 deletions scripts/gen-selfhost-env-reference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ function collectEnvReads(source: string, fileName: string): EnvRead[] {
if (!ENV_NAME_RE.test(name) || INJECTED_BINDING_NAMES.has(name)) return;
reads.push({ name });
};
// Locally-declared `const NAME = ["A", "B", ...]` literal-string arrays, so a `for (const x of NAME)` loop
// whose body reads `env[x]` can be resolved back to the concrete var names -- src/selfhost/preflight.ts's
// CRITICAL_SECRET_VARS loop reads four tokens (GITHUB_WEBHOOK_SECRET etc.) this way and nowhere else (#8652).
const literalArrays = collectLiteralStringArrays(sourceFile);
const visit = (node: ts.Node) => {
if (ts.isPropertyAccessExpression(node) && isEnvContainer(node.expression)) {
addRead(node.name.text);
Expand All @@ -95,6 +99,8 @@ function collectEnvReads(source: string, fileName: string): EnvRead[] {
const arg = node.arguments[argIndex];
if (arg && ts.isStringLiteralLike(arg)) addRead(arg.text);
}
} else if (ts.isForOfStatement(node)) {
for (const name of envReadingForOfArrayLiterals(node, literalArrays)) addRead(name);
}
ts.forEachChild(node, visit);
};
Expand Down Expand Up @@ -141,6 +147,58 @@ function isEnvNameLiteralArgHelperCall(node: ts.CallExpression): boolean {
return argIndexes !== undefined && argIndexes.some((argIndex) => node.arguments.length > argIndex && ts.isStringLiteralLike(node.arguments[argIndex]!));
}

// Collect every locally-declared `const NAME = ["A", "B", ...]` whose initializer is an array of only string
// literals (unwrapping a trailing `as const`). Used to resolve `for (const x of NAME) { env[x] }` loops back to
// concrete var names. Generalizes to any such array -- no var name is special-cased.
function collectLiteralStringArrays(sourceFile: ts.SourceFile): Map<string, string[]> {
const arrays = new Map<string, string[]>();
const walk = (node: ts.Node) => {
if (ts.isVariableDeclaration(node) && ts.isIdentifier(node.name) && node.initializer) {
const init = unwrapEnvExpression(node.initializer);
if (ts.isArrayLiteralExpression(init) && init.elements.length > 0 && init.elements.every((element) => ts.isStringLiteralLike(element))) {
arrays.set(
node.name.text,
init.elements.map((element) => (element as ts.StringLiteralLike).text),
);
}
}
ts.forEachChild(node, walk);
};
walk(sourceFile);
return arrays;
}

// If `node` iterates a known literal-string array with a single identifier loop variable whose body reads
// `env[<loopVar>]`, return that array's literal names; otherwise []. This is the `for (const name of
// LOCAL_ARRAY) { env[name] }` computed-read pattern the plain element-access branch can't see (the argument is
// an identifier, not a string literal).
function envReadingForOfArrayLiterals(node: ts.ForOfStatement, literalArrays: Map<string, string[]>): string[] {
const iterable = unwrapEnvExpression(node.expression);
if (!ts.isIdentifier(iterable)) return [];
const literals = literalArrays.get(iterable.text);
if (!literals) return [];
if (!ts.isVariableDeclarationList(node.initializer) || node.initializer.declarations.length !== 1) return [];
const loopVar = node.initializer.declarations[0]!.name;
if (!ts.isIdentifier(loopVar)) return [];
return bodyReadsEnvByName(node.statement, loopVar.text) ? literals : [];
}

// True if `body` reads `env[<loopVar>]` anywhere -- a computed element access whose object is an env container
// and whose argument is the loop variable identifier.
function bodyReadsEnvByName(body: ts.Statement, loopVar: string): boolean {
let found = false;
const walk = (node: ts.Node) => {
if (found) return;
if (ts.isElementAccessExpression(node) && isEnvContainer(node.expression) && ts.isIdentifier(node.argumentExpression) && node.argumentExpression.text === loopVar) {
found = true;
return;
}
ts.forEachChild(node, walk);
};
walk(body);
return found;
}

function bindingElementName(element: ts.BindingElement): string | null {
const candidate = element.propertyName ?? element.name;
if (ts.isIdentifier(candidate) || ts.isStringLiteralLike(candidate)) return candidate.text;
Expand Down
52 changes: 52 additions & 0 deletions test/unit/selfhost-env-reference-script.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,58 @@ describe("gen-selfhost-env-reference (#2081)", () => {
expect(after).toEqual(before);
});

it("REGRESSION (#8652): detects env reads inside a for-of over a locally-declared literal-name array", () => {
const root = mkdtempSync(join(tmpdir(), "gt-env-reference-forof-"));
mkdirSync(join(root, "src", "selfhost"), { recursive: true });
// Mirrors src/selfhost/preflight.ts's CRITICAL_SECRET_VARS loop: the var names come from iterating a local
// const array of string literals, and `env[name]` is a computed access whose argument is the loop variable,
// not a string literal -- invisible to the plain element-access branch.
writeFileSync(
join(root, "src", "selfhost", "loops.ts"),
[
'const SECRET_VARS = ["ALPHA_TOKEN", "BETA_TOKEN"] as const;',
"for (const name of SECRET_VARS) {",
" const value = nonBlank(env[name]);",
" void value;",
"}",
// Negative: a literal-name array whose loop never touches env must NOT be surfaced.
'const UNUSED_VARS = ["GAMMA_UNUSED"];',
"for (const label of UNUSED_VARS) {",
" console.log(label);",
"}",
// Negative: destructuring loop variable over a known literal array is not a plain `env[name]` read.
"for (const [first] of SECRET_VARS) {",
" void first;",
"}",
// Negative: an assignment-target loop (no `const` declaration list) is ignored.
"let reused;",
"for (reused of SECRET_VARS) {",
" void reused;",
"}",
// Negative: iterable is a call expression, not an identifier -> ignored.
"for (const other of Object.keys(env)) {",
" void other;",
"}",
// Negative: iterable identifier is not a collected literal-string array -> ignored.
"for (const missing of NOT_DECLARED_HERE) {",
" void env[missing];",
"}",
// Non-string / empty arrays are never treated as literal-name arrays.
"const NUMBERS = [1, 2, 3];",
"const EMPTY_ARRAY = [];",
"for (const n of NUMBERS) {",
" void n;",
"}",
"void EMPTY_ARRAY;",
"",
].join("\n"),
);
expect(collectSelfHostEnvVars({ rootDir: root })).toEqual([
{ name: "ALPHA_TOKEN", firstReference: "src/selfhost/loops.ts" },
{ name: "BETA_TOKEN", firstReference: "src/selfhost/loops.ts" },
]);
});

it("scans configured JavaScript roots and rejects file-shaped directories", () => {
const root = fixtureRoot();

Expand Down
Loading