Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

- mission: on-demand JavaScript dependency PR risk summary
- JS package manager scope: support `package.json` with `package-lock.json` or
`pnpm-lock.yaml`, plus `pnpm-workspace.yaml` for pnpm workspace discovery
`pnpm-lock.yaml`, plus `pnpm-workspace.yaml` for pnpm workspace discovery,
and support classic `yarn.lock` fallback for root, workspace, and nested
standalone Yarn targets
- keep GitHub I/O in `internal/github`
- keep orchestration in `internal/app`
- keep deterministic logic in `internal/analysis` and `internal/npm`
Expand Down
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@ All notable changes to `gh-dep-risk` will be documented in this file.

## Unreleased

- Fixed local fallback against large repository lockfiles fetched from the
GitHub contents API by following blob-backed `encoding: none` responses for
files such as large `yarn.lock` and `pnpm-lock.yaml`.
- Marked local npm and Yarn file/workspace packages consistently during local
fallback so local targets are not treated like registry packages for target
traversal or publish-age lookups.
- Tightened the public scope wording to distinguish GitHub dependency-review
ecosystem coverage from the narrower local fallback support matrix.

## v0.1.7

- Expanded the dependency-review path to surface mixed-ecosystem pull requests
Expand Down
3 changes: 2 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,12 @@ go build -o gh-dep-risk.exe .
.\gh-dep-risk.exe version
```

The supported JS package manager scope is:
The supported local JS fallback scope is:

- npm: `package.json`, `package-lock.json`
- pnpm: `package.json`, `pnpm-lock.yaml`
- pnpm workspace discovery: `pnpm-workspace.yaml`
- yarn: `package.json`, `yarn.lock` (Yarn Classic only)

Build with explicit metadata:

Expand Down
7 changes: 5 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ webhook, queue, database, or dashboard infrastructure.

## Scope

- API-first multi-ecosystem dependency review
- on-demand pull request dependency review through GitHub Dependency Review
- local fallback support matrix:
- npm: `package.json`, `package-lock.json`
- pnpm: `package.json`, `pnpm-lock.yaml`
Expand All @@ -23,7 +23,8 @@ webhook, queue, database, or dashboard infrastructure.
- no server, webhook receiver, GitHub App, DB, queue, dashboard, bun, or
broad non-JS local fallback in this release

Dependency-review path ecosystems in this release:
Dependency Review ecosystems surfaced in this release when GitHub provides
them:

- Cargo
- Composer
Expand Down Expand Up @@ -298,6 +299,8 @@ Notes:
- Yarn local fallback supports classic `yarn.lock` installs only
- likely Yarn Berry / Plug'n'Play lockfiles are detected and reported as an
unsupported local-fallback case instead of being analyzed inaccurately
- large lockfiles served by the GitHub contents API without inline content are
still fetched through the corresponding blob object instead of failing early
- if a lockfile-only workspace change cannot be mapped exactly, the report calls
out that attribution is approximate instead of failing
- if both `package-lock.json` and `pnpm-lock.yaml` exist for the same target
Expand Down
10 changes: 8 additions & 2 deletions RELEASING.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@ git status --short --branch

The tree should be clean before cutting a release tag.

## 2. Run tests
## 2. Run local verification

```bash
go test ./...
go build ./...
git diff --check
```

## 3. Build a release-quality binary locally
Expand All @@ -40,7 +42,11 @@ go build -ldflags "-s -w -X gh-dep-risk/cmd.version=$version -X gh-dep-risk/cmd.
.\gh-dep-risk.exe version --json
```

## 4. Optional manual workflow smoke test
## 4. Run live CLI checks and optional manual workflow smoke test

Before tagging, run the local binary against real pull requests if you have
GitHub auth available. Prefer at least one smaller PR and one larger PR so the
final release is not validated only against fixtures.

If you have GitHub auth and repository access available, run:

Expand Down
4 changes: 4 additions & 0 deletions docs/smoke-test.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ Verify:

- the command exits `0`, `2`, `3`, or `4` as documented
- the report includes repo, PR, score, blast radius, and recommended actions
- before release, run at least one smaller PR and one larger PR instead of
relying only on fixture-backed tests
- if the target repository has a very large lockfile, verify the run does not
fail on a GitHub contents API `encoding: none` response

## 2. Workflow dispatch run

Expand Down
48 changes: 48 additions & 0 deletions internal/analysis/types_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package analysis

import (
"testing"

"gh-dep-risk/internal/npm"
)

func TestCollectRegistryTargetsSkipsLocalWorkspacePackages(t *testing.T) {
input := Input{
Target: AnalysisTarget{
DisplayName: "apps/web",
ManifestPath: "apps/web/package.json",
LockfilePath: "yarn.lock",
Kind: TargetKindWorkspace,
PackageManager: "yarn",
OwningDirectory: "apps/web",
},
BaseManifest: manifest(nil, nil, nil),
HeadManifest: manifest(map[string]string{
"local-lib": "file:../local-lib",
}, nil, nil),
BaseLockfile: &npm.Lockfile{
Manager: "yarn",
Packages: map[string]npm.LockPackage{},
Importers: map[string]npm.LockImporter{},
},
HeadLockfile: &npm.Lockfile{
Manager: "yarn",
Packages: map[string]npm.LockPackage{
"node_modules/local-lib": {
Path: "node_modules/local-lib",
Name: "local-lib",
Version: "1.0.0",
Resolved: "file:../local-lib",
WorkspaceLocal: true,
Dependencies: map[string]string{},
},
},
Importers: map[string]npm.LockImporter{},
},
}

targets := CollectRegistryTargets(input)
if len(targets) != 0 {
t.Fatalf("expected no registry targets for local workspace package, got %#v", targets)
}
}
55 changes: 47 additions & 8 deletions internal/github/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@ func (c *APIClient) GetRepositoryFile(ctx context.Context, repo Repo, path, ref
var resp struct {
Content string `json:"content"`
Encoding string `json:"encoding"`
SHA string `json:"sha"`
}

contentPath := fmt.Sprintf("repos/%s/%s/contents/%s?ref=%s", repo.Owner, repo.Name, escapeContentPath(path), url.QueryEscape(ref))
Expand All @@ -364,14 +365,20 @@ func (c *APIClient) GetRepositoryFile(ctx context.Context, repo Repo, path, ref
}
return nil, classifyAuthError(err)
}
if resp.Encoding != "base64" {
return nil, fmt.Errorf("unsupported content encoding %q for %s", resp.Encoding, path)
}
decoded, err := base64.StdEncoding.DecodeString(strings.ReplaceAll(resp.Content, "\n", ""))
if err != nil {
return nil, fmt.Errorf("decode %s: %w", path, err)
}
return decoded, nil
return decodeRepositoryContent(path, resp.Content, resp.Encoding, resp.SHA, func(sha string) (string, string, error) {
var blob struct {
Content string `json:"content"`
Encoding string `json:"encoding"`
}
blobPath := fmt.Sprintf("repos/%s/%s/git/blobs/%s", repo.Owner, repo.Name, url.PathEscape(sha))
if err := client.DoWithContext(ctx, "GET", blobPath, nil, &blob); err != nil {
if IsHTTPStatus(err, 404) {
return "", "", ErrNotFound
}
return "", "", classifyAuthError(err)
}
return blob.Content, blob.Encoding, nil
})
}

func (c *APIClient) ListIssueComments(ctx context.Context, repo Repo, issueNumber int) ([]IssueComment, error) {
Expand Down Expand Up @@ -596,6 +603,38 @@ func escapeContentPath(path string) string {
return strings.Join(parts, "/")
}

func decodeRepositoryContent(path, content, encoding, sha string, fetchBlob func(sha string) (string, string, error)) ([]byte, error) {
switch encoding {
case "base64":
return decodeBase64RepositoryContent(path, content)
case "none":
if strings.TrimSpace(sha) == "" {
return nil, fmt.Errorf("unsupported content encoding %q for %s", encoding, path)
}
if fetchBlob == nil {
return nil, fmt.Errorf("unsupported content encoding %q for %s", encoding, path)
}
blobContent, blobEncoding, err := fetchBlob(sha)
if err != nil {
return nil, err
}
if blobEncoding != "base64" {
return nil, fmt.Errorf("unsupported blob encoding %q for %s", blobEncoding, path)
}
return decodeBase64RepositoryContent(path, blobContent)
default:
return nil, fmt.Errorf("unsupported content encoding %q for %s", encoding, path)
}
}

func decodeBase64RepositoryContent(path, content string) ([]byte, error) {
decoded, err := base64.StdEncoding.DecodeString(strings.ReplaceAll(content, "\n", ""))
if err != nil {
return nil, fmt.Errorf("decode %s: %w", path, err)
}
return decoded, nil
}

type AuthError struct {
Op string
Err error
Expand Down
27 changes: 27 additions & 0 deletions internal/github/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package github
import (
"bytes"
"context"
"encoding/base64"
"errors"
"reflect"
"strings"
Expand Down Expand Up @@ -189,6 +190,32 @@ func TestClassifyAuthErrorWrapsHTTP401AsAuthError(t *testing.T) {
}
}

func TestDecodeRepositoryContentSupportsLargeBlobFallback(t *testing.T) {
want := []byte("hello from blob fallback")
content, err := decodeRepositoryContent("yarn.lock", "", "none", "blob-sha", func(sha string) (string, string, error) {
if sha != "blob-sha" {
t.Fatalf("unexpected blob sha %q", sha)
}
return base64.StdEncoding.EncodeToString(want), "base64", nil
})
if err != nil {
t.Fatal(err)
}
if !bytes.Equal(content, want) {
t.Fatalf("unexpected decoded content: got %q want %q", string(content), string(want))
}
}

func TestDecodeRepositoryContentRejectsUnsupportedEncodingWithoutBlobSHA(t *testing.T) {
_, err := decodeRepositoryContent("pnpm-lock.yaml", "", "none", "", nil)
if err == nil {
t.Fatal("expected error")
}
if !strings.Contains(err.Error(), `unsupported content encoding "none"`) {
t.Fatalf("unexpected error: %v", err)
}
}

func strconvFormatBool(value bool) string {
if value {
return "true"
Expand Down
6 changes: 6 additions & 0 deletions internal/npm/jsgraph.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ func (l *Lockfile) CollectTargetPackages(targetDir string, directNames []string)
continue
}
result.All[pkg.Path] = pkg
if pkg.WorkspaceLocal {
continue
}
Comment on lines 150 to +153
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The BFS now correctly avoids enqueueing WorkspaceLocal packages, but the later nested-package sweep (that adds any package whose path is under an already-included package) will still pull in .../node_modules/... dependencies under a workspace-local package path. That can re-introduce external packages as transitive even though traversal was intentionally skipped. Consider teaching that sweep to ignore workspace-local parents (e.g., only add nested packages when the parent result.All[existingPath] is not WorkspaceLocal).

Copilot uses AI. Check for mistakes.
Comment on lines +151 to +153
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Prevent descendant sweep from re-adding local package deps

Skipping queue traversal for WorkspaceLocal packages here is incomplete because the package path is still inserted into result.All, and the later descendant sweep (strings.HasPrefix(pkg.Path, existingPath+"/node_modules/")) will re-include nested dependencies under that local package. In repositories where a workspace/file dependency has non-hoisted children (for example node_modules/local-lib/node_modules/*), those external packages are still counted as target transitives, so risk score and publish-age lookup behavior now depends on hoisting layout instead of consistently excluding local-package trees.

Useful? React with 👍 / 👎.

queue = append(queue, pkg)
}

Expand All @@ -166,6 +169,9 @@ func (l *Lockfile) CollectTargetPackages(targetDir string, directNames []string)
continue
}
result.All[dep.Path] = dep
if dep.WorkspaceLocal {
continue
}
queue = append(queue, dep)
}
}
Expand Down
22 changes: 22 additions & 0 deletions internal/npm/lockfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ func parseNPMLockfile(data []byte) (*Lockfile, error) {
OS: append([]string(nil), entry.OS...),
CPU: append([]string(nil), entry.CPU...),
Dependencies: cloneMap(entry.Dependencies),
WorkspaceLocal: isLocalPackageReference(entry.Resolved),
}
}
return lockfile, nil
Expand Down Expand Up @@ -290,6 +291,7 @@ func parseYarnLockfile(data []byte) (*Lockfile, error) {
if entry.Version == "" {
continue
}
entry.WorkspaceLocal = isLocalPackageReference(entry.Resolved)
key := entry.Name + "@" + entry.Version
if existingPath, ok := seen[key]; ok {
existing := lockfile.Packages[existingPath]
Expand All @@ -302,6 +304,7 @@ func parseYarnLockfile(data []byte) (*Lockfile, error) {
if existing.Integrity == "" {
existing.Integrity = entry.Integrity
}
existing.WorkspaceLocal = existing.WorkspaceLocal || entry.WorkspaceLocal
lockfile.Packages[existingPath] = existing
continue
}
Expand Down Expand Up @@ -350,6 +353,7 @@ func flattenLegacyDependencies(target map[string]LockPackage, parent string, dep
OS: append([]string(nil), dep.OS...),
CPU: append([]string(nil), dep.CPU...),
Dependencies: cloneMap(dep.Requires),
WorkspaceLocal: isLocalPackageReference(dep.Resolved),
}
flattenLegacyDependencies(target, path, dep.Dependencies)
}
Expand Down Expand Up @@ -678,6 +682,24 @@ func isPNPMWorkspaceLink(version string) bool {
return strings.HasPrefix(lower, "link:") || strings.HasPrefix(lower, "workspace:")
}

func isLocalPackageReference(value string) bool {
lower := strings.ToLower(strings.TrimSpace(value))
switch {
case strings.HasPrefix(lower, "file:"):
return true
case strings.HasPrefix(lower, "link:"):
return true
case strings.HasPrefix(lower, "workspace:"):
return true
case strings.HasPrefix(lower, "portal:"):
return true
case lower == "workspace-local":
return true
default:
return false
}
}

func pnpmResolved(tarball, repo, resolutionType string) string {
switch {
case strings.TrimSpace(tarball) != "":
Expand Down
Loading