Skip to content

docs: add format-specific lockfile parser limitations - #212

Merged
sonukapoor merged 1 commit into
OWASP:mainfrom
mvanhorn:osc/3-format-specific-parser-limitations
Apr 23, 2026
Merged

sonukapoor merged 1 commit into
OWASP:mainfrom
mvanhorn:osc/3-format-specific-parser-limitations

Conversation

@mvanhorn

Copy link
Copy Markdown
Contributor

Summary

Adds a new "Format-specific limitations" section to docs/parser-coverage.md so bug reporters can tell which lockfile-format behaviors are intentional vs bugs.

Closes #3

Why this matters

The existing docs/parser-coverage.md covered general edge cases (monorepos, nested node_modules, optional deps, private registries) and the supported-formats table, but did not document the format-specific behaviors each parser actually has today. Issue #3 asks for that coverage across all four parsers plus the package.json fallback, with the explicit goal of making bug reports easier to submit.

Each bullet is cross-referenced against the current implementation in src/parsers/*.ts - not from memory:

  • src/parsers/package-lock.ts - v1 legacy and v2/v3 flat paths; node_modules/-path filter at line 11; dev flag read directly from the lockfile
  • src/parsers/pnpm-lock.ts - branches on lockfileVersion >= 9; normalizePnpmDepRef strips link: / workspace: (lines 189, 206) and returns null for relative-path refs
  • src/parsers/yarn-lock.ts - no prodOnly parameter (yarn.lock Classic has no dev/prod distinction); path is always ["project", name]; matches the MVP note already in src/parsers/index.ts
  • src/parsers/bun-lock.ts - JSONC trailing-comma strip (line 7); dev inferred from workspace section membership
  • src/parsers/package-json.ts - skipped.slice(0, 50) report cap

Changes

  • docs/parser-coverage.md: added "Format-specific limitations" section between the package.json fallback block and the existing "Known edge cases" block. No existing content was modified.

Testing

Docs-only change. Verified the diff renders correctly and all claims match the parser source.

This contribution was developed with AI assistance (Claude Code).

Closes OWASP#3

The parser-coverage doc previously documented general edge cases
(monorepos, nested node_modules, optional deps, private registries)
but did not spell out the format-specific behaviors each parser has
today. This section pulls those out per format, cross-referenced
against src/parsers/*.ts so bug reports can hit the right target.

- package-lock.json: v1/v2/v3 handling, node_modules path filter,
  dev flag source.
- pnpm-lock.yaml: v5/v6/v9 branch, link: / workspace: stripping,
  approximated path reconstruction.
- yarn.lock: --prod-only not honored (no dev/prod distinction in
  Classic lockfiles), flattened paths.
- bun.lock: JSONC parsing, workspace-based dev inference.
- package.json fallback: 50-entry skipped-deps cap, no transitive
  visibility.
@sonukapoor

Copy link
Copy Markdown
Collaborator

Thanks for the PR - this is a useful addition overall.

I checked the new documentation against the current parser implementation, and I think the pnpm section needs to be toned down before merge.

Right now the docs say:

  • v5, v6, and v9+ are supported
  • v9+ uses the newer snapshots model
  • the parser branches on lockfileVersion

But the current parser does not branch on lockfileVersion, and it only reads packages + importers. I don’t see logic that reads snapshots, so that wording looks stronger than what the implementation currently guarantees.

I think the rest of the additions look broadly aligned with the code. If you update just the pnpm wording to be more implementation-accurate, this should be in good shape.

@mvanhorn

Copy link
Copy Markdown
Contributor Author

Thanks @sonukapoor. Double-checked src/parsers/pnpm-lock.ts on main before softening, and I think the current wording actually matches the code:

// src/parsers/pnpm-lock.ts, lines 8-13 on main
export function loadFromPnpmLock(filePath: string, prodOnly: boolean): PackageRef[] {
  const content = fs.readFileSync(filePath, "utf8");
  const parsed = YAML.parse(content) as any;
  const majorVersion = parseInt(String(parsed?.lockfileVersion ?? "0"), 10);
  return majorVersion >= 9 ? loadV9(parsed, prodOnly) : loadLegacy(parsed, prodOnly);
}

And loadV9 does read from snapshots:

// src/parsers/pnpm-lock.ts, line 93 on main
function loadV9(parsed: any, prodOnly: boolean): PackageRef[] {
  const snapshotsSection = parsed?.snapshots ?? {};
  // ...
}

Both were added in #198 ("feat: add pnpm lockfile v9 support") so they've been in since v1.7.0. If you're looking at a local branch that predates that, my docs would read stronger than the code you're seeing. Happy to tone things down if you'd still prefer the docs be conservative about v9 coverage - just wanted to flag the mismatch before changing the wording.

@sonukapoor

Copy link
Copy Markdown
Collaborator

That makes sense. Thank you for your contribution @mvanhorn

@sonukapoor
sonukapoor merged commit 9e1b0f8 into OWASP:main Apr 23, 2026
4 checks passed
@mvanhorn

Copy link
Copy Markdown
Contributor Author

Thanks for the docs merge, @sonukapoor. Calling out parser limits up front saves debugging.

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.

Document known lockfile edge cases

2 participants