Skip to content

fix: render lockfile poisoning findings for non-http(s) resolved URLs#746

Merged
abhisek merged 2 commits into
mainfrom
fix/lfp-empty-non-http-urls
Jun 24, 2026
Merged

fix: render lockfile poisoning findings for non-http(s) resolved URLs#746
abhisek merged 2 commits into
mainfrom
fix/lfp-empty-non-http-urls

Conversation

@KunalSin9h

Copy link
Copy Markdown
Member

Problem

Users on v1.17.3 reported a ** Lockfile Poisoning Detected section with no findings under it — the header prints but the body is empty.

Root cause

The console (summary.go) and markdown (markdown_summary.go) reporters aggregate lockfile poisoning findings by re-parsing the analyzer's free-text messages with regex (aggregateLFPMessages). The URL-extraction regex only matched http(s):

var lfpURLRe = regexp.MustCompile("`(https?://[^`]+)`")

The npm analyzer legitimately flags packages resolving to any untrusted scheme — including git+ssh://, git+https://, git://, ssh:// (common for git dependencies). Those messages failed the regex and were silently dropped during aggregation (the continue in the loop).

The section header gates on the raw event count (len(r.lockfilePoisoning) > 0), while the body iterates over the aggregated groups. So a lockfile whose poisoning findings were all on non-http(s) URLs rendered the header with an empty body.

This was introduced in v1.17.3 by add61ab ("consolidate lockfile poisoning output by URL"), which added the reporter-side string re-parsing. Before that, the reporter printed each raw message directly, so all schemes always showed.

Fix

Relax lfpURLRe to match any URL scheme ([a-zA-Z][a-zA-Z0-9+.-]*://). The package name is the first backtick group and never contains ://, so the URL group is still matched unambiguously.

Testing

Added pkg/reporter/lfp_aggregation_test.go with regression coverage for git+ssh, git+https, git, and ssh schemes, plus existing http(s) and path-convention cases.

Verified end-to-end on a lockfile mixing all schemes:

Before: only http:// and https:// packages shown; the three git-based deps dropped.
After: all five untrusted URLs render; trusted registry.npmjs.org correctly not flagged.

🤖 Generated with Claude Code

The console and markdown reporters re-parse the analyzer's free-text
lockfile poisoning messages via regex to aggregate findings by URL. The
URL-extraction regex (lfpURLRe) only matched http(s) URLs, so any finding
whose package resolved to a git+ssh, git+https, git, or ssh URL was
silently dropped during aggregation.

Because the "Lockfile Poisoning Detected" header gates on the raw
(unaggregated) event count while the body iterates over the aggregated
groups, a lockfile whose poisoning findings were all on non-http(s) URLs
(common for git dependencies) rendered the header with an empty body.

Relax lfpURLRe to match any URL scheme. The package name is the first
backtick group and never contains "://", so the URL group is still
matched unambiguously.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@safedep

safedep Bot commented Jun 24, 2026

Copy link
Copy Markdown

SafeDep Report Summary

Green Malicious Packages Badge Green Vulnerable Packages Badge Green Risky License Badge

No dependency changes detected. Nothing to scan.

View complete scan results →

This report is generated by SafeDep Github App

@KunalSin9h KunalSin9h requested a review from abhisek June 24, 2026 06:05
@KunalSin9h KunalSin9h self-assigned this Jun 24, 2026
@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 12.46%. Comparing base (c9a6321) to head (6a65b72).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #746      +/-   ##
==========================================
+ Coverage   12.41%   12.46%   +0.04%     
==========================================
  Files         372      372              
  Lines       49231    49231              
==========================================
+ Hits         6111     6135      +24     
+ Misses      42634    42609      -25     
- Partials      486      487       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@abhisek abhisek merged commit 23b07a0 into main Jun 24, 2026
16 checks passed
@abhisek abhisek deleted the fix/lfp-empty-non-http-urls branch June 24, 2026 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants