l3montree-dev / l3montree-dev/devguard

Init vulns from nearest git ancestor branch, not latest event

Open
#2,796 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Go
Stars
161
Forks
43
Avg merge
1d 8h
Merged PRs (30d)
37

Description

Problem

A VEX decision made on main isn't reliably respected: if some unrelated branch happens to log a later event, a brand-new branch cut from main inherits that unrelated branch's state instead of main's, since inheritance is ordered by CreatedAt, not git ancestry. Reproduced in tests/scan_integration_test.go:TestBranchInheritanceIgnoresGitAncestry: scan main → scan other-branch → accept on main → reopen on other-branch → scan new new-branch. Expected accepted (inherited from its real ancestor main), got open (inherited from other-branch merely because its event is newer).

DiffVulnsBetweenBranches (statemachine/dependency_vuln_statemachine.go:161) matches a vuln against candidates from all other asset versions (GetDependencyVulnsByOtherAssetVersions, called from services/scan_service.go:389 and :565), then replays their events sorted only by CreatedAt (:182-189). No git topology involved — a stale branch with a recent event can override state from the actual parent branch.

AssetVersion (database/models/asset_version_model.go:21) has no commit info, only Name/Slug/Type/DefaultBranch.

Proposal

The scanner CLI runs inside the CI job with the repo checked out, so it has real git history available — no need to reconstruct topology server-side from timestamps alone.

  • At scan time, CLI collects the last ~250 commit hashes of the current HEAD (git log -n 250 --format=%H) and sends them along with the scan request.
  • Store them on AssetVersion as RecentCommitHashes jsonb (ordered array, newest-first), capped/trimmed on write. Simpler than a Postgres array column to work with from Go/GORM, and a GIN index makes containment lookups fast.
  • Nearest-ancestor lookup in one SQL query, no app-side looping: jsonb_array_elements_text(...) WITH ORDINALITY on the scanning branch's list (rank by recency), join candidates using the ? containment operator (exact element match, no substring-collision risk), GROUP BY asset version taking MIN(ordinality), ORDER BY ascending, LIMIT 1 — the candidate sharing the commit closest to HEAD wins.
  • Use that single branch's events instead of the pooled/sorted set from all branches.
-- find the nearest-ancestor asset version for :target_asset_version_name
SELECT candidate.name AS nearest_branch, MIN(t.ord) AS closest_rank
FROM asset_versions target
CROSS JOIN LATERAL jsonb_array_elements_text(target.recent_commit_hashes) WITH ORDINALITY AS t(hash, ord)
JOIN asset_versions candidate
  ON candidate.asset_id = target.asset_id
 AND candidate.name <> target.name
 AND candidate.recent_commit_hashes ? t.hash
WHERE target.name = :target_asset_version_name
  AND target.asset_id = :asset_id
GROUP BY candidate.name
ORDER BY closest_rank ASC
LIMIT 1;

Storage cost

Measured against a live Postgres instance: a 250-entry jsonb array of 7-char short hashes costs ~2.8 KB per asset-version row, plus a GIN index (needed for fast ? containment lookups) at roughly similar per-row overhead. Scales linearly with asset-version count in the DB — e.g. ~28 MB data + ~28 MB index (~55-60 MB total) for 10,000 asset versions at the full 250-hash cap.

Open questions

  • Window sliding: once a branch (typically main) advances past the ~250-commit cap, the original fork commit ages out of its stored window even though it's still the true ancestor — the query then returns zero rows, indistinguishable from no relationship at all. Shelf life scales with the source branch's commit velocity, not the child branch's — an active main will silently stop being recognized as the ancestor of long-lived feature branches well before 250 commits pass on the feature branch itself. Needs either a much larger window, walking further back (e.g. to the merge-base with the default branch) on the CLI side, or accepting this as a known limitation with a defined fallback.
  • Tie-breaking: if two sibling branches share the fork commit at the same rank (equally active, forked from the same point), the query returns both as ties — LIMIT 1 picks one non-deterministically. Needs an explicit tiebreaker (e.g. prefer DefaultBranch, or most-recently-scanned candidate).
  • Fallback when no common commit is found within the stored window (current behavior? DefaultBranch?)
  • CLI/API surface: new field on the scan request payload, size/perf impact of sending hashes on every scan.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with TestBranchInheritanceIgnoresGitAncestry in tests/scan_integration_test.go, then trace DiffVulnsBetweenBranches in statemachine/dependency_vuln_statemachine.go and the scan paths in services/scan_service.go. Review AssetVersion in database/models/asset_version_model.go and the scan request flow before resolving the commit-window, tie-breaking, fallback, and storage questions. Done means new branches inherit VEX state from the nearest git ancestor and the integration test passes.

Written by the indexing model from the issue text.

Assessment

Tech stack
git, go, postgresql
Domain
backend, cli, database
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.