From f05e2222f320fe3d786fbfcb20cf613c70f19e64 Mon Sep 17 00:00:00 2001 From: jesse-merhi <79823012+jesse-merhi@users.noreply.github.com> Date: Thu, 7 May 2026 14:08:59 +1000 Subject: [PATCH] lint: allow managed proxy mutation scopes --- .../check-managed-proxy-runtime-mutation.mjs | 162 ++++++++++++------ scripts/lib/callsite-guard.mjs | 39 ++++- ...eck-managed-proxy-runtime-mutation.test.ts | 51 +++++- 3 files changed, 194 insertions(+), 58 deletions(-) diff --git a/scripts/check-managed-proxy-runtime-mutation.mjs b/scripts/check-managed-proxy-runtime-mutation.mjs index 48dd5318dda..0fa91340244 100644 --- a/scripts/check-managed-proxy-runtime-mutation.mjs +++ b/scripts/check-managed-proxy-runtime-mutation.mjs @@ -6,55 +6,19 @@ import { runAsScript, toLine, unwrapExpression } from "./lib/ts-guard-utils.mjs" const sourceRoots = ["src", "extensions"]; -const allowedManagedProxyRuntimeMutationCallsites = new Set([ +const allowedManagedProxyRuntimeMutationScopes = new Set([ // Canonical managed proxy lifecycle owns process proxy env/global-agent mutation. - "src/infra/net/proxy/proxy-lifecycle.ts:114", - "src/infra/net/proxy/proxy-lifecycle.ts:117", - "src/infra/net/proxy/proxy-lifecycle.ts:119", - "src/infra/net/proxy/proxy-lifecycle.ts:120", - "src/infra/net/proxy/proxy-lifecycle.ts:121", - "src/infra/net/proxy/proxy-lifecycle.ts:123", - "src/infra/net/proxy/proxy-lifecycle.ts:131", - "src/infra/net/proxy/proxy-lifecycle.ts:133", - "src/infra/net/proxy/proxy-lifecycle.ts:146", - "src/infra/net/proxy/proxy-lifecycle.ts:147", - "src/infra/net/proxy/proxy-lifecycle.ts:148", - "src/infra/net/proxy/proxy-lifecycle.ts:178", - "src/infra/net/proxy/proxy-lifecycle.ts:180", - "src/infra/net/proxy/proxy-lifecycle.ts:199", - "src/infra/net/proxy/proxy-lifecycle.ts:200", - "src/infra/net/proxy/proxy-lifecycle.ts:201", - "src/infra/net/proxy/proxy-lifecycle.ts:312", - "src/infra/net/proxy/proxy-lifecycle.ts:313", - "src/infra/net/proxy/proxy-lifecycle.ts:314", - "src/infra/net/proxy/proxy-lifecycle.ts:315", - "src/infra/net/proxy/proxy-lifecycle.ts:316", - "src/infra/net/proxy/proxy-lifecycle.ts:317", - "src/infra/net/proxy/proxy-lifecycle.ts:318", - "src/infra/net/proxy/proxy-lifecycle.ts:319", - "src/infra/net/proxy/proxy-lifecycle.ts:329", - "src/infra/net/proxy/proxy-lifecycle.ts:330", - "src/infra/net/proxy/proxy-lifecycle.ts:331", - "src/infra/net/proxy/proxy-lifecycle.ts:332", - "src/infra/net/proxy/proxy-lifecycle.ts:333", - "src/infra/net/proxy/proxy-lifecycle.ts:334", - "src/infra/net/proxy/proxy-lifecycle.ts:335", - "src/infra/net/proxy/proxy-lifecycle.ts:336", - "src/infra/net/proxy/proxy-lifecycle.ts:369", - "src/infra/net/proxy/proxy-lifecycle.ts:376", - "src/infra/net/proxy/proxy-lifecycle.ts:484", - "src/infra/net/proxy/proxy-lifecycle.ts:507", - "src/infra/net/proxy/proxy-lifecycle.ts:508", - "src/infra/net/proxy/proxy-lifecycle.ts:515", - "src/infra/net/proxy/proxy-lifecycle.ts:516", + "src/infra/net/proxy/proxy-lifecycle.ts#applyProxyEnv", + "src/infra/net/proxy/proxy-lifecycle.ts#restoreProxyEnv", + "src/infra/net/proxy/proxy-lifecycle.ts#restoreGlobalAgentRuntime", + "src/infra/net/proxy/proxy-lifecycle.ts#restoreNodeHttpStack", + "src/infra/net/proxy/proxy-lifecycle.ts#bootstrapNodeHttpStack", + "src/infra/net/proxy/proxy-lifecycle.ts#writeGlobalAgentNoProxy", + "src/infra/net/proxy/proxy-lifecycle.ts#disableGlobalAgentProxyForIpv6GatewayLoopback", - // Browser CDP loopback control-plane helper leases NO_PROXY only for localhost/loopback CDP URLs. - "extensions/browser/src/browser/cdp-proxy-bypass.ts:87", - "extensions/browser/src/browser/cdp-proxy-bypass.ts:88", - "extensions/browser/src/browser/cdp-proxy-bypass.ts:120", - "extensions/browser/src/browser/cdp-proxy-bypass.ts:122", - "extensions/browser/src/browser/cdp-proxy-bypass.ts:125", - "extensions/browser/src/browser/cdp-proxy-bypass.ts:127", + // Browser CDP loopback helper leases NO_PROXY only for localhost/loopback CDP URLs. + "extensions/browser/src/browser/cdp-proxy-bypass.ts#NoProxyLeaseManager.acquire", + "extensions/browser/src/browser/cdp-proxy-bypass.ts#NoProxyLeaseManager.release", ]); const forbiddenEnvKeys = new Set([ @@ -78,6 +42,55 @@ function stringLiteralText(node) { return ts.isStringLiteral(node) || ts.isNoSubstitutionTemplateLiteral(node) ? node.text : null; } +function propertyNameText(name) { + if (ts.isIdentifier(name) || ts.isStringLiteral(name) || ts.isNumericLiteral(name)) { + return name.text; + } + return null; +} + +function qualifiedScopeName(name, classScopes) { + const classScope = classScopes.at(-1); + return classScope ? `${classScope}.${name}` : name; +} + +function functionExpressionScopeName(node, classScopes) { + const parent = node.parent; + if (ts.isVariableDeclaration(parent) && ts.isIdentifier(parent.name)) { + return parent.name.text; + } + if (ts.isPropertyAssignment(parent)) { + const name = propertyNameText(parent.name); + return name ? qualifiedScopeName(name, classScopes) : null; + } + if (ts.isPropertyDeclaration(parent) && parent.name) { + const name = propertyNameText(parent.name); + return name ? qualifiedScopeName(name, classScopes) : null; + } + return null; +} + +function scopeNameForNode(node, classScopes) { + if (ts.isFunctionDeclaration(node) && node.name) { + return node.name.text; + } + if (ts.isFunctionExpression(node) || ts.isArrowFunction(node)) { + return functionExpressionScopeName(node, classScopes); + } + if ( + ts.isMethodDeclaration(node) || + ts.isGetAccessorDeclaration(node) || + ts.isSetAccessorDeclaration(node) + ) { + const name = propertyNameText(node.name); + return name ? qualifiedScopeName(name, classScopes) : null; + } + if (ts.isConstructorDeclaration(node)) { + return qualifiedScopeName("constructor", classScopes); + } + return null; +} + function isGlobalIdentifier(node, context = { globalAliases: new Set() }) { const unwrapped = unwrapExpression(node); return ( @@ -398,7 +411,7 @@ function mutatingCallTarget(expression, context) { return null; } -export function findManagedProxyRuntimeMutationLines(content, fileName = "source.ts") { +export function findManagedProxyRuntimeMutations(content, fileName = "source.ts") { const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.Latest, true); const globalAliases = collectGlobalAliases(sourceFile); const context = { @@ -411,19 +424,61 @@ export function findManagedProxyRuntimeMutationLines(content, fileName = "source envAliases: collectEnvAliases(sourceFile), stringConstants: collectStringConstants(sourceFile), }; - const lines = []; + const mutations = []; + const classScopes = []; + const scopeStack = []; const visit = (node) => { + let pushedClass = false; + let pushedScope = false; + if (ts.isClassDeclaration(node) && node.name) { + classScopes.push(node.name.text); + pushedClass = true; + } + const scopeName = scopeNameForNode(node, classScopes); + if (scopeName) { + scopeStack.push(scopeName); + pushedScope = true; + } + const match = assignmentTarget(node, context) ?? deleteTarget(node, context) ?? mutatingCallTarget(node, context); if (match) { - lines.push(toLine(sourceFile, match)); + mutations.push({ + line: toLine(sourceFile, match), + scope: scopeStack.at(-1) ?? null, + }); } ts.forEachChild(node, visit); + + if (pushedScope) { + scopeStack.pop(); + } + if (pushedClass) { + classScopes.pop(); + } }; visit(sourceFile); - return lines; + return mutations; +} + +export function findManagedProxyRuntimeMutationLines(content, fileName = "source.ts") { + return findManagedProxyRuntimeMutations(content, fileName).map((mutation) => mutation.line); +} + +export function isAllowedManagedProxyRuntimeMutation(violation) { + if (!violation.scope) { + return false; + } + return allowedManagedProxyRuntimeMutationScopes.has( + `${violation.relativePath}#${violation.scope}`, + ); +} + +function formatManagedProxyRuntimeMutationCallsite(violation) { + const scope = violation.scope ? ` (${violation.scope})` : ""; + return `${violation.relativePath}:${violation.line}${scope}`; } export async function main() { @@ -437,8 +492,9 @@ export async function main() { ".e2e.test.ts", ".integration.test.ts", ], - findCallLines: findManagedProxyRuntimeMutationLines, - allowCallsite: (callsite) => allowedManagedProxyRuntimeMutationCallsites.has(callsite), + findCallViolations: findManagedProxyRuntimeMutations, + allowViolation: isAllowedManagedProxyRuntimeMutation, + formatViolation: formatManagedProxyRuntimeMutationCallsite, header: "Found unmanaged managed-proxy runtime mutation:", footer: "Only proxy lifecycle code may mutate GLOBAL_AGENT or proxy-related process.env runtime state.", diff --git a/scripts/lib/callsite-guard.mjs b/scripts/lib/callsite-guard.mjs index 94715e9cb9b..a4ff9d205c0 100644 --- a/scripts/lib/callsite-guard.mjs +++ b/scripts/lib/callsite-guard.mjs @@ -6,6 +6,19 @@ import { resolveSourceRoots, } from "./ts-guard-utils.mjs"; +function normalizeViolation(rawViolation, relPath) { + if (typeof rawViolation === "number") { + return { + line: rawViolation, + callsite: `${relPath}:${rawViolation}`, + }; + } + return { + ...rawViolation, + callsite: rawViolation.callsite ?? `${relPath}:${rawViolation.line}`, + }; +} + export async function runCallsiteGuard(params) { const repoRoot = resolveRepoRoot(params.importMetaUrl); const sourceRoots = resolveSourceRoots(repoRoot, params.sourceRoots); @@ -20,12 +33,30 @@ export async function runCallsiteGuard(params) { continue; } const content = await fs.readFile(filePath, "utf8"); - for (const line of params.findCallLines(content, filePath)) { - const callsite = `${relPath}:${line}`; - if (params.allowCallsite?.(callsite)) { + const rawViolations = params.findCallViolations + ? params.findCallViolations(content, filePath) + : params.findCallLines(content, filePath); + for (const rawViolation of rawViolations) { + const violation = normalizeViolation(rawViolation, relPath); + if ( + params.allowViolation?.({ + ...violation, + relativePath: relPath, + filePath, + }) ?? + params.allowCallsite?.(violation.callsite, violation) + ) { continue; } - violations.push(callsite); + violations.push( + params.formatViolation + ? params.formatViolation({ + ...violation, + relativePath: relPath, + filePath, + }) + : violation.callsite, + ); } } diff --git a/test/scripts/check-managed-proxy-runtime-mutation.test.ts b/test/scripts/check-managed-proxy-runtime-mutation.test.ts index d64af060f69..7774e7c5eaa 100644 --- a/test/scripts/check-managed-proxy-runtime-mutation.test.ts +++ b/test/scripts/check-managed-proxy-runtime-mutation.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from "vitest"; -import { findManagedProxyRuntimeMutationLines } from "../../scripts/check-managed-proxy-runtime-mutation.mjs"; +import { + findManagedProxyRuntimeMutationLines, + findManagedProxyRuntimeMutations, + isAllowedManagedProxyRuntimeMutation, +} from "../../scripts/check-managed-proxy-runtime-mutation.mjs"; describe("check-managed-proxy-runtime-mutation", () => { it("finds assignments and deletes for proxy env vars", () => { @@ -181,4 +185,49 @@ describe("check-managed-proxy-runtime-mutation", () => { expect(findManagedProxyRuntimeMutationLines(source)).toEqual([]); }); + + it("reports the enclosing owner scope for each mutation", () => { + const source = ` + function applyProxyEnv() { + process.env.HTTP_PROXY = "http://proxy"; + } + + class NoProxyLeaseManager { + release() { + delete process.env.NO_PROXY; + } + } + + const updateProxy = () => { + global.GLOBAL_AGENT.NO_PROXY = "localhost"; + }; + `; + + expect(findManagedProxyRuntimeMutations(source)).toEqual([ + { line: 3, scope: "applyProxyEnv" }, + { line: 8, scope: "NoProxyLeaseManager.release" }, + { line: 13, scope: "updateProxy" }, + ]); + }); + + it("allows approved owner scopes without exact line allowlists", () => { + expect( + isAllowedManagedProxyRuntimeMutation({ + relativePath: "src/infra/net/proxy/proxy-lifecycle.ts", + scope: "applyProxyEnv", + }), + ).toBe(true); + expect( + isAllowedManagedProxyRuntimeMutation({ + relativePath: "extensions/browser/src/browser/cdp-proxy-bypass.ts", + scope: "NoProxyLeaseManager.release", + }), + ).toBe(true); + expect( + isAllowedManagedProxyRuntimeMutation({ + relativePath: "src/infra/net/proxy/proxy-lifecycle.ts", + scope: "startProxy", + }), + ).toBe(false); + }); });