From df2ed009ce3a8f5099224f5dfa86254e335923ed Mon Sep 17 00:00:00 2001 From: luyua9 Date: Fri, 22 May 2026 01:24:27 +0800 Subject: [PATCH] fix(group): detect httpx AsyncClient alias imports (#1687) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(group): detect httpx AsyncClient alias imports * fix(group): anchor httpx dotted imports and skip shadowed aliases Addresses Findings 1-3 of the production-readiness review on PR #1687. - F1: the `(dotted_name (identifier) @module)` capture matches every segment of a dotted module path, so `import package.httpx as hx` and `from package.httpx import AsyncClient` would falsely populate the alias sets. Anchor the check on `moduleNode.parent?.text === 'httpx'` so the full dotted_name must equal `httpx`. - F2: `moduleAliases` and `asyncClientAliases` were file-global and unaware of Python scope. A function-local rebind like `AsyncClient = lambda: MockClient()` left the alias entry intact and any subsequent `client = AsyncClient(); client.get(...)` emitted a false-positive consumer contract. Walk every `(assignment left: (identifier) @name)` whose name matches an alias, record the enclosing function/class scope as poisoned, and skip direct- and module-attribute matches when the call site is inside that scope chain. - F3: extend the existing fixture with dotted-package look-alikes and three local-shadow cases (`shadow_direct_alias`, `shadow_module_alias`, `shadow_direct_context`) and assert the would-be FP contractIds are not emitted. - F6: refresh the module-level docstring to mention the supported import-alias forms and the shadow-exclusion behavior. * refactor(group): tighten httpx alias shadow detection and broaden tests Follow-up addressing the residual review findings on PR #1687. - Replace inline scope-key construction in isAliasShadowed with a getScopeKey call so the two helpers cannot drift apart (M1). - Collapse the double tree traversal in collectHttpxAsyncClients: build one combined alias set and pass it to a single collectAliasShadowScopes call (perf, P2). - Add a `shadowScopeKey` helper that returns the scope a rebind actually shadows under Python LEGB rules: function scope for in-function rebinds, 'module' for top-level rebinds, and `null` for class-body rebinds (class attributes do not shadow bare-name lookups in methods). Removes the previous blanket `scopeKey === 'module'` skip and now correctly poisons module-level rebinds (correctness #1). - Extend `ALIAS_SHADOW_PATTERNS` to cover tuple, list, and pattern_list destructuring targets (correctness #2). - Rename `ALIAS_REBIND_PATTERNS` to `ALIAS_SHADOW_PATTERNS` and update the block comment to say "shadowed" rather than "poisoned" (M4). - Collapse `callScopeKeys` to a single-line return; the dead Set wrap was misleading future readers (M2). Tests: - New negative fixtures for 3-segment dotted import (`import a.b.c.httpx as deep_evil`), relative import (`from .httpx import AsyncClient as rel_evil_async`), tuple destructuring rebind, and an isolated file exercising the module-level rebind path (T1, correctness #2, expanded F2). - New positive fixture confirming that a class-body assignment of `AsyncClient` does NOT poison the surrounding methods. - Add a positive control assertion for `module_direct_client` so the dotted-package negative assertions cannot pass vacuously (T3). --------- Co-authored-by: Gergő Magyar Co-authored-by: Test --- .../group/extractors/http-patterns/python.ts | 247 +++++++++++++++++- .../unit/group/http-route-extractor.test.ts | 141 ++++++++++ 2 files changed, 374 insertions(+), 14 deletions(-) diff --git a/gitnexus/src/core/group/extractors/http-patterns/python.ts b/gitnexus/src/core/group/extractors/http-patterns/python.ts index 0d3385247..1667d0de9 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/python.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/python.ts @@ -13,7 +13,12 @@ import type { HttpDetection, HttpLanguagePlugin } from './types.js'; * - FastAPI `@app.get("/path")` provider decorators * - `requests.get/post/...("url")` consumer calls * - Generic `requests.request("METHOD", "url")` consumer calls - * - `httpx.AsyncClient` instances calling `.get/.post/...("url")` + * - `httpx.AsyncClient` instances calling `.get/.post/...("url")`, including + * aliased imports such as `import httpx as hx`, + * `from httpx import AsyncClient`, and + * `from httpx import AsyncClient as HttpxAsyncClient`. + * Locally rebound names (e.g. `AsyncClient = mock_factory()` inside a + * function) are excluded to avoid false-positive consumer contracts. */ const FASTAPI_VERBS: Record = { @@ -80,12 +85,50 @@ const REQUESTS_GENERIC_PATTERNS = compilePatterns({ } satisfies LanguagePatterns>); // ─── Consumer: httpx.AsyncClient assignments ──────────────────────── -// NOTE: This targeted detector only tracks explicit `httpx.AsyncClient(...)` -// construction. Direct imports (`from httpx import AsyncClient`) and module -// aliases (`import httpx as hx`) and annotated assignments (`client: httpx.AsyncClient = ...`) -// are intentionally left for a follow-up. Module-scope clients are only matched +// Module-scope clients are only matched // at module scope; calls inside functions require a function/class-local tracked // client to avoid false positives from same-name local variables. +const HTTPX_MODULE_IMPORT_PATTERNS = compilePatterns({ + name: 'python-httpx-module-imports', + language: Python, + patterns: [ + { + meta: {}, + query: ` + (import_statement + name: (aliased_import + name: (dotted_name (identifier) @module) + alias: (identifier) @alias)) + `, + }, + ], +} satisfies LanguagePatterns>); + +const HTTPX_ASYNC_CLIENT_IMPORT_PATTERNS = compilePatterns({ + name: 'python-httpx-async-client-imports', + language: Python, + patterns: [ + { + meta: {}, + query: ` + (import_from_statement + module_name: (dotted_name (identifier) @module) + name: (dotted_name (identifier) @client_class)) + `, + }, + { + meta: {}, + query: ` + (import_from_statement + module_name: (dotted_name (identifier) @module) + name: (aliased_import + name: (dotted_name (identifier) @client_class) + alias: (identifier) @alias)) + `, + }, + ], +} satisfies LanguagePatterns>); + const HTTPX_ASYNC_CLIENT_ASSIGN_PATTERNS = compilePatterns({ name: 'python-httpx-async-client-assign', language: Python, @@ -97,8 +140,24 @@ const HTTPX_ASYNC_CLIENT_ASSIGN_PATTERNS = compilePatterns({ left: (_) @client right: (call function: (attribute - object: (identifier) @module (#eq? @module "httpx") - attribute: (identifier) @client_class (#eq? @client_class "AsyncClient")))) + object: (identifier) @module + attribute: (identifier) @client_class))) + `, + }, + ], +} satisfies LanguagePatterns>); + +const HTTPX_ASYNC_CLIENT_DIRECT_ASSIGN_PATTERNS = compilePatterns({ + name: 'python-httpx-async-client-direct-assign', + language: Python, + patterns: [ + { + meta: {}, + query: ` + (assignment + left: (_) @client + right: (call + function: (identifier) @client_class)) `, }, ], @@ -115,8 +174,24 @@ const HTTPX_ASYNC_CLIENT_WITH_ALIAS_PATTERNS = compilePatterns({ (as_pattern (call function: (attribute - object: (identifier) @module (#eq? @module "httpx") - attribute: (identifier) @client_class (#eq? @client_class "AsyncClient"))) + object: (identifier) @module + attribute: (identifier) @client_class)) + (as_pattern_target (identifier) @client)) + `, + }, + ], +} satisfies LanguagePatterns>); + +const HTTPX_ASYNC_CLIENT_DIRECT_WITH_ALIAS_PATTERNS = compilePatterns({ + name: 'python-httpx-async-client-direct-with-alias', + language: Python, + patterns: [ + { + meta: {}, + query: ` + (as_pattern + (call + function: (identifier) @client_class) (as_pattern_target (identifier) @client)) `, }, @@ -150,17 +225,137 @@ function trackedClientScopeKey(clientNode: Parser.SyntaxNode): string { } function callScopeKeys(clientNode: Parser.SyntaxNode): string[] { - const keys = new Set(); - const preferClass = clientNode.text.includes('.'); - const nearestScope = getScopeKey(clientNode.parent, preferClass); + return [getScopeKey(clientNode.parent, clientNode.text.includes('.'))]; +} - keys.add(nearestScope); +// Returns the scope key that a rebind of an imported alias would shadow under +// Python LEGB rules, or `null` when the rebind does not shadow anything that +// could produce a false-positive consumer detection. +// - Rebind inside a function/method → that function's scope. +// - Rebind at module top level → 'module' (shadows the whole file). +// - Rebind in a class body without an enclosing function → null. Python +// class attributes do not shadow bare-name lookups inside methods (methods +// see the module binding, not the class attribute), so we must not poison +// them. +function shadowScopeKey(node: Parser.SyntaxNode | null): string | null { + let current = node; + let passedThroughClass = false; + while (current) { + if (current.type === 'function_definition') { + // Reuse getScopeKey's key format so the two helpers cannot drift apart. + return getScopeKey(current); + } + if (current.type === 'class_definition') { + passedThroughClass = true; + } + current = current.parent; + } + return passedThroughClass ? null : 'module'; +} - return [...keys]; +function collectHttpxImportAliases(tree: Parser.Tree): { + moduleAliases: Set; + asyncClientAliases: Set; +} { + const moduleAliases = new Set(['httpx']); + const asyncClientAliases = new Set(); + + // The @module capture is a single identifier inside a `dotted_name`, so for + // `import package.httpx as hx` the pattern would match the inner `httpx` + // segment. Check the full `dotted_name` text via `parent` to anchor the match. + for (const match of runCompiledPatterns(HTTPX_MODULE_IMPORT_PATTERNS, tree)) { + const moduleNode = match.captures.module; + const aliasNode = match.captures.alias; + if (moduleNode?.parent?.text === 'httpx' && aliasNode) moduleAliases.add(aliasNode.text); + } + + for (const match of runCompiledPatterns(HTTPX_ASYNC_CLIENT_IMPORT_PATTERNS, tree)) { + const moduleNode = match.captures.module; + const classNode = match.captures.client_class; + if (moduleNode?.parent?.text !== 'httpx' || classNode?.text !== 'AsyncClient') continue; + asyncClientAliases.add(match.captures.alias?.text ?? classNode.text); + } + + return { moduleAliases, asyncClientAliases }; +} + +// Tracks local rebindings (`AsyncClient = ...`, `hx = ...`) that shadow an +// imported alias. We treat the whole enclosing scope (module, class, or +// function) as shadowed for that alias name, so subsequent constructions in +// that scope are not falsely detected as httpx consumers. Covers bare-identifier +// targets and the common tuple / list destructuring shapes. +const ALIAS_SHADOW_PATTERNS = compilePatterns({ + name: 'python-httpx-alias-shadow', + language: Python, + patterns: [ + { + meta: {}, + query: `(assignment left: (identifier) @name)`, + }, + { + meta: {}, + query: `(assignment left: (pattern_list (identifier) @name))`, + }, + { + meta: {}, + query: `(assignment left: (tuple_pattern (identifier) @name))`, + }, + { + meta: {}, + query: `(assignment left: (list_pattern (identifier) @name))`, + }, + ], +} satisfies LanguagePatterns>); + +function collectAliasShadowScopes( + tree: Parser.Tree, + aliases: Set, +): Map> { + const shadowed = new Map>(); + if (aliases.size === 0) return shadowed; + + for (const match of runCompiledPatterns(ALIAS_SHADOW_PATTERNS, tree)) { + const nameNode = match.captures.name; + if (!nameNode || !aliases.has(nameNode.text)) continue; + const scopeKey = shadowScopeKey(nameNode.parent); + if (scopeKey === null) continue; + const set = shadowed.get(nameNode.text) ?? new Set(); + set.add(scopeKey); + shadowed.set(nameNode.text, set); + } + + return shadowed; +} + +function isAliasShadowed( + shadowed: Map>, + aliasName: string, + node: Parser.SyntaxNode, +): boolean { + const scopes = shadowed.get(aliasName); + if (!scopes || scopes.size === 0) return false; + let current: Parser.SyntaxNode | null = node.parent; + while (current) { + if (current.type === 'function_definition') { + // Reuse getScopeKey's key format so the two helpers cannot drift apart. + if (scopes.has(getScopeKey(current))) return true; + } + current = current.parent; + } + // A module-level rebind shadows the alias for the entire file. + return scopes.has('module'); } function collectHttpxAsyncClients(tree: Parser.Tree): Map> { const clients = new Map>(); + const { moduleAliases, asyncClientAliases } = collectHttpxImportAliases(tree); + // Module aliases (`hx`) and AsyncClient aliases (`AsyncClient`, + // `HttpxAsyncClient`) share disjoint name spaces, so one shadow map keyed by + // alias name serves both lookups and we only walk the tree for rebinds once. + const shadowed = collectAliasShadowScopes( + tree, + new Set([...moduleAliases, ...asyncClientAliases]), + ); const addClient = (clientNode: Parser.SyntaxNode | undefined) => { if (!clientNode) return; @@ -172,10 +367,34 @@ function collectHttpxAsyncClients(tree: Parser.Tree): Map> { }; for (const match of runCompiledPatterns(HTTPX_ASYNC_CLIENT_ASSIGN_PATTERNS, tree)) { + const moduleNode = match.captures.module; + const classNode = match.captures.client_class; + if (!moduleNode || !classNode) continue; + if (!moduleAliases.has(moduleNode.text) || classNode.text !== 'AsyncClient') continue; + if (isAliasShadowed(shadowed, moduleNode.text, moduleNode)) continue; + addClient(match.captures.client); + } + + for (const match of runCompiledPatterns(HTTPX_ASYNC_CLIENT_DIRECT_ASSIGN_PATTERNS, tree)) { + const classNode = match.captures.client_class; + if (!classNode || !asyncClientAliases.has(classNode.text)) continue; + if (isAliasShadowed(shadowed, classNode.text, classNode)) continue; addClient(match.captures.client); } for (const match of runCompiledPatterns(HTTPX_ASYNC_CLIENT_WITH_ALIAS_PATTERNS, tree)) { + const moduleNode = match.captures.module; + const classNode = match.captures.client_class; + if (!moduleNode || !classNode) continue; + if (!moduleAliases.has(moduleNode.text) || classNode.text !== 'AsyncClient') continue; + if (isAliasShadowed(shadowed, moduleNode.text, moduleNode)) continue; + addClient(match.captures.client); + } + + for (const match of runCompiledPatterns(HTTPX_ASYNC_CLIENT_DIRECT_WITH_ALIAS_PATTERNS, tree)) { + const classNode = match.captures.client_class; + if (!classNode || !asyncClientAliases.has(classNode.text)) continue; + if (isAliasShadowed(shadowed, classNode.text, classNode)) continue; addClient(match.captures.client); } diff --git a/gitnexus/test/unit/group/http-route-extractor.test.ts b/gitnexus/test/unit/group/http-route-extractor.test.ts index d2c1b3fa4..a1756be3b 100644 --- a/gitnexus/test/unit/group/http-route-extractor.test.ts +++ b/gitnexus/test/unit/group/http-route-extractor.test.ts @@ -538,8 +538,29 @@ def create_order(): path.join(dir, 'src', 'client.py'), ` import httpx +import httpx as hx +from httpx import AsyncClient +from httpx import AsyncClient as HttpxAsyncClient + +# Dotted-package look-alikes — must NOT be detected as httpx. +import my_pkg.httpx as evil_mod +from my_pkg.httpx import AsyncClient as evil_async +# Longer dotted path — must also NOT be detected. +import a.b.c.httpx as deep_evil +from a.b.c.httpx import AsyncClient as deep_evil_async +# Relative import — module_name is a relative_import node, not dotted_name, so +# it must not produce a contract either. +from .httpx import AsyncClient as rel_evil_async module_client = httpx.AsyncClient(base_url="https://svc.local") +module_alias_client = hx.AsyncClient(base_url="https://svc.local") +module_direct_client = AsyncClient(base_url="https://svc.local") +module_renamed_client = HttpxAsyncClient(base_url="https://svc.local") +evil_mod_client = evil_mod.AsyncClient(base_url="https://svc.local") +evil_direct_client = evil_async(base_url="https://svc.local") +deep_evil_mod_client = deep_evil.AsyncClient(base_url="https://svc.local") +deep_evil_direct_client = deep_evil_async(base_url="https://svc.local") +rel_evil_direct_client = rel_evil_async(base_url="https://svc.local") class TopicClient: def __init__(self): @@ -561,6 +582,18 @@ async def check_duplicate(): service.request("POST", "/nope") return await client.post("https://svc.local/questions/duplicate-check") +async def import_aliases(): + local_alias_client = hx.AsyncClient(base_url="https://svc.local") + local_direct_client = AsyncClient(base_url="https://svc.local") + local_renamed_client = HttpxAsyncClient(base_url="https://svc.local") + await local_alias_client.get("/alias-topic") + await local_direct_client.patch("/direct-topic") + await local_renamed_client.request("PUT", "/renamed-topic") + async with hx.AsyncClient() as alias_context: + await alias_context.delete("/alias-context") + async with AsyncClient() as direct_context: + return await direct_context.post("/direct-context") + def unrelated_scope_collision(): client = acquire_cache_client() return client.get("/ignored-same-name") @@ -569,7 +602,63 @@ def module_scope_shadow_collision(): client = acquire_cache_client() return client.get("/ignored-module-same-name") +def shadow_direct_alias(): + AsyncClient = lambda: FakeClient() + client = AsyncClient() + return client.get("/shadow-direct-fp") + +def shadow_module_alias(): + hx = FakeMod() + client = hx.AsyncClient() + return client.get("/shadow-module-fp") + +async def shadow_direct_context(): + AsyncClient = lambda: FakeClient() + async with AsyncClient() as client: + return await client.get("/shadow-direct-context-fp") + +def shadow_tuple_destructure(): + AsyncClient, _other = (lambda: FakeClient()), 42 + client = AsyncClient() + return client.get("/shadow-tuple-fp") + +# Class-body assignment of an imported alias is a class attribute under Python +# LEGB rules — methods inside still see the module binding. The detector must +# NOT poison the methods, so the legitimate httpx call below should still emit. +class ClassBodyRebindHolder: + AsyncClient = lambda: FakeClient() + + def __init__(self): + self._client = httpx.AsyncClient(base_url="https://svc.local") + + async def fetch(self): + return await self._client.get("/class-body-rebind-ok") + module_client.get("/module-topic") +module_alias_client.get("/module-alias-topic") +module_direct_client.get("/module-direct-topic") +module_renamed_client.get("/module-renamed-topic") +evil_mod_client.get("/evil-module-dotted-fp") +evil_direct_client.get("/evil-direct-dotted-fp") +deep_evil_mod_client.get("/deep-evil-module-dotted-fp") +deep_evil_direct_client.get("/deep-evil-direct-dotted-fp") +rel_evil_direct_client.get("/rel-evil-direct-fp") +`, + ); + + // Isolated file for module-level rebind: shadowing applies file-wide, so + // it must not affect the assertions in client.py above. + fs.writeFileSync( + path.join(dir, 'src', 'module_rebind.py'), + ` +from httpx import AsyncClient + +# Module-level rebind: the rest of this file's bare AsyncClient calls must NOT +# emit httpx consumer contracts. +AsyncClient = lambda: FakeClient() + +shadowed_module_client = AsyncClient(base_url="https://svc.local") +shadowed_module_client.get("/module-level-rebind-fp") `, ); @@ -581,7 +670,19 @@ module_client.get("/module-topic") 'http::POST::/questions/import', 'http::DELETE::/topic', 'http::POST::/questions/duplicate-check', + 'http::GET::/alias-topic', + 'http::PATCH::/direct-topic', + 'http::PUT::/renamed-topic', + 'http::DELETE::/alias-context', + 'http::POST::/direct-context', 'http::GET::/module-topic', + 'http::GET::/module-alias-topic', + 'http::GET::/module-direct-topic', + 'http::GET::/module-renamed-topic', + // Class-body rebind of `AsyncClient` is a class attribute, not a + // method-scope shadow — the legitimate httpx.AsyncClient call inside + // the class must still emit. + 'http::GET::/class-body-rebind-ok', ]; for (const contractId of expected) { @@ -590,6 +691,13 @@ module_client.get("/module-topic") expect(consumer?.meta.framework).toBe('python-httpx'); } + // Positive control: the legitimate `module_direct_client = AsyncClient(...)` + // path was actually exercised, so the negative dotted-package assertions + // below are not passing vacuously. + expect( + consumers.find((c) => c.contractId === 'http::GET::/module-direct-topic'), + ).toBeDefined(); + expect(consumers.find((c) => c.contractId === 'http::GET::/nope')).toBeUndefined(); expect(consumers.find((c) => c.contractId === 'http::POST::/nope')).toBeUndefined(); expect( @@ -598,6 +706,39 @@ module_client.get("/module-topic") expect( consumers.find((c) => c.contractId === 'http::GET::/ignored-module-same-name'), ).toBeUndefined(); + // Finding 1: dotted-package look-alikes (`my_pkg.httpx`, three-segment + // `a.b.c.httpx`, and relative `.httpx`) must not be detected. + expect( + consumers.find((c) => c.contractId === 'http::GET::/evil-module-dotted-fp'), + ).toBeUndefined(); + expect( + consumers.find((c) => c.contractId === 'http::GET::/evil-direct-dotted-fp'), + ).toBeUndefined(); + expect( + consumers.find((c) => c.contractId === 'http::GET::/deep-evil-module-dotted-fp'), + ).toBeUndefined(); + expect( + consumers.find((c) => c.contractId === 'http::GET::/deep-evil-direct-dotted-fp'), + ).toBeUndefined(); + expect( + consumers.find((c) => c.contractId === 'http::GET::/rel-evil-direct-fp'), + ).toBeUndefined(); + // Finding 2: locally rebound imported aliases must not be detected. + expect( + consumers.find((c) => c.contractId === 'http::GET::/shadow-direct-fp'), + ).toBeUndefined(); + expect( + consumers.find((c) => c.contractId === 'http::GET::/shadow-module-fp'), + ).toBeUndefined(); + expect( + consumers.find((c) => c.contractId === 'http::GET::/shadow-direct-context-fp'), + ).toBeUndefined(); + // Tuple/list destructuring rebinds must also shadow the alias. + expect(consumers.find((c) => c.contractId === 'http::GET::/shadow-tuple-fp')).toBeUndefined(); + // Module-level rebind in a separate file must shadow the whole file. + expect( + consumers.find((c) => c.contractId === 'http::GET::/module-level-rebind-fp'), + ).toBeUndefined(); }); it('extracts Java RestTemplate, WebClient and OkHttp calls', async () => {