feat(locations): LocationData.code factory + get_locations() hash symmetry + location docs#15171
Draft
Maffooch wants to merge 5 commits into
Draft
feat(locations): LocationData.code factory + get_locations() hash symmetry + location docs#15171Maffooch wants to merge 5 commits into
Maffooch wants to merge 5 commits into
Conversation
…metry Adds a LocationData.code() factory so parsers can emit static-analysis code coordinates as first-class locations: identity is file_path (+ line), while volatile context (snippet, end_line, sink/source metadata) is carried separately and omitted when unset, so downstream location subtypes can route it onto the finding reference instead of the location identity. Also fixes an asymmetry in Finding.get_locations(): the saved path filters a finding's location references to URL type before feeding the "endpoints" hash ingredient, but the unsaved path hashed every unsaved location type (dependencies included). A finding whose hash was computed before save could therefore never match its own hash recomputed after save. The unsaved path now applies the same URL-type filter. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tching Two new pages plus cross-links from the existing docs: - asset_modelling/locations/PRO__source_code_locations.md — the Code location subtype: file+line identity, scan-managed lifecycle, the All Source Code / asset-scoped views, the movement-history trail, and the toggle-independent reference status sync. - triage_findings/finding_deduplication/PRO__location_drift_matching.md — the full feature guide: the churn problem, enabling the per-tool toggle, the two-stage matching model (stable identity + evidence passes per location type), severity re-score tolerance, scan-owned field refresh with human-edit protection, scan-pure hash adoption, location history, upgrade/enablement guidance for existing data, hash-field guidance (all-volatile fallback, location-in-title), the dependency dedupe stable-key interaction, the historical churn consolidation command, and the safeguards/limits. - PRO__deduplication_tuning.md — Content Fingerprint hash field (with the backfill-before-select ordering) and the Track Findings as Locations Change toggle, plus best-practice entries. - PRO__locations_overview.md — Source Code Locations join the subtype list. - import_data/import_intro/reimport.md — cross-link from the "line number shift" troubleshooting note to the drift matching page. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Follows the method PR #14395 established for dependencies: rather than downstream synthesis from finding scalars alone, static parsers now emit their code coordinates as LocationData.code() into unsaved_locations, gated on V3_FEATURE_LOCATIONS, alongside the unchanged scalar fields. Context the parser already extracts rides along (snippet, end_line, source/sink objects, taint source file/line) so location subtypes can route it onto the finding reference instead of the location identity. Coverage: 45 parser directories converted (~60 emission sites) — SAST, secret scanners, and IaC scanners. SCA parsers are deliberately skipped (their file_path is a dependency manifest; they already emit LocationData.dependency), as are dynamic findings and non-source paths (container-image filesystem hits, JIRA/Confluence URLs, cloud service names). Mixed parsers (trivy, checkmarx, aws_inspector2, wizcli, xygeni, mobsf, deepfence, rusty_hog, snyk_issue_api) emit only for their code-shaped findings. Multi-location: SARIF explodes results into one finding per location, each with its own emission; xygeni secrets emits one location per leaked line. Fabricated line numbers (trufflehog's line=0 dedup placeholder) are never propagated — real coordinates only. Secret scanners never pass matched secret content as snippets. unittests/test_code_location_emission.py exercises nine representative parsers against real fixtures: emission coherence with the finding scalars, snippet context, SARIF per-location behavior, and a gate-off class proving the sweep is inert without V3 (full unittests.tools suite: 1531 tests green with V3 off). Known follow-ups (out of scope): mend and meterian have no dependency emission from #14395 either; github_vulnerability's code-scanning branch parses no location fields at all today. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ilot scalars
CI (the V3-on rest-framework matrix variant) caught two coupled defects the
local V3-off run could not, since the emission only fires under V3:
- LocationData.code filtered unset context with `value not in {"", None}` — a
SET membership test that hashes `value`, so any unhashable value crashed with
"cannot use 'list' as a set element". Switched to tuple membership (`in
("", None)`), which compares by equality and never hashes — robust for every
call site, present and future.
- progpilot reports taint-source fields as single-element arrays
(source_name=["$sql"], source_line=[610]); the sweep passed those lists
straight into the context kwargs. Now collapsed to scalars via a _first()
helper, with str()/int guards.
Audited every other converted parser's context kwargs: all pass .text/int/
string scalars (checkmarx pathnode elements, sarif get_snippet, etc.) — progpilot
was the only list-valued source. Verified against the full unittests.tools suite
run with V3_FEATURE_LOCATIONS=True.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the crash-prone set-membership filter with a truthiness dict comprehension: never hashes the value (crash-robust for any parser input), and satisfies the repo's ruff rules (no manual loop, no != "" comparison). Drops None/""/0/empty — all "no data" for the code location's context fields. Same fix, lint-clean form. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
1 similar comment
Contributor
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Shortcut: sc-13523
What & why
Two changes that let static-analysis code coordinates participate in the Locations model, plus documentation for the location-tracking features built on top.
LocationData.code()factory (dojo/tools/locations.py)Parsers can emit static-analysis code coordinates as first-class locations: identity is
file_path(+line), while volatile context (snippet, end_line, sink/source metadata) is carried separately and omitted when unset, so downstream location subtypes can route it onto the finding reference instead of the location identity.Finding.get_locations()hash symmetry fix (dojo/finding/models.py)The saved path filters a finding's location references to URL type before feeding the "endpoints" hash ingredient, but the unsaved path hashed every unsaved location type. A finding whose hash was computed before save could therefore never match its own hash recomputed after save. The unsaved path now applies the same URL-type filter.
Stored-hash impact: none. Audited every producer of
unsaved_locations: no current import path places non-URL location objects there at hash-computation time (URL conversions only; dependency location data is carried asLocationDatawrappers that the type registry does not convert; code locations are routed separately). The fix therefore changes no hash any production flow computes today — it is purely protective against future location types leaking into the endpoints ingredient.Parsers emit
LocationData.code(the #14395 method, applied to code)Following the approach #14395 established for dependencies, static parsers now emit their code coordinates as
LocationData.code()intounsaved_locations— gated onV3_FEATURE_LOCATIONS, scalars unchanged. 45 parser directories converted (~60 emission sites): SAST, secret scanners, and IaC scanners. Deliberate exclusions: SCA parsers (theirfile_pathis a dependency manifest and they already emitLocationData.dependency), dynamic findings, and non-source paths (container-image filesystem hits, issue-tracker URLs, cloud service names). Mixed parsers emit only for their code-shaped findings. Context the parser already extracts rides along (snippet, end line, source/sink objects, taint source coordinates); fabricated line numbers (e.g. trufflehog'sline=0placeholder) are never propagated, and secret scanners never pass matched secret content as snippets.Documentation
New pages for Source Code Locations and location drift matching (tracking findings whose location moves between reimports), plus updates to the Locations overview, deduplication tuning, and reimport pages.
Testing
unittests/test_location_data.py(new, 7 tests): factory identity/context semantics and pre/post-save hash symmetry, including the mixed-location case.unittests/test_code_location_emission.py(new, 12 tests): nine representative parsers against real fixtures — emission coherence with the finding scalars, snippet context, SARIF per-location behavior, and a gate-off class proving the sweep is inert without V3.unittests.toolssuite: 1531 tests green with V3 off (the sweep changes nothing behind the gate).devunder identical conditions with identical results.