dependency-review

Files (8)

• SKILL.md
• CI-AND-CONTAINER-REVIEW.md · references/
• DETECTION-PLAYBOOK.md · references/
• IS-IT-NEEDED.md · references/
• PACKAGE-IDENTITY.md · references/
• PROVENANCE-AND-LICENSING.md · references/
• QUALITY-FILTER.md · references/
• REVIEW-VOICE.md · references/

SKILL.md (first lines)

# Dependency & Supply-Chain Review

The lens for PRs where the risk is not in the diff's own code but in the code the diff
invites in. Layered on top of [`pr-review`](https://github.com/priyank766/OpenSource-SKILL/blob/main/skills/pr-review/SKILL.md).

**Read first:** [`DETECTION-PLAYBOOK.md`](references/DETECTION-PLAYBOOK.md) ·
[`QUALITY-FILTER.md`](references/QUALITY-FILTER.md) ·
[`REVIEW-VOICE.md`](references/REVIEW-VOICE.md)

> **Where the time goes.** Finding the flaw is the review. The playbook above is the
> general flaw-hunting procedure; this file is the domain-specific lens on top of it.
> Scoring and writing the comment are the last ten minutes, not the work.

**Severity floor: 7.5** — lower than the suite default of 8.0. A malicious or compromised
dependency executes with full build and runtime privilege on every contributor's laptop
and inside CI, where the tokens live. A false positive costs one thread of discussion. A
miss costs the project's release signing key. That asymmetry justifies the lower bar; it
does not justify posting health-score trivia.

---

## The governing rule

> Adding a dependency adds every one of that package's maintainers to your project's trust
> boundary — and every maintainer of every one of its transitive dependencies too.

So the review question is never "is this library good". It is:

**What does merging this let someone else execute, and where?**

"Where" has three answers, in descending order of severity: in CI with repository
credentials, on a contributor's machine at install time, and in production at runtime.
Score against the worst reachable one.

---

## Why this exists

Dependency PRs are the ones reviewers rubber-stamp. The manifest diff is one line, the
bot's changelog link looks fine, CI is green — and green CI is exactly what an
install-time payload produces. Meanwhile the actual review surface, the lockfile, is
collapsed by default in the GitHub UI as a generated file.

The default AI pass on these PRs is worse than useless: it summarises the changelog and
notes that the package is popular. That is a 5/10 comment on a diff with a 10/10 finding
three hundred lines down.

### What each role gets out of it

| Role | Time this gives back |
| :--- | :--- |
| **Maintainer** | The lockfile read you keep postponing, done as a checklist. You get "12 new transitive packages, one with a postinstall, here it is" instead of a 3,000-line generated diff you were going to scroll past. |
| **Contributor** | A pre-flight on your own dependency addition — wrong section, duplicate major version, copyleft license — so the first maintainer reply is not a request to remove it. |
| **Project owner** | A repeatable bar for what enters the trust boundary, and a blast-radius argument you can point at when justifying pinning, `--ignore-scripts`, or SHA-pinned actions. |

---

## Procedure

```mermaid
flowchart TD
    A[Dependency or CI change detected] --> B[1. Diff the lockfile, not the manifest]
    B --> C[1b. Enumerate newly added packages, direct and transitive]
    C --> D[2. Run ecosystem vulnerability scanners]
    D --> E[3. Enumerate install-time scripts across the new tree]
    E --> F[4. Check package identity]
    F --> G[5. Check provenance, health, and license]
    G --> H[6. Review CI workflow and image changes]
    H --> I[7. Ask whether the dependency is needed at all]
    I --> J[Score against the 10-point filter]
    J -->|"score below 7.5"| K[Discard or batch into an issue]
    J -->|"score 7.5 or above"| L[Report with evidence]
    J -->|"suspected active compromise"| M[Do not comment publicly. Contact maintainers privately]
```

---

## 1. Read the lockfile diff, not the manifest

A one-line manifest change can pull forty transitive packages. The manifest states intent;
the lockfile states what actually gets installed. Only one of those is the trust boundary.

```bash
git diff base...head -- \
  package.json package-lock.json yarn.lock pnpm-lock.yaml npm-shrinkwrap.json \
  pyproject.toml uv.lock poetry.lock Pipfile.lock requirements*.txt \
  go.mod go.sum Cargo.toml Cargo.lock Gemfile Gemfile.lock composer.json composer.lock

git diff --stat base...head -- '*lock*' '*.sum'
```

Count what actually arrived. A "bump lodash" PR that adds 40 package entries is not a bump.

```bash
# npm / pnpm / yarn — new package entries in the lockfile
git diff base...head -- package-lock.json | grep -E '^\+\s+"node_modules/' | sort -u | wc -l
git diff base...head -- pnpm-lock.yaml    | grep -E '^\+\s{2}/' | sort -u

# Go — every added module line
git diff base...head -- go.sum | grep '^+' | awk '{print $1, $2}' | sort -u

# Cargo / Poetry / uv — added package stanzas
git diff base...head -- Cargo.lock  | grep -A2 '^\+name = '
git diff base...head -- poetry.lock uv.lock | grep -E '^\+name = '

# Python requirements
git diff base...head -- 'requirements*.txt' | grep -E '^\+[^-+]'
```

Then diff the resolved sets rather than the text, which removes reordering noise:

```bash
git show base:package-lock.json | jq -r '.packages | keys[]' | sort > /tmp/before.txt
jq -r '.packages | keys[]' package-lock.json | sort > /tmp/after.txt
comm -13 /tmp/before.txt /tmp/after.txt          # packages added by this PR
```

Three lockfile shapes deserve their own look, regardless of scanner output:

From priyank766/OpenSource-SKILL, Apache-2.0.

More from priyank766/OpenSource-SKILL