Skip to content

feat(checks): flag a lockfile change with no manifest change - #19

Merged
bkd-dotcom merged 1 commit into
mainfrom
feat/dependency-skew-check
Aug 18, 2026
Merged

feat(checks): flag a lockfile change with no manifest change#19
bkd-dotcom merged 1 commit into
mainfrom
feat/dependency-skew-check

Conversation

@bkd-dotcom

Copy link
Copy Markdown
Member

Closes #16.

What I found on the way in

The gate this issue feeds was dead. supply.dependency_skew is referenced in three places — the gate report (__init__.py), the verdict logic (verdict.py), and the rendered comment (render.py) — but no scanner ever produced it.

So every review comment this project has ever posted printed:

| Dependency skew | ✅ ok |

…from a check that did not exist. This issue is that check.

The check

scan_dependency_skew flags a lockfile that moved without its sibling manifest:

ecosystem lockfile manifest
npm / yarn / pnpm / bun package-lock.json, npm-shrinkwrap.json, yarn.lock, pnpm-lock.yaml, bun.lockb package.json
Python uv.lock, poetry.lock / Pipfile.lock pyproject.toml / Pipfile
Rust Cargo.lock Cargo.toml
Go go.sum go.mod
PHP composer.lock composer.json
Ruby Gemfile.lock Gemfile

Matching is per-directory, so a monorepo's packages/a/package-lock.json is not satisfied by packages/b/package.json. Comparison is case-insensitive (Cargo.lock, Gemfile.lock, Pipfile.lock).

Advisory, not blocking — and why that needed a second fix

Resolved versions moving with no declared intent is the shape of a dependency-substitution attack. It's also exactly what npm audit fix, cargo update and Dependabot do, legitimately, all the time.

The old gate wiring ignored the blocking flag:

dep_ok = not any(f.id == "supply.dependency_skew" for f in findings)

unlike secret_scan_clean and no_forbidden_perm_change, which both key off blocking. Because a failed gate produces verdict=BLOCK / "not mergeable until resolved", emitting this finding under the old wiring would have hard-blocked every Dependabot PR. Fixed to match the other two gates.

The safety property survives without blocking

I checked rather than assumed. auto_merge_eligible requires worst.rank <= LOW, and this finding is MEDIUM:

change verdict worst auto-merge (opted in)
lockfile only safe medium False
lockfile + manifest safe info True

So a lockfile-substitution PR is surfaced and withheld from auto-merge, without rejecting honest refreshes. Both directions are pinned by test_dependency_skew_withholds_auto_merge.

Verification

29 tests pass (23 existing + 6 new), ruff clean. Confirmed the finding renders in the comment as a 🟡 advisory item alongside a ✅ gate row.

New tests: all ten lockfile/manifest pairs · the paired case · advisory (non-BLOCK) behaviour · per-directory monorepo matching · a non-lockfile control · the auto-merge invariant.

Note

If you'd rather this escalate to NEEDS_HUMAN instead of SAFE-with-no-auto-merge, that's a one-line change in verdict.py — I went with the less intrusive option since the auto-merge bar already withholds authority.

Closes #16.

While implementing this I found the gate it feeds was dead:
`supply.dependency_skew` was referenced in the gate report
(`__init__.py`), the verdict logic (`verdict.py`) and the rendered
comment (`render.py`), but no scanner ever produced it. Every review to
date printed "Dependency skew | ✅ ok" from a check that did not exist.

scan_dependency_skew flags a lockfile that moved without its sibling
manifest across npm/yarn/pnpm/bun, uv/poetry/Pipenv, Cargo, Go, Composer
and Bundler. Matching is per-directory, so packages/a/package-lock.json
is not satisfied by packages/b/package.json.

Advisory, not blocking — and that distinction needed a second fix. The
gate read

    dep_ok = not any(f.id == "supply.dependency_skew" for f in findings)

ignoring `blocking`, unlike secret_scan_clean and
no_forbidden_perm_change which both key off it. Since a failed gate means
verdict=BLOCK / "not mergeable until resolved", emitting this finding
under the old wiring would have hard-blocked every `npm audit fix` and
Dependabot PR. It now keys off a blocking skew, consistent with the other
two gates.

The safety property still holds without blocking: the finding is MEDIUM,
and auto_merge_eligible requires worst.rank <= LOW, so a lockfile-only
change is surfaced AND withheld from auto-merge, while a lockfile moving
together with its manifest stays eligible. Both directions are pinned by
a test.

6 new tests: all ten lockfile/manifest pairs, the paired case, advisory
(non-BLOCK) behaviour, per-directory monorepo matching, a non-lockfile
control, and the auto-merge invariant.
@bkd-dotcom
bkd-dotcom force-pushed the feat/dependency-skew-check branch from 27d4695 to 91117bc Compare August 18, 2026 19:17
@bkd-dotcom
bkd-dotcom merged commit 06c3b17 into main Aug 18, 2026
4 checks passed
@bkd-dotcom
bkd-dotcom deleted the feat/dependency-skew-check branch August 18, 2026 19:18
bkd-dotcom added a commit that referenced this pull request Aug 18, 2026
The dependency-skew check (#16, #19) plus the CI-comment false-positive
fix (#18). Minor bump rather than patch because a new deterministic check
landed, not just fixes.

Also fixes a stale pin: action.yml fell back to @v0.1.1 even after v0.1.2
shipped, so any workflow not setting signetry-reviewer-version silently
installed a release behind.

Co-authored-by: Binay <bkd-dotcom@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Check: flag a lockfile change without a corresponding manifest change

1 participant