mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-28 06:22:22 +00:00
fix(mcp): report cross-function explain lines 1-based
The interprocedural branch of explain passed the endpoints' and hops' graph startLine through unchanged. Those are 0-based (tree-sitter rows), so every cross-function source, sink and hop sat one line above the function's declaration in an editor, while the intra-procedural findings of the same response were already 1-based. source.line, sink.line and hops[].line now go through toDisplayLine like context, query and impact (#2377), and the response note says these are declaration lines. The interproc integration block asserts the exact fixture lines: handle on line 7 of source.ts, runIt on line 6 of sink.ts. Fixes #3491
This commit is contained in:
parent
89602560d3
commit
1e7e8efa70
2 changed files with 26 additions and 4 deletions
|
|
@ -6139,15 +6139,27 @@ export class LocalBackend {
|
|||
const total = Number((ipCountRows[0] as any)?.total ?? (ipCountRows[0] as any)?.[0] ?? 0);
|
||||
const findings = ipRows.map((r: any) => {
|
||||
const decoded = decodeTaintPath(r.reason ?? r[5]);
|
||||
// Cross-function lines are the endpoints' and hops' 0-based symbol
|
||||
// `startLine` (the Function node property, and what interproc-emit
|
||||
// encodes into the path codec), unlike the intra-procedural rows
|
||||
// above, whose PDG statement lines are already 1-based. Convert at
|
||||
// the response boundary like `context`/`query`/`impact` (#2377), so
|
||||
// both layers of one response agree with the editor (#3491).
|
||||
const hops = decoded.ok
|
||||
? decoded.hops.map((h) => ({ function: h.variable, line: h.line }))
|
||||
? decoded.hops.map((h) => ({ function: h.variable, line: toDisplayLine(h.line) }))
|
||||
: [];
|
||||
return {
|
||||
interprocedural: true,
|
||||
file: String(r.file ?? r[0] ?? ''),
|
||||
sinkKind: decoded.ok ? (decoded.kind ?? 'unknown') : 'unknown',
|
||||
source: { function: String(r.sourceFn ?? r[1] ?? ''), line: r.sourceLine ?? r[2] },
|
||||
sink: { function: String(r.sinkFn ?? r[3] ?? ''), line: r.sinkLine ?? r[4] },
|
||||
source: {
|
||||
function: String(r.sourceFn ?? r[1] ?? ''),
|
||||
line: toDisplayLine(r.sourceLine ?? r[2]),
|
||||
},
|
||||
sink: {
|
||||
function: String(r.sinkFn ?? r[3] ?? ''),
|
||||
line: toDisplayLine(r.sinkLine ?? r[4]),
|
||||
},
|
||||
hops,
|
||||
...(decoded.ok && decoded.truncated ? { pathIncomplete: true } : {}),
|
||||
};
|
||||
|
|
@ -6262,7 +6274,7 @@ export class LocalBackend {
|
|||
findings: pageFindings,
|
||||
totalFindings: totalFindings + interprocTotal,
|
||||
...(truncated ? { truncated: true } : {}),
|
||||
note: 'Intra-procedural (TAINTED, statement hops) AND cross-function (TAINT_PATH, function hops, `interprocedural: true`) flows are modeled, including sources read inside inline callbacks and returned handlers. Captured closure-variable, property/field, and implicit flows are NOT modeled; absence of a finding is not proof of safety. Cross-function findings are context-insensitive and may over-attribute among same-named callees. SANITIZES (kill) edges are queryable via cypher.',
|
||||
note: 'Intra-procedural (TAINTED, statement hops) AND cross-function (TAINT_PATH, function hops, `interprocedural: true`) flows are modeled, including sources read inside inline callbacks and returned handlers. Captured closure-variable, property/field, and implicit flows are NOT modeled; absence of a finding is not proof of safety. Cross-function findings are context-insensitive and may over-attribute among same-named callees. All line numbers are 1-based; for cross-function findings, `source.line`, `sink.line` and `hops[].line` are the declaration lines of the functions, not statement lines. SANITIZES (kill) edges are queryable via cypher.',
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -530,6 +530,16 @@ withTestLbugDB(
|
|||
expect(hr!.sinkKind).toBe('command-injection');
|
||||
expect(Array.isArray(hr!.hops)).toBe(true);
|
||||
expect((hr!.hops as unknown[]).length).toBeGreaterThan(0);
|
||||
// Lines are 1-based declaration lines, as in the editor: `handle` is
|
||||
// declared on line 7 of source.ts and `runIt` on line 6 of sink.ts
|
||||
// (#3491 — the graph stores 0-based startLine; before the fix these
|
||||
// read 6 and 5, one above every intra-procedural line in the same
|
||||
// response).
|
||||
expect(hr!.source).toEqual({ function: 'handle', line: 7 });
|
||||
expect(hr!.sink).toEqual({ function: 'runIt', line: 6 });
|
||||
const hops = hr!.hops as Array<{ function: string; line: number }>;
|
||||
expect(hops[0]).toEqual({ function: 'handle', line: 7 });
|
||||
expect(hops.at(-1)).toEqual({ function: 'runIt', line: 6 });
|
||||
});
|
||||
|
||||
it('symbol-anchored on the sink function surfaces the cross-function finding', async () => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue