mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-21 00:21:30 +00:00
* fix(query): stop impact()/context() under-reporting blast radius (#2129, #1858) Two read-side fixes to the "run impact before editing" safety workflow, both about the tools rendering "I could not give a single confident answer" as "no impact" — the most dangerous failure mode for a refactor-safety tool. #2129 — ambiguous resolution no longer hides a real caller behind a bare `impactedCount: 0`. When a bare name collides with several symbols, the resolver returns `ambiguous`; previously the payload carried a flat `impactedCount: 0`, so the real caller (which calls a *different* same-name node) was invisible unless the user already knew to disambiguate. The ambiguous branch now runs a bounded, summary-only BFS per candidate (capped at 6) and surfaces each candidate's true count plus the top-level `maxImpactedCount` / `maxRisk`, ranked most-impactful-first. `risk` stays `UNKNOWN` (ambiguity must not read as "safe"), `impactedCount` stays 0 (no single resolved symbol). The BFS and edge storage are unchanged — an empirical repro confirmed they are correct; the bug was purely in how the ambiguous case reported. Disambiguation by uid still returns the exact result. #1858 — impact()/context() now carry an additive `epistemic` field. When the queried symbol sits on an interface / indirection boundary (it implements or extends an interface, or is one) whose consumers bind via a DI container or dynamic dispatch, those callers are not traced to the concrete symbol, so the count is a lower bound. The result is annotated `epistemic: 'lower-bound'` with a human-readable `boundaries[]` note; a fully resolved leaf stays `epistemic: 'exact'`. Aligned to the surviving numeric confidence model (the 0.85 IMPACT_RELATION_CONFIDENCE heritage floor), not the long-deleted TIER_CONFIDENCE enum. Purely additive — no existing field or count changes. Tests: impact-ambiguous-blast-radius (per-candidate surfacing + uid disambiguation) and impact-epistemic-lower-bound (interface boundary → lower-bound, resolved leaf → exact, context parity). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(routes): configurable fetch wrappers + faster consumer scan (#1589/#1852) Closes the residual gap behind the now-merged #1852 (which fixed #1589): the fetch-wrapper consumer scan only traced wrappers the parse phase auto-detected as calling the bare global `fetch()`. A wrapper built on axios / a custom client, or one named outside the built-in convention, was invisible — route_map silently returned `consumers: []` (the exact "named outside convention → silent zero" hole #1858 calls out as needing a backstop). - Configurable wrappers: `.gitnexusrc` gains a `fetchWrappers: [...]` list (validated as identifier/member names, de-duped, capped, regex-safe), threaded AnalyzeOptions → PipelineOptions → routes phase. Configured names are unioned with the auto-detected ones; configured names alone now trigger the scan even when nothing was auto-detected. - Perf (F3 from #1852's review): the cross-file scan built one RegExp per (file × wrapper) — O(files × wrappers). It now builds a single alternation regex per file (O(files)) and reuses file contents already read for handler extraction instead of re-reading them. Tests: configurable-fetch-wrapper (axios-based `doRequest` wrapper — invisible without config, traced with it) + .gitnexusrc `fetchWrappers` validation cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(review): harden the under-reporting fixes after adversarial review Addresses findings from a reviewer-swarm pass over the two prior commits: - CLI text false-safe (major): `formatImpactResult` (eval-server.ts) had no ambiguous branch, so `gitnexus impact <colliding-name>` printed "No dependencies found. This symbol appears isolated." for an ambiguous target — the exact false-safe #2129 exists to kill, defeating the JSON-layer fix at the text surface. Added an ambiguous branch (per-candidate blast radius + maxImpactedCount/maxRisk) and a lower-bound branch for both the zero-count and non-zero paths, mirroring the context formatter. Covered by new unit tests. - Group fan-out dead work (major): impactByUid now passes skipEpistemic:true — the group cross-impact fan-out consumes only byDepth, so computing the #1858 boundary per neighbor was wasted round-trips on the highest-volume path. - Ambiguous all-UNKNOWN risk (minor): if every per-candidate probe fails, maxRisk now reports 'UNKNOWN' instead of falling to the 'LOW' seed (which would read as "safe"). - Candidate-probe cost (minor): the per-candidate summary BFS now sets skipEnrichment:true, bypassing the process/module aggregation passes it does not use. - Epistemic latency (minor): computeEpistemicBoundary now runs concurrently with the impact BFS instead of as a trailing serial round-trip. - Wrapper over-match (minor): the consumer-scan regex uses a `(?<![.\w$])` lookbehind instead of `\b`, so a bare configured name like `get` matches the free call `get('/x')` but not a member access `client.get(` (and `apiFetch` no longer matches `myApiFetch`). - Boundary wording (nit): correct article ("a class" vs "an interface") and singular/plural ("1 implementation"). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(lint): drop unused describe import in new impact tests The withTestLbugDB harness wraps describe internally, so the explicit describe import was unused — unused-imports/no-unused-imports is an error (not a warning) in the root eslint config, failing quality/lint. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(query): flag partialProbe when an ambiguous candidate probe fails (#2129 review F1) The ambiguous-impact branch hoists maxRisk/maxImpactedCount so a colliding name can't read as "isolated". But if a per-candidate BFS throws (e.g. DB pool contention during the ≤6-way fan-out), it was recorded as risk:'UNKNOWN', impactedCount:0 and silently masked by any benign sibling success — maxRisk reduced to the benign tier and maxImpactedCount reflected only successful probes. Track probeFailed and surface partialProbe:true (additive, intentionally distinct from the traversal-interrupted `partial` flag); formatImpactResult prints a lower-bound warning. Covered by a formatter unit test (a natural in-harness probe throw is unreachable — _runImpactBFS is fully self-catching under summaryOnly+skipEpistemic+ skipEnrichment). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(query): report the full match count when ambiguous candidates are truncated (#2129 review F11) The ambiguous candidate list is capped at AMBIGUOUS_MAX_CANDIDATES (6), but the CLI headline read the truncated `candidates[]` length — so a name matching 9 symbols printed "6 symbols share this name" while the JSON message stated the true count. Add an additive `totalCandidates` field carrying the full match count, include a "showing N of M" clause in the message when truncated, and have formatImpactResult report the full count. Covered by formatter unit tests for the truncated and non-truncated cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(query): run context() epistemic probe concurrently with methodMetadata (#1858 review F2) impact() overlaps the #1858 boundary probe with its BFS, but _contextImpl awaited computeEpistemicBoundary serially after every other query. Start the probe right after `symKind` is known (the earliest point it can — symKind depends on the incoming/outgoing round-trips) so it runs concurrently with the methodMetadata fetch, and await it at result assembly. Output is unchanged (covered by the existing epistemic context() tests). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(query): flag a leaf interface as lower-bound in context() (#1858 review F3) context() passed `symKind` to computeEpistemicBoundary, but symKind collapses a single-resolved Interface to 'Class' (resolvedLabel is '' on the single-candidate path), so the `symType === 'Interface'` self-boundary branch never fired and a directly-queried leaf interface (implements nothing, but consumed) was under-reported as 'exact'. Pass an interface-preserving type (`resolvedLabel || sym.type || symKind`) instead — enrichCandidateLabels runs before the single-candidate early return and patches sym.type to 'Interface', mirroring impact()'s derivation. impact() was already unaffected. Covered by a new context()-on-a-leaf-interface test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(query): hoist epistemic relation-type lists + add USES to the allowlist (#1858/#2129 review F4, F5) F4: promote computeEpistemicBoundary's function-local heritage/consumer relation-type lists to module-level readonly constants (EPISTEMIC_HERITAGE_RELATION_TYPES / EPISTEMIC_CONSUMER_RELATION_TYPES) next to VALID_RELATION_TYPES / IMPACT_RELATION_CONFIDENCE, so a future heritage edge type is visible to the probe. Kept as arrays (not Sets) because they bind as Cypher params. F5 (latent bug): USES is emitted (emit-references.ts) and already in the default impact relTypes + context() queries, but was missing from VALID_RELATION_TYPES — so impact({relationTypes:['USES']}) filtered to [] and silently ran the full default traversal. Add it (0.5 confidence fallback, matching FETCHES/WRAPS). Updates the security.test.ts allowlist assertions (size 15→16, USES now valid). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(query): document the _runImpactBFS enrichment skip-flag composition (#1858/#2129 review F6) The three skip-flags (skipPerSymbolEnrichment / skipEpistemic / skipEnrichment) suppress distinct sub-phases and compose implicitly. Add a JSDoc block at the opts type listing what each suppresses, the three real call patterns, and the key interaction (skipEnrichment makes skipPerSymbolEnrichment a no-op). Comment-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(cli): genericize the shared string-array validation messages (#1589/#1852 review F7) The shared `string-array` ValueKind hardcoded fetch-wrapper phrasing in three messages (non-array, identifier-shape, empty-list). Since `source` already names the config key, genericize all three so the shared normalizer carries no fetchWrappers coupling — a future string-array config key gets sensible errors. Test assertions updated to the new wording. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(query): type the ambiguous candidate summary + epistemicPromise (#1858/#2129 review F8) The ambiguous per-candidate summary was read through `any`, so a rename of _runImpactBFS's return fields would silently zero candidate counts. Name the read shape ({impactedCount, risk, summary?.direct}) at the narrowing site, and type epistemicPromise as the optional-epistemic union (the skip case's `{}` subtype) — keeping computeEpistemicBoundary's own return precise (epistemic required). Type-only; no runtime change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(routes): trust validated fetchWrappers config, drop redundant re-filter (#1589/#1852 review F9) `ctx.options.fetchWrappers` is already trimmed/shape-validated/de-duped/capped in analyze-config.ts, so the routes-phase re-trim/re-typeof pre-pass was redundant. Pass it straight through; the single Set-construction filter remains to guard the auto-detected functionName values (which don't pass through analyze-config). No behavior change — covered by the existing fetch-wrapper route suites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(routes): make the wrapper-call boundary Unicode-aware (#1852 review F10) The consumer-scan lookbehind used ASCII `\w`, so a configured bare wrapper name preceded by a non-ASCII identifier character (`caféget('/x')`) satisfied the boundary and produced a spurious FETCHES edge. Switch to the `u` flag with Unicode property classes (`(?<![.\p{L}\p{N}_$])`). Covered by a fixture consumer (`cafédoRequest('/api/things')`) asserting no spurious edge. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(routes): count wrapper-scan line numbers incrementally (#1852 review F12) The wrapper consumer scan computed each match's line number via content.substring(0, match.index).split('\n').length — an O(matchIndex) allocation per match. Matches arrive in ascending index, so accumulate newlines with a running counter instead. 1-based line numbers are byte-identical (covered by the existing fetch-wrapper route suites). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(test): keep the #1858 epistemic probe from skewing the impact-pagination mock The impact-pagination mock counts every query containing `r.type IN` as a BFS depth level. Once the #1858 epistemic boundary probe was parallelized with the BFS (it fires `MATCH (x)-[r]->(iface) ... r.type IN $heritage` before the frontier loop), that query was miscounted as depth-1, shifting the real depths so multi-depth impactedCount read 50 instead of 200. Short-circuit the epistemic queries (uniquely aliased `iface`) to empty in both mock setups so only frontier queries count. Test-only; production is unaffected (the epistemic query is a separate real query there). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
126 lines
4 KiB
TypeScript
126 lines
4 KiB
TypeScript
/**
|
|
* P0 Unit Tests: Security Hardening
|
|
*
|
|
* Tests security-related utility helpers in isolation:
|
|
* - Relation type allowlist
|
|
* - Path traversal detection
|
|
* - isTestFilePath patterns
|
|
*/
|
|
import { describe, it, expect } from 'vitest';
|
|
import {
|
|
VALID_RELATION_TYPES,
|
|
VALID_NODE_LABELS,
|
|
isTestFilePath,
|
|
} from '../../src/mcp/local/local-backend.js';
|
|
|
|
// ─── Relation type allowlist ──────────────────────────────────────────
|
|
|
|
describe('VALID_RELATION_TYPES', () => {
|
|
it('contains all expected relation types', () => {
|
|
expect(VALID_RELATION_TYPES.size).toBe(16);
|
|
for (const t of [
|
|
'CALLS',
|
|
'IMPORTS',
|
|
'EXTENDS',
|
|
'IMPLEMENTS',
|
|
'HAS_METHOD',
|
|
'HAS_PROPERTY',
|
|
'METHOD_OVERRIDES',
|
|
'OVERRIDES',
|
|
'METHOD_IMPLEMENTS',
|
|
'ACCESSES',
|
|
// USES is an emitted edge type (emit-references.ts) used in the default
|
|
// impact relTypes + context queries; added to the allowlist in F5.
|
|
'USES',
|
|
'HANDLES_ROUTE',
|
|
'FETCHES',
|
|
'HANDLES_TOOL',
|
|
'ENTRY_POINT_OF',
|
|
'WRAPS',
|
|
]) {
|
|
expect(VALID_RELATION_TYPES.has(t)).toBe(true);
|
|
}
|
|
});
|
|
|
|
it('rejects invalid relation types', () => {
|
|
expect(VALID_RELATION_TYPES.has('CONTAINS')).toBe(false);
|
|
expect(VALID_RELATION_TYPES.has('calls')).toBe(false); // case-sensitive
|
|
expect(VALID_RELATION_TYPES.has('DROP_TABLE')).toBe(false);
|
|
});
|
|
});
|
|
|
|
// ─── Valid node labels ───────────────────────────────────────────────
|
|
|
|
describe('VALID_NODE_LABELS', () => {
|
|
it('contains core node types', () => {
|
|
for (const label of [
|
|
'File',
|
|
'Folder',
|
|
'Function',
|
|
'Class',
|
|
'Interface',
|
|
'Method',
|
|
'CodeElement',
|
|
]) {
|
|
expect(VALID_NODE_LABELS.has(label)).toBe(true);
|
|
}
|
|
});
|
|
|
|
it('contains meta node types', () => {
|
|
for (const label of ['Community', 'Process']) {
|
|
expect(VALID_NODE_LABELS.has(label)).toBe(true);
|
|
}
|
|
});
|
|
|
|
it('contains multi-language node types', () => {
|
|
for (const label of ['Struct', 'Enum', 'Macro', 'Trait', 'Impl', 'Namespace']) {
|
|
expect(VALID_NODE_LABELS.has(label)).toBe(true);
|
|
}
|
|
});
|
|
|
|
it('rejects invalid labels', () => {
|
|
expect(VALID_NODE_LABELS.has('InvalidType')).toBe(false);
|
|
expect(VALID_NODE_LABELS.has('function')).toBe(false); // case-sensitive
|
|
});
|
|
});
|
|
|
|
// ─── Path traversal detection ────────────────────────────────────────
|
|
|
|
describe('path traversal (isTestFilePath as proxy for path handling)', () => {
|
|
it('isTestFilePath matches .test. files', () => {
|
|
expect(isTestFilePath('src/foo.test.ts')).toBe(true);
|
|
expect(isTestFilePath('src/foo.spec.ts')).toBe(true);
|
|
});
|
|
|
|
it('isTestFilePath matches __tests__ directory', () => {
|
|
expect(isTestFilePath('src/__tests__/foo.ts')).toBe(true);
|
|
});
|
|
|
|
it('isTestFilePath matches /test/ directory', () => {
|
|
expect(isTestFilePath('src/test/foo.ts')).toBe(true);
|
|
});
|
|
|
|
it('isTestFilePath handles Windows backslash paths', () => {
|
|
expect(isTestFilePath('src\\test\\foo.ts')).toBe(true);
|
|
expect(isTestFilePath('src\\__tests__\\bar.ts')).toBe(true);
|
|
});
|
|
|
|
it('isTestFilePath is case-insensitive', () => {
|
|
expect(isTestFilePath('SRC/TEST/Foo.ts')).toBe(true);
|
|
expect(isTestFilePath('SRC/Foo.Test.ts')).toBe(true);
|
|
});
|
|
|
|
it('isTestFilePath matches Go test files', () => {
|
|
expect(isTestFilePath('pkg/handler_test.go')).toBe(true);
|
|
});
|
|
|
|
it('isTestFilePath matches Python test files', () => {
|
|
expect(isTestFilePath('tests/test_handler.py')).toBe(true);
|
|
expect(isTestFilePath('pkg/handler_test.py')).toBe(true);
|
|
});
|
|
|
|
it('isTestFilePath returns false for non-test files', () => {
|
|
expect(isTestFilePath('src/main.ts')).toBe(false);
|
|
expect(isTestFilePath('src/utils/helper.ts')).toBe(false);
|
|
});
|
|
});
|