Skip to content

feat(gate): add a lockfile-tamper-risk check - #2676

Merged
JSONbored merged 1 commit into
mainfrom
feat/lockfile-tamper-risk-gate-check
Jul 3, 2026
Merged

feat(gate): add a lockfile-tamper-risk check#2676
JSONbored merged 1 commit into
mainfrom
feat/lockfile-tamper-risk-gate-check

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Adds a deterministic lockfile_tamper_risk finding (src/review/lockfile-tamper.ts) that flags a
    changed package-lock.json diff hunk where a resolved/integrity value changed WITHOUT the
    corresponding package.json dependency version changing, or where a resolved URL points outside
    registry.npmjs.org — the classic supply-chain hand-edit tell. Matches any package-lock.json path
    depth (root, review-enrichment/, apps/gittensory-ui/, or a future workspace) rather than a
    hardcoded path list.
  • Distinct from the existing dependency-diff/OSV.dev CVE analyzer
    (review-enrichment/src/analyzers/lockfile-drift.ts) — that flags KNOWN-CVE versions in changed
    deps; this flags lockfile-tamper/resolved-URL integrity, a different threat model, regardless of
    whether the substituted version has a published CVE.
  • Wires a new gate.lockfileIntegrity: off|advisory|block gate dimension through the full
    config-as-code chain, following the exact same pattern as the existing sizeGateMode /
    premergeContentRecheck fields (src/types.ts, src/signals/focus-manifest.ts,
    src/rules/advisory.ts's isConfiguredGateBlocker, .gittensory.yml.example, and
    src/openapi/schemas.ts's RepositorySettingsSchema): config-as-code only (no DB column or
    dashboard toggle), off by default, so a repo that has not opted in sees zero behavior change.
  • Wires the scan into the live review path in src/queue/processors.ts (maybeAddLockfileTamperFinding,
    mirroring maybeAddSecretLeakFinding's structure) — the scan is skipped entirely unless
    lockfileIntegrityGateMode is not off, and any load error is fail-safe swallowed so it can never
    destabilize the gate.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked an issue, or this is small enough that the summary explains why an issue is not needed.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally; codecov/patch requires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate. — src/review/lockfile-tamper.ts is 100% lines/branches/functions (91/91 lines, 85/85 branches per lcov); the touched lines in src/rules/advisory.ts, src/signals/focus-manifest.ts, and src/queue/processors.ts are also fully covered (verified directly against coverage/lcov.info).
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate — 0 vulnerabilities
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries — test/unit/lockfile-tamper.test.ts (19 tests: legitimate bump does not trigger, hand-edited resolved/integrity with no version bump triggers, off-registry resolved URL triggers, scoped packages, multi-package title capping, etc.), test/unit/lockfile-tamper-wiring.test.ts (6 tests: off/unset/advisory/block mode gating, files reuse vs. lazy-load, fail-safe error swallowing), plus new gate.lockfileIntegrity coverage in test/unit/focus-manifest.test.ts (parse/round-trip/resolve/warn) and test/unit/gate-check-policy.test.ts (blocker escalation under block, advisory-only under advisory/off).

Additionally ran (not in the template list but part of the full local gate): npm run db:migrations:check (100 migrations OK, unaffected — no migration in this PR), npm run selfhost:env-reference:check, npm run selfhost:validate-observability, npm run cf-typegen:check, npm run ui:openapi:settings-parity, npm run ui:version-audit, npm run ui:test, npm run build:miner, npm run rees:test — all green.

If any required check was skipped, explain why:

  • None skipped. Two unrelated, pre-existing failures were investigated and confirmed present on a clean
    main checkout with zero changes (reproduced by git stash before re-running):
    test/unit/check-migrations-script.test.ts (a worktree/environment-specific subprocess-spawn issue,
    unrelated to migrations content — the real db:migrations:check passes cleanly) and
    test/unit/github-graphql-cache.test.ts's single-flight timing test (passes in isolation, flakes only
    under full-suite parallel load). Neither touches any file this PR changes.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. — N/A, no auth/cookie/CORS/session changes.
  • API/OpenAPI/MCP behavior is updated and tested where needed. — RepositorySettingsSchema gained lockfileIntegrityGateMode; npm run ui:openapi regenerated and committed; ui:openapi:settings-parity passes.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. — N/A, no UI changes.
  • Visible UI changes include a UI Evidence section below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. — N/A, no visible UI changes (backend gate feature only).
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. — .gittensory.yml.example documents the new gate.lockfileIntegrity field; CHANGELOG.md untouched.

UI Evidence

N/A — this PR has no visible UI change (a new backend gate dimension + deterministic finding only).

Notes

  • Left the metadata-only MCP gate predictor (src/rules/predicted-gate.ts) out of scope: it never
    receives PR diff content (no source upload), and the existing secret_leak finding — the closest
    analogue, also a diff-content-based deterministic finding — is likewise absent from that predictor for
    the same reason. Matching that established precedent.
  • Left the dashboard/DB-backed settings-write route (maintainerSettingsSchema in src/api/routes.ts)
    and the settings-preview surface (src/signals/settings-preview.ts) out of scope: this field follows
    the sizeGateMode / premergeContentRecheck precedent of being config-as-code only (no DB column, no
    dashboard toggle) — those two existing fields are likewise absent from both surfaces.
  • Only the npm package-lock.json format is parsed today (matching the issue's named paths); other
    lockfile formats (yarn.lock, pnpm-lock.yaml, etc.) are not in scope for this check.

Closes #2563

Contributor supply-chain attacks can hand-edit a package-lock.json entry's
resolved/integrity value (or point it at a non-registry host) without
touching the corresponding package.json version, which the existing
dependency-diff/OSV.dev analyzer never sees since it only looks for
KNOWN-CVE versions, not resolved-URL/integrity tampering.

Adds a deterministic lockfile_tamper_risk finding (src/review/lockfile-tamper.ts)
wired through the same config-as-code chain as sizeGateMode/premergeContentRecheck
(gate.lockfileIntegrity: off|advisory|block, off by default, config-as-code
only — no DB column or dashboard toggle, matching sizeGateMode's footprint)
so a repo that has not opted in sees zero behavior change.

Closes #2563
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 8d2b409 Commit Preview URL

Branch Preview URL
Jul 03 2026, 04:09 AM

@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 96.11%. Comparing base (642b12d) to head (8d2b409).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/queue/processors.ts 83.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2676      +/-   ##
==========================================
+ Coverage   96.10%   96.11%   +0.01%     
==========================================
  Files         237      238       +1     
  Lines       26538    26638     +100     
  Branches     9624     9663      +39     
==========================================
+ Hits        25505    25604      +99     
  Misses        424      424              
- Partials      609      610       +1     
Files with missing lines Coverage Δ
src/openapi/schemas.ts 100.00% <ø> (ø)
src/review/lockfile-tamper.ts 100.00% <100.00%> (ø)
src/rules/advisory.ts 97.65% <100.00%> (+<0.01%) ⬆️
src/signals/focus-manifest.ts 99.29% <100.00%> (+<0.01%) ⬆️
src/queue/processors.ts 92.55% <83.33%> (-0.03%) ⬇️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored JSONbored self-assigned this Jul 3, 2026
@JSONbored
JSONbored merged commit 00d1490 into main Jul 3, 2026
13 checks passed
@JSONbored
JSONbored deleted the feat/lockfile-tamper-risk-gate-check branch July 3, 2026 05:16
JSONbored added a commit that referenced this pull request Jul 3, 2026
…links

Fixes two confirmed false-positive bugs in the already-merged #2676
(issue #2563): packagesWithManifestVersionChange only saw package.json's
DIRECT dependency blocks, so a transitive dependency's resolved/
integrity bump -- the vast majority of any real lockfile diff -- was
always misclassified as tamper risk. Replaced it with a signal
comparing a lockfile entry's resolved/integrity change against that
SAME entry's own "version" line within the same diff, which a genuine
install/update always bumps together for both direct and transitive
entries.

Also exempts non-http(s) resolved values (npm workspace-local relative
paths, e.g. "packages/gittensory-mcp") from the off-registry check --
they were never resolved from a registry, so they can't be off-registry.

Addresses gate-review findings on #2676.
JSONbored added a commit that referenced this pull request Jul 3, 2026
…links (#2692)

* fix(review): stop false-positiving on transitive bumps and workspace links

Fixes two confirmed false-positive bugs in the already-merged #2676
(issue #2563): packagesWithManifestVersionChange only saw package.json's
DIRECT dependency blocks, so a transitive dependency's resolved/
integrity bump -- the vast majority of any real lockfile diff -- was
always misclassified as tamper risk. Replaced it with a signal
comparing a lockfile entry's resolved/integrity change against that
SAME entry's own "version" line within the same diff, which a genuine
install/update always bumps together for both direct and transitive
entries.

Also exempts non-http(s) resolved values (npm workspace-local relative
paths, e.g. "packages/gittensory-mcp") from the off-registry check --
they were never resolved from a registry, so they can't be off-registry.

Addresses gate-review findings on #2676.

* fix(review): drop dead both-undefined guard in lockfile versionChanged

removed !== added already evaluates to false when both are undefined,
so the guard was unreachable dead code (codecov/patch flagged it).

* fix(review): key lockfile-tamper candidates by full entry path, not bare name

Two distinct package-lock.json entries can share the same bare package
name at different node_modules nesting depths (npm nests a second copy
when dependents need incompatible versions). Keying candidates by bare
name merged those entries into one shared record, so a legitimate
version bump on one entry could mask an unbumped, tampered
resolved/integrity edit on the other. The full node_modules/... path is
unique per entry and is now used as the map key instead.

Addresses gate-review findings on #2692.
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.

feat(gate): add a lockfile-tamper-risk check

1 participant