diff --git a/.changeset/findings-dependency-chains.md b/.changeset/findings-dependency-chains.md new file mode 100644 index 0000000..212385f --- /dev/null +++ b/.changeset/findings-dependency-chains.md @@ -0,0 +1,5 @@ +--- +"@codacy/codacy-cloud-cli": minor +--- + +`codacy findings` and `codacy finding` now show the vulnerable dependency's import chain for SCA findings that carry the new `dependencyChains` field. Each finding is labelled **Direct** (`Update to `) or **Transitive** (` → … → (Fixed in )`), and chains with 4+ packages collapse their middle to ` → ... N more ... → `. The list shows the first chain plus `... and X more`; the detail lists every chain aligned under a single label. `dependencyChains` is also included in `--output json`. diff --git a/.claude/commands/ship-it.md b/.claude/commands/ship-it.md index ccbb0b2..99f406e 100644 --- a/.claude/commands/ship-it.md +++ b/.claude/commands/ship-it.md @@ -1,16 +1,25 @@ --- -description: Changeset + branch + commit + push + PR for the current working tree +description: Changeset + branch + commit + push + PR, then wait for AI reviews and auto-run /pr-fixup --- # Ship it Take the current uncommitted changes on `main` (or on a branch already derived -from `main` for this task) and turn them into an open PR. End-to-end: make -sure there's a changeset, cut a branch, commit, push, open the PR. This is a -user-triggered action — invoking this command IS the explicit authorisation -required by the repo's "never commit, push, or open PRs without asking" rule, -so you can proceed without further confirmation once you've sanity-checked -what's about to be shipped. +from `main` for this task) and turn them into an open PR, **then wait for the +AI reviewers and automatically run `/pr-fixup` on their feedback**. End-to-end: +make sure there's a changeset, cut a branch, commit, push, open the PR (Phases +0–5); then poll for the real AI reviews and chain into `/pr-fixup` (Phases 6–7); +finally report (Phase 8). This is a user-triggered action — invoking this +command IS the explicit authorisation required by the repo's "never commit, +push, or open PRs without asking" rule, so you can proceed without further +confirmation once you've sanity-checked what's about to be shipped. + +**The wait-for-reviews + auto-fixup stage runs by default.** It commits, pushes, +and posts reply comments on the PR without a separate prompt — that is the +intended behavior. Pass `--no-fixup` to skip it and get the classic +"open the PR and stop" flow. Note that the auto-fixup step depends on the +personal `/pr-fixup` command being installed; if it isn't, ship-it still opens +the PR and waits for reviews but skips fixup with a note (see Phase 7). **Arguments:** `$ARGUMENTS` @@ -21,6 +30,9 @@ Optional, space-separated, in any order: - A bump type: `patch`, `minor`, or `major`. If absent, infer — see Phase 1. - A quoted PR title (wrap in double quotes if it contains spaces). If absent, derive from the commit/changeset — see Phase 5. +- `--no-fixup` — skip the post-open "wait for AI reviews + auto-run `/pr-fixup`" + stage (Phases 6–7) and behave like classic ship-it: open the PR and stop. By + default (no flag) ship-it DOES wait for the real reviews and auto-fixup. --- @@ -164,7 +176,14 @@ CI on `main` fails any PR without a changeset, so this step is mandatory. ## Phase 5: Push and open the PR -1. Push with upstream tracking: +1. Capture the moment just before pushing — the review poller in Phase 6 uses + it to ignore any stale reviews from earlier pushes (matters on re-runs): + + ```bash + START="$(date -u +%Y-%m-%dT%H:%M:%SZ)" # remember this value for Phase 6 + ``` + + Then push with upstream tracking: ```bash git push -u origin @@ -204,16 +223,136 @@ CI on `main` fails any PR without a changeset, so this step is mandatory. - Use the argument if provided; otherwise derive from the changeset title or the commit subject. -4. Capture the PR URL from `gh pr create`'s stdout and print it in the - end-of-turn summary. +4. Capture the PR **URL and number** from `gh pr create`'s stdout (or + `gh pr view --json number,url`). You need the number for Phase 6 and the URL + for the final report. + +--- + +## Phase 6: Wait for the AI reviews + +**If `--no-fixup` was passed, skip this phase and Phase 7 — go straight to +Phase 8** (classic "open the PR and stop" behavior). + +This repo has three AI reviewers wired up. Each one posts an **immediate +summary/help comment that is NOT the review**, then its real review a few +minutes later. You must wait for the _real_ review, not the placeholder: + +| Reviewer | Bot login | Immediate comment (ignore) | Real review (wait for) | +|----------|-----------|----------------------------|------------------------| +| Gemini Code Assist | `gemini-code-assist[bot]` | issue comment: "## Summary of Changes … I'll post my feedback shortly" | review: "## Code Review …" | +| Codacy | `codacy-production[bot]` | issue comment: "## Up to standards …" | review: "### Pull Request Overview …" | +| GitHub Copilot | `copilot-pull-request-reviewer[bot]` | (none) | review: "## Pull request overview …" | + +**The reliable signal:** a reviewer's real review is a submitted entry in the +Pull-Request *reviews* API (`pulls/{n}/reviews`). The immediate summary/help +comments only ever land as *issue* comments (`issues/{n}/comments`) — they never +appear in the reviews API. So "all reviews are in" = every expected bot login +appears in `pulls/{n}/reviews` with a `submitted_at` at/after the push from +Phase 5. (Historically all three land within ~6 minutes of opening.) + +Launch a background poller and **do not block the foreground** — the harness +re-invokes you when it exits (one completion notification). Substitute the PR +number from Phase 5 and the `START` timestamp you captured before pushing, then +run this with `run_in_background: true`: + +```bash +OWNER="codacy"; REPO="codacy-cloud-cli" +PR="__PR_NUMBER__" # from Phase 5 +START="__START_ISO8601_UTC__" # from Phase 5, e.g. 2026-06-24T12:30:00Z +MAX_WAIT=900 # 15-minute hard cap +POLL=90 # seconds between polls (never below ~30s — GitHub rate limits) +# AI reviewers configured on this repo. Their *real* reviews land in the reviews +# API; their immediate "summary/help" comments do not. Edit this list if the +# repo's reviewer set changes. +EXPECTED=("gemini-code-assist[bot]" "copilot-pull-request-reviewer[bot]" "codacy-production[bot]") + +deadline=$(( $(date +%s) + MAX_WAIT )) +while :; do + # Distinct bot logins that have SUBMITTED a review at/after the push. + arrived="$(gh api "repos/$OWNER/$REPO/pulls/$PR/reviews" --paginate \ + --jq '.[] | select(.submitted_at != null and .submitted_at >= "'"$START"'") | .user.login' \ + 2>/dev/null | sort -u)" + missing=() + for bot in "${EXPECTED[@]}"; do + grep -qxF "$bot" <<<"$arrived" || missing+=("$bot") + done + if [ "${#missing[@]}" -eq 0 ]; then + echo "READY arrived=[$(paste -sd, - <<<"$arrived")]" + exit 0 + fi + if [ "$(date +%s)" -ge "$deadline" ]; then + echo "TIMEOUT after ${MAX_WAIT}s arrived=[$(paste -sd, - <<<"$arrived")] missing=[$(IFS=,; echo "${missing[*]}")]" + exit 0 + fi + sleep "$POLL" +done +``` + +When the poller exits you are re-invoked with its final stdout line. Read it: + +- `READY arrived=[…]` → all three real reviews are in. Proceed to Phase 7. +- `TIMEOUT … missing=[…]` → not everyone posted within 15 min. **Proceed to + Phase 7 anyway** against the reviews that did arrive, and carry the `missing` + list into the Phase 8 report so the user knows to re-run later. + +Why a background poller and not a Haiku subagent: the "real review vs. summary +comment" distinction is fully deterministic (presence in the reviews API), so no +model judgment is needed during the wait — a background shell loop costs zero +tokens and the harness wakes you the instant it finishes. A transient `gh api` +failure just yields an empty poll; the loop retries on the next tick. + +--- + +## Phase 7: Auto-run /pr-fixup (if available) + +(Reached only when `--no-fixup` was NOT passed.) + +**Dependency check first.** `/pr-fixup` is a *personal* command — it normally +lives in `~/.claude/commands/pr-fixup.md` and is **not** vendored into this repo. +ship-it is committed and shared, so don't assume it's present. Check both the +project and user locations: + +```bash +{ test -f .claude/commands/pr-fixup.md || test -f ~/.claude/commands/pr-fixup.md; } \ + && echo "pr-fixup: available" || echo "pr-fixup: MISSING" +``` + +- **MISSING** → skip the rest of this phase. The PR is open and the reviews are + in; there's just no fixup command to run here. Carry this into the Phase 8 + report: state that auto-fixup was skipped because `/pr-fixup` isn't installed + in this environment, and that the user should run their own fixup (or vendor + `pr-fixup` into the repo) to continue. Do not try to hand-roll the triage. +- **Available** → continue with the steps below. + +1. Invoke the `/pr-fixup` command (via the Skill tool, skill `pr-fixup`) for the + PR you just opened. It triages every review comment, replies with decisions, + pulls in Codacy analysis, and applies the fixes worth making. +2. When `/pr-fixup` has applied its fixes, **commit and push them** — never + leave them uncommitted (this is the standing `/pr-fixup` expectation). Use a + `fix:` / `chore:` commit that references the review round, then `git push`. + Include a fresh changeset only if the fixes change package code in a way the + existing changeset doesn't already cover. +3. Do **one** pass only. Do NOT loop back to Phase 6 to wait for re-reviews of + the pushed fixes — that risks an endless ship→review→fix cycle. Report and + stop; the user can re-run `/ship-it` or `/pr-fixup` if they want another round. --- -## Phase 6: Report +## Phase 8: Report + +Lead with one sentence on what shipped, plus the PR URL. Don't re-summarise the +diff — the PR body already does. Then, unless `--no-fixup` was passed, add a +short recap of the review round: + +- Which reviewers' real reviews arrived (and, if the poller timed out, which + were still `missing` — call this out so the user can re-run later). +- What `/pr-fixup` did: comments addressed vs. dismissed, any Codacy issues + fixed/ignored, and whether fixup changes were committed and pushed — or, if + `/pr-fixup` wasn't installed in this environment, that auto-fixup was skipped + for that reason and the user should run their own. -One sentence on what shipped, plus the PR URL. Don't re-summarise the diff — -the PR body already does. If anything was skipped or changed from the -defaults (e.g. bump type defaulted to patch because ambiguous, branch name -had a suffix appended because of collision, pre-commit hook required a -retry), mention it in a single parenthetical line so the user can course- -correct if needed. +If anything was skipped or changed from the defaults (e.g. bump type defaulted +to patch because ambiguous, branch name had a suffix appended because of +collision, pre-commit hook required a retry, reviews timed out), mention it in a +single parenthetical line so the user can course-correct if needed. diff --git a/SPECS/README.md b/SPECS/README.md index 8118722..47c2cf0 100644 --- a/SPECS/README.md +++ b/SPECS/README.md @@ -69,3 +69,4 @@ _No pending tasks._ All commands implemented. | 2026-06-02 | `issues --overview` improvements: relabel False Positives buckets (`belowThreshold`/`equalOrAboveThreshold` → "Not a False Positive"/"Potential False Positive"), and a "Suggested actions to reduce noise" section that flags noisy patterns (≥10% of issues or ≥3× the average) with a runnable `codacy pattern … --disable` command, resolving the tool via its `prefix` (3 new tests, 360 total) | | 2026-06-02 | Pattern config-file & coding-standard awareness: new `pattern ` **info mode** (same card as `patterns`); `pattern`/`patterns` skip listing and refuse updates when a tool uses a local config file; `pattern` refuses to modify coding-standard-enforced patterns; `issues --overview` noise suggestions now render a manual "update your config file / coding standard" step instead of a command when a pattern can't be disabled via CLI. `printPatternCard`/`PATTERN_JSON_FIELDS` moved to `utils/formatting.ts` (11 new tests, 371 total) | | 2026-06-18 | `repo --output json` now includes `repository.fileCount`, plucked from `coverage.numberTotalFiles` on the existing `getRepositoryWithAnalysis` response (present even without coverage data — no extra API call). Unlocks repo-size visibility for downstream consumers like the `configure-codacy-cloud` skill (1 new test, 373 total) | +| 2026-06-24 | `findings` and `finding` now surface the vulnerable dependency's import chain from the new `dependencyChains` field: Direct (`Update to `) vs Transitive (` (Fixed in )`), with the middle collapsed to `... N more ...` for 4+ packages. List shows the first chain + `... and X more`; detail shows all chains aligned under a single label. New helpers in `utils/formatting.ts` (`formatDependencyChain`, `formatDependencyChainsLine`, `formatDependencyChainsBlock`); `dependencyChains` added to both JSON projections (17 new tests, 390 total) | diff --git a/SPECS/commands/finding.md b/SPECS/commands/finding.md index 1f73c2d..a2631b6 100644 --- a/SPECS/commands/finding.md +++ b/SPECS/commands/finding.md @@ -44,6 +44,7 @@ The `findingId` is the UUID shown in dim gray at the end of each findings card. {Finding title} {Status} {DueAt} | {Optional: CVE/CWE} | {Optional: AffectedVersion → FixedVersion} | {Optional: Application} | {Optional: AffectedTargets} +{Optional: Dependency import chains (SCA findings with dependencyChains)} {Optional: Ignored by {name} on {date}} {Optional: Ignored reason} @@ -69,6 +70,17 @@ When `item.cve` is present, fetch CVE data from `https://cveawg.mitre.org/api/cv For Codacy-source findings, the CVE block is injected between the code context and the pattern documentation. For non-Codacy-source findings, it follows the prose fields. +## Dependency import chains (SCA) + +When a finding carries `dependencyChains` (`string[][]`), **all** chains are listed +below the status line. The Direct/Transitive label (from the first chain) appears +once; continuation lines are indented so the `-` aligns under it. The +`AffectedVersion → FixedVersion` segment is dropped from the status line. + +Same per-chain rules as `findings` (direct → `Update to `; +transitive → ` (Fixed in )`; 4+ packages collapse the middle +to ` → ... N more ... → `). See `SPECS/commands/findings.md`. + ## Tests -File: `src/commands/finding.test.ts` — 14 tests (9 original + 5 for CVE enrichment). +File: `src/commands/finding.test.ts` — 23 tests. diff --git a/SPECS/commands/findings.md b/SPECS/commands/findings.md index 24f69ab..4b0c330 100644 --- a/SPECS/commands/findings.md +++ b/SPECS/commands/findings.md @@ -45,6 +45,7 @@ Card-style format: {Optional: affectedTargets} {Status} {DueAt} | {Optional: CVE or CWE} | {Optional: AffectedVersion → FixedVersion} | {Optional: Application} +{Optional: Dependency import chain (SCA findings with dependencyChains)} ──────────────────────────────────────── ``` @@ -55,6 +56,18 @@ Priority colors: Critical=red, High=orange, Medium=yellow, Low=blue. Shows pagination warning if more results exist. +### Dependency import chain (SCA) + +When a finding carries `dependencyChains` (`string[][]` — one ordered import chain +per entry, root → vulnerable package), a dedicated line is shown below the status +line, built from the **first** chain. The `AffectedVersion → FixedVersion` segment +is dropped from the status line (it would duplicate the chain line). + +- **Direct** (chain has 1 package): `Direct - Update to ` +- **Transitive** (2+ packages): `Transitive - → … → (Fixed in )` +- Chains with **4+ packages** collapse their middle: ` → ... N more ... → ` (N = length − 2). +- Multiple chains append `... and X more` (X = chains − 1). + ## Tests -File: `src/commands/findings.test.ts` — 13 tests. +File: `src/commands/findings.test.ts` — 24 tests. diff --git a/package.json b/package.json index 1a33189..ea78c8d 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "prepublishOnly": "npm run update-api && npm run build", "start": "npx ts-node src/index.ts", "start:dist": "node dist/index.js", - "fetch-api": "curl https://artifacts.codacy.com/api/codacy-api/55.12.1/apiv3-bundled.yaml -o ./api-v3/api-swagger.yaml --create-dirs", + "fetch-api": "curl https://artifacts.codacy.com/api/codacy-api/56.1.1/apiv3-bundled.yaml -o ./api-v3/api-swagger.yaml --create-dirs", "generate-api": "rm -rf ./src/api/client && openapi --input ./api-v3/api-swagger.yaml --output ./src/api/client --useUnionTypes --indent 2 --client fetch", "update-api": "npm run fetch-api && npm run generate-api", "check-types": "tsc --noEmit" diff --git a/src/commands/AGENTS.md b/src/commands/AGENTS.md index 5bddea7..00ddc5e 100644 --- a/src/commands/AGENTS.md +++ b/src/commands/AGENTS.md @@ -122,6 +122,12 @@ Several helpers are shared between `repository.ts` and `pull-request.ts` via `ut - `formatPrCoverage(pr, passing)` — diffCoverage% (+/-deltaCoverage%) - `formatPrIssues(pr, passing)` — +newIssues (colored by gate) / -fixedIssues (always gray) +Dependency-chain helpers shared between `findings.ts` (list) and `finding.ts` (detail): +- `formatVersionSegment(affectedVersion, fixedVersion, { includeUpdatePrefix })` — the `affected → fixed` status-line segment shown when a finding has no dependency chains; `includeUpdatePrefix` prepends `Update ` (list uses it, detail doesn't); returns `null` when there's no affected version +- `formatDependencyChain(chain)` — joins a chain with ` → `, collapsing the middle to ` → ... N more ... → ` when it has 4+ packages (≤ 3 shown in full) +- `formatDependencyChainsLine(chains, fixedVersion)` — one-line list summary: first chain with its Direct/Transitive label + `... and N more`; returns `null` for no chains +- `formatDependencyChainsBlock(chains, fixedVersion)` — multi-line detail block: all chains, label shown once, continuation lines aligned under the label; returns `null` for no chains + Pattern helpers shared between `patterns.ts` (list) and `pattern.ts` (single info): - `printPatternCard(cp)` — the configured-pattern card (icons, enforced-by line, metadata, why/how, parameters) - `PATTERN_JSON_FIELDS` — `pickDeep` paths for the JSON projection of a `ConfiguredPattern` @@ -213,6 +219,7 @@ Keeps the two command handlers thin: they only supply the API-specific callbacks - `-R, --ignore-reason`: `AcceptedUse` (default) | `FalsePositive` | `NotExploitable` | `TestCode` | `ExternalCode` - `-m, --ignore-comment`: optional free-text comment - **`--unignore` mode** (`-U`): calls `SecurityService.unignoreSecurityItem`; skips rendering finding details +- **Dependency import chains** (SCA findings): when `item.dependencyChains` (`string[][]`) is present, both `finding` (detail) and `findings` (list) render the vulnerable dependency's import path. A chain with a single package is a **direct** dependency (`Direct - Update to `); 2+ packages is **transitive** (`Transitive - (Fixed in )`). Chains with **4+ packages** collapse the middle to ` → ... N more ... → ` (N = length − 2). The list shows only the first chain + `... and X more`; the detail lists **all** chains with the Direct/Transitive label shown once and continuation lines indented so the `-` aligns. When chains are present, the redundant `AffectedVersion → FixedVersion` segment is dropped from the status line. Mixed direct/transitive chains (rare) take their label from the first chain. Rendering lives in `formatDependencyChainsLine` / `formatDependencyChainsBlock` (see Shared Formatting Utilities). ## pull-request command (`pull-request.ts`) diff --git a/src/commands/finding.test.ts b/src/commands/finding.test.ts index 143ae0e..886af9f 100644 --- a/src/commands/finding.test.ts +++ b/src/commands/finding.test.ts @@ -108,6 +108,28 @@ const mockScaFinding = { remediation: "Upgrade to lodash >= 4.17.21", }; +// An SCA finding reached transitively via multiple import chains (one collapsed). +const mockScaWithChains = { + id: "mno-345-chains", + itemSource: "SCA" as any, + itemSourceId: "npm-minimatch-1", + title: "Denial of Service in minimatch", + repository: "my-repo", + openedAt: "2025-01-01T00:00:00Z", + dueAt: "2026-06-16T00:00:00Z", + priority: "High", + status: "Overdue", + securityCategory: "InsecureModulesLibraries", + scanType: "SCA", + cve: "CVE-2026-39363", + affectedVersion: "0.1.2", + fixedVersion: ["0.1.5"], + dependencyChains: [ + ["package@1.0.0", "anotherPackage@0.5.2", "minimatch@0.1.2"], + ["anotherPackage@1.0.0", "b@1", "c@2", "d@3", "e@4", "minimatch@0.1.1"], + ], +}; + // A Codacy-source finding linked to a quality issue const mockCodacyFinding = { id: "def-456-codacy", @@ -239,6 +261,43 @@ describe("finding command", () => { expect(output).toContain("abc-123-sca"); }); + it("should render all dependency chains with the label once and a collapsed middle", async () => { + vi.mocked(SecurityService.getSecurityItem).mockResolvedValue({ + data: mockScaWithChains, + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "finding", "gh", "test-org", "mno-345-chains", + ]); + + const output = getAllOutput(); + expect(output).toContain( + "Transitive - package@1.0.0 → anotherPackage@0.5.2 → minimatch@0.1.2 (Fixed in 0.1.5)", + ); + // Continuation line aligned under "Transitive ", with the long chain collapsed. + expect(output).toContain( + " - anotherPackage@1.0.0 → ... 4 more ... → minimatch@0.1.1 (Fixed in 0.1.5)", + ); + // The redundant affectedVersion segment is dropped when chains exist. + expect(output).not.toContain("0.1.2 → 0.1.5"); + }); + + it("should include dependencyChains in JSON output", async () => { + vi.mocked(SecurityService.getSecurityItem).mockResolvedValue({ + data: mockScaWithChains, + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "--output", "json", "finding", "gh", "test-org", "mno-345-chains", + ]); + + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('"dependencyChains"'), + ); + }); + it("should show pattern info for Codacy-source findings", async () => { vi.mocked(SecurityService.getSecurityItem).mockResolvedValue({ data: mockCodacyFinding, diff --git a/src/commands/finding.ts b/src/commands/finding.ts index b05f02c..0f0e2de 100644 --- a/src/commands/finding.ts +++ b/src/commands/finding.ts @@ -8,6 +8,8 @@ import { colorPriority, colorStatus, formatDueDate, + formatVersionSegment, + formatDependencyChainsBlock, printCveBlock, printIssueCodeContext, } from "../utils/formatting"; @@ -76,12 +78,15 @@ function printFindingDetail( if (item.cve) line2Parts.push(ansis.dim(item.cve)); else if (item.cwe) line2Parts.push(ansis.dim(`CWE-${item.cwe}`)); - if (item.affectedVersion) { - const fixed = - item.fixedVersion && item.fixedVersion.length > 0 - ? ` → ${item.fixedVersion.join(", ")}` - : ""; - line2Parts.push(ansis.dim(`${item.affectedVersion}${fixed}`)); + // When dependency chains are present they carry the vulnerable package and + // fixed version on their own line, so the redundant version segment is dropped. + const hasChains = !!item.dependencyChains?.length; + if (!hasChains) { + const versionSegment = formatVersionSegment( + item.affectedVersion, + item.fixedVersion, + ); + if (versionSegment) line2Parts.push(ansis.dim(versionSegment)); } if (item.application) line2Parts.push(ansis.dim(item.application)); @@ -89,6 +94,15 @@ function printFindingDetail( console.log(line2Parts.join(pipe)); + // Dependency import chains (SCA findings with dependencyChains) + if (hasChains) { + const chainBlock = formatDependencyChainsBlock( + item.dependencyChains, + item.fixedVersion, + ); + if (chainBlock) console.log(ansis.dim(chainBlock)); + } + // Ignored section if (item.ignored) { const ig = item.ignored; @@ -249,6 +263,7 @@ Examples: "finding.fixedVersion", "finding.application", "finding.affectedTargets", + "finding.dependencyChains", "finding.ignored", "finding.summary", "finding.additionalInfo", diff --git a/src/commands/findings.test.ts b/src/commands/findings.test.ts index 37238b2..3f2fba9 100644 --- a/src/commands/findings.test.ts +++ b/src/commands/findings.test.ts @@ -75,6 +75,47 @@ const mockPenTestFinding = { application: "my-app", }; +// SCA finding reached transitively via multiple import chains. +const mockScaTransitive = { + id: "finding-sca-transitive", + itemSource: "SCA", + itemSourceId: "sca-1", + title: "Denial of Service in minimatch", + repository: "my-repo", + openedAt: "2024-02-01T00:00:00Z", + dueAt: "2026-06-16T00:00:00Z", + priority: "High", + status: "Overdue", + securityCategory: "InsecureModulesLibraries", + scanType: "SCA", + cve: "CVE-2026-27903", + affectedVersion: "0.1.2", + fixedVersion: ["0.1.5"], + dependencyChains: [ + ["package@1.0.0", "anotherPackage@0.5.2", "minimatch@0.1.2"], + ["other@1.0.0", "minimatch@0.1.2"], + ], +}; + +// SCA finding for a directly-imported vulnerable dependency. +const mockScaDirect = { + id: "finding-sca-direct", + itemSource: "SCA", + itemSourceId: "sca-2", + title: "Denial of Service in minimatch", + repository: "my-repo", + openedAt: "2024-02-01T00:00:00Z", + dueAt: "2026-06-16T00:00:00Z", + priority: "High", + status: "Overdue", + securityCategory: "InsecureModulesLibraries", + scanType: "SCA", + cve: "CVE-2026-27903", + affectedVersion: "0.1.2", + fixedVersion: ["0.1.5"], + dependencyChains: [["minimatch@0.1.2"]], +}; + function getAllOutput(): string { return (console.log as ReturnType).mock.calls .map((c) => c[0]) @@ -445,6 +486,48 @@ describe("findings command", () => { expect(output).toContain("1.0.1"); }); + it("should show a transitive dependency chain with '... and N more' for multiple chains", async () => { + vi.mocked(SecurityService.searchSecurityItems).mockResolvedValue({ + data: [mockScaTransitive], + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", + "test", + "findings", + "gh", + "test-org", + "test-repo", + ]); + + const output = getAllOutput(); + expect(output).toContain( + "Transitive - package@1.0.0 → anotherPackage@0.5.2 → minimatch@0.1.2 (Fixed in 0.1.5) ... and 1 more", + ); + // The redundant "Update " segment is dropped when chains exist. + expect(output).not.toContain("Update 0.1.2"); + }); + + it("should show a direct dependency as actionable update text", async () => { + vi.mocked(SecurityService.searchSecurityItems).mockResolvedValue({ + data: [mockScaDirect], + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", + "test", + "findings", + "gh", + "test-org", + "test-repo", + ]); + + const output = getAllOutput(); + expect(output).toContain("Direct - Update minimatch@0.1.2 to 0.1.5"); + }); + it("should show likelihood and effortToFix for pen test findings", async () => { vi.mocked(SecurityService.searchSecurityItems).mockResolvedValue({ data: [mockPenTestFinding], diff --git a/src/commands/findings.ts b/src/commands/findings.ts index ad1ab78..32ea180 100644 --- a/src/commands/findings.ts +++ b/src/commands/findings.ts @@ -15,6 +15,8 @@ import { colorPriority, colorStatus, formatDueDate, + formatVersionSegment, + formatDependencyChainsLine, } from "../utils/formatting"; import { SecurityService } from "../api/client/services/SecurityService"; import { SrmItem } from "../api/client/models/SrmItem"; @@ -110,18 +112,32 @@ function printFindingCard(item: SrmItem, showRepo: boolean): void { if (item.cve) line3Parts.push(ansis.dim(item.cve)); else if (item.cwe) line3Parts.push(ansis.dim(`CWE-${item.cwe}`)); - if (item.affectedVersion) { - const fixed = - item.fixedVersion && item.fixedVersion.length > 0 - ? ` → ${item.fixedVersion.join(", ")}` - : ""; - line3Parts.push(ansis.dim(`Update ${item.affectedVersion}${fixed}`)); + // When dependency chains are present they carry the vulnerable package and + // fixed version on their own line, so the redundant version segment is dropped. + const hasChains = !!item.dependencyChains?.length; + if (!hasChains) { + const versionSegment = formatVersionSegment( + item.affectedVersion, + item.fixedVersion, + { includeUpdatePrefix: true }, + ); + if (versionSegment) line3Parts.push(ansis.dim(versionSegment)); } if (item.application) line3Parts.push(ansis.dim(item.application)); //if (item.affectedTargets) line3Parts.push(ansis.dim(item.affectedTargets)); console.log(line3Parts.join(pipe)); + + // Line 4: dependency import chain (SCA findings with dependencyChains) + if (hasChains) { + const chainLine = formatDependencyChainsLine( + item.dependencyChains, + item.fixedVersion, + ); + if (chainLine) console.log(ansis.dim(chainLine)); + } + console.log(); console.log(separator); } @@ -306,6 +322,7 @@ Examples: "fixedVersion", "application", "affectedTargets", + "dependencyChains", ])), total, }); diff --git a/src/utils/formatting.test.ts b/src/utils/formatting.test.ts index c31a367..1a1c854 100644 --- a/src/utils/formatting.test.ts +++ b/src/utils/formatting.test.ts @@ -4,6 +4,10 @@ import { resolveToolUuids, formatDuration, isBeingAnalyzed, + formatVersionSegment, + formatDependencyChain, + formatDependencyChainsLine, + formatDependencyChainsBlock, } from "./formatting"; // Mock ansis to return raw text for easier testing @@ -210,3 +214,132 @@ describe("isBeingAnalyzed", () => { expect(isBeingAnalyzed(undefined, undefined)).toBe(false); }); }); + +describe("formatVersionSegment", () => { + it("returns null when there is no affected version", () => { + expect(formatVersionSegment(undefined, ["1.0.1"])).toBeNull(); + }); + + it("formats affected → fixed without a prefix by default", () => { + expect(formatVersionSegment("1.0.0", ["1.0.1", "1.1.0"])).toBe( + "1.0.0 → 1.0.1, 1.1.0", + ); + }); + + it("prepends 'Update ' when requested", () => { + expect( + formatVersionSegment("1.0.0", ["1.0.1"], { includeUpdatePrefix: true }), + ).toBe("Update 1.0.0 → 1.0.1"); + }); + + it("omits the fixed suffix when no fixed version is given", () => { + expect(formatVersionSegment("1.0.0", [])).toBe("1.0.0"); + expect(formatVersionSegment("1.0.0")).toBe("1.0.0"); + }); +}); + +describe("formatDependencyChain", () => { + it("shows a 2-package chain in full", () => { + expect(formatDependencyChain(["a@1", "m@0.1.2"])).toBe("a@1 → m@0.1.2"); + }); + + it("shows a 3-package chain in full", () => { + expect(formatDependencyChain(["a@1", "b@2", "m@0.1.2"])).toBe( + "a@1 → b@2 → m@0.1.2", + ); + }); + + it("collapses the middle of a 4-package chain to '2 more'", () => { + expect(formatDependencyChain(["a@1", "b@2", "c@3", "d@4"])).toBe( + "a@1 → ... 2 more ... → d@4", + ); + }); + + it("collapses the middle of a 5-package chain to '3 more'", () => { + expect(formatDependencyChain(["a@1", "b@2", "c@3", "d@4", "e@5"])).toBe( + "a@1 → ... 3 more ... → e@5", + ); + }); + + it("shows a single-package chain as-is", () => { + expect(formatDependencyChain(["m@0.1.2"])).toBe("m@0.1.2"); + }); +}); + +describe("formatDependencyChainsLine", () => { + it("returns null for empty/undefined chains", () => { + expect(formatDependencyChainsLine([])).toBeNull(); + expect(formatDependencyChainsLine(undefined)).toBeNull(); + }); + + it("renders a direct dependency as actionable update text", () => { + expect(formatDependencyChainsLine([["minimatch@0.1.2"]], ["0.1.5"])).toBe( + "Direct - Update minimatch@0.1.2 to 0.1.5", + ); + }); + + it("renders a transitive chain with the fixed version", () => { + expect( + formatDependencyChainsLine( + [["package@1.0.0", "anotherPackage@0.5.2", "minimatch@0.1.2"]], + ["0.1.5"], + ), + ).toBe( + "Transitive - package@1.0.0 → anotherPackage@0.5.2 → minimatch@0.1.2 (Fixed in 0.1.5)", + ); + }); + + it("appends '... and N more' when there are extra chains", () => { + expect( + formatDependencyChainsLine( + [ + ["a@1", "m@0.1.2"], + ["b@1", "m@0.1.2"], + ["c@1", "m@0.1.2"], + ], + ["0.1.5"], + ), + ).toBe("Transitive - a@1 → m@0.1.2 (Fixed in 0.1.5) ... and 2 more"); + }); + + it("omits the fixed-version suffix when none is provided", () => { + expect(formatDependencyChainsLine([["a@1", "m@0.1.2"]])).toBe( + "Transitive - a@1 → m@0.1.2", + ); + expect(formatDependencyChainsLine([["minimatch@0.1.2"]])).toBe( + "Direct - Update minimatch@0.1.2", + ); + }); +}); + +describe("formatDependencyChainsBlock", () => { + it("returns null for empty/undefined chains", () => { + expect(formatDependencyChainsBlock([])).toBeNull(); + expect(formatDependencyChainsBlock(undefined)).toBeNull(); + }); + + it("renders all chains with the label once and aligned continuation lines", () => { + const block = formatDependencyChainsBlock( + [ + ["package@1.0.0", "anotherPackage@0.5.2", "minimatch@0.1.2"], + ["anotherPackage@1.0.0", "b@1", "c@2", "d@3", "e@4", "minimatch@0.1.1"], + ], + ["0.1.5"], + ); + expect(block).toBe( + "Transitive - package@1.0.0 → anotherPackage@0.5.2 → minimatch@0.1.2 (Fixed in 0.1.5)\n" + + " - anotherPackage@1.0.0 → ... 4 more ... → minimatch@0.1.1 (Fixed in 0.1.5)", + ); + }); + + it("aligns continuation lines under a shorter 'Direct' label", () => { + const block = formatDependencyChainsBlock( + [["minimatch@0.1.2"], ["minimatch@0.1.3"]], + ["0.1.5"], + ); + expect(block).toBe( + "Direct - Update minimatch@0.1.2 to 0.1.5\n" + + " - Update minimatch@0.1.3 to 0.1.5", + ); + }); +}); diff --git a/src/utils/formatting.ts b/src/utils/formatting.ts index 4991e61..5adc7da 100644 --- a/src/utils/formatting.ts +++ b/src/utils/formatting.ts @@ -83,6 +83,97 @@ export function formatDueDate(dateStr: string): string { return dateFnsFormat(date, "yyyy-MM-dd"); } +// --- Dependency chains (SCA findings) ------------------------------------- +// +// An SCA finding may carry `dependencyChains` (string[][]): each inner array is +// one ordered import chain from a root package down to the vulnerable package +// (the last segment). A finding can have several chains reaching the same +// vulnerable package via different paths. We surface these on both the findings +// list and the finding detail. + +// Chains with more than this many packages collapse their middle into "... N more ...". +const CHAIN_FULL_MAX = 3; + +/** + * Join a dependency chain with " → ", collapsing the middle when the chain has + * more than 3 packages (showing only the first and last, e.g. + * `a@1 → ... 2 more ... → d@4`). Chains with ≤ 3 packages are shown in full. + */ +export function formatDependencyChain(chain: string[]): string { + if (chain.length <= CHAIN_FULL_MAX) return chain.join(" → "); + const hidden = chain.length - 2; + return `${chain[0]} → ... ${hidden} more ... → ${chain[chain.length - 1]}`; +} + +/** + * Render the body of a single chain line (without the Direct/Transitive label). + * A single-package chain is a direct dependency and gets actionable update text; + * longer chains show the (possibly collapsed) import path plus the fixed version. + */ +function dependencyChainBody(chain: string[], fixedVersion?: string[]): string { + const fixed = fixedVersion?.length ? fixedVersion.join(", ") : ""; + if (chain.length === 1) { + // Direct dependency: the package is imported directly, so show how to fix it. + return fixed ? `Update ${chain[0]} to ${fixed}` : `Update ${chain[0]}`; + } + const suffix = fixed ? ` (Fixed in ${fixed})` : ""; + return `${formatDependencyChain(chain)}${suffix}`; +} + +/** + * Format the "affected → fixed" version segment shown on a finding's status line + * for SCA findings that have no dependency chains. Returns null when there is no + * affected version. `includeUpdatePrefix` prepends "Update " (the findings list + * uses it; the finding detail does not). + */ +export function formatVersionSegment( + affectedVersion?: string, + fixedVersion?: string[], + options?: { includeUpdatePrefix?: boolean }, +): string | null { + if (!affectedVersion) return null; + const fixed = fixedVersion?.length ? ` → ${fixedVersion.join(", ")}` : ""; + const prefix = options?.includeUpdatePrefix ? "Update " : ""; + return `${prefix}${affectedVersion}${fixed}`; +} + +/** + * One-line dependency summary for the findings list: the first chain prefixed + * with its Direct/Transitive label, plus "... and N more" when there are extra + * chains. Returns null when there are no chains. + */ +export function formatDependencyChainsLine( + chains?: string[][], + fixedVersion?: string[], +): string | null { + if (!chains?.length) return null; + const first = chains[0]; + const label = first.length === 1 ? "Direct" : "Transitive"; + let line = `${label} - ${dependencyChainBody(first, fixedVersion)}`; + if (chains.length > 1) line += ` ... and ${chains.length - 1} more`; + return line; +} + +/** + * Multi-line dependency block for the finding detail: every chain on its own + * line. The label (from the first chain) is shown once; continuation lines are + * indented so the "-" aligns under it. Returns null when there are no chains. + */ +export function formatDependencyChainsBlock( + chains?: string[][], + fixedVersion?: string[], +): string | null { + if (!chains?.length) return null; + const label = chains[0].length === 1 ? "Direct" : "Transitive"; + const indent = " ".repeat(label.length + 1) + "- "; + return chains + .map( + (chain, i) => + `${i === 0 ? `${label} - ` : indent}${dependencyChainBody(chain, fixedVersion)}`, + ) + .join("\n"); +} + /** * Print a single issue card shared by the `issues` and `pull-request` commands. * The issue ID (resultDataId) is appended at the end of the first line in a