Skip to content

fix: preserve maximum advisory severity - #1401

Merged
kang-heewon merged 2 commits into
trunkfrom
issue-1340-preserve-advisory-severity
Jul 15, 2026
Merged

fix: preserve maximum advisory severity#1401
kang-heewon merged 2 commits into
trunkfrom
issue-1340-preserve-advisory-severity

Conversation

@kang-heewon

@kang-heewon kang-heewon commented Jul 15, 2026

Copy link
Copy Markdown
Member

Fixes #1340.

Summary

Merged dependency-audit evidence now preserves the maximum observed severity regardless of audit-pass order, so later low or moderate records cannot downgrade high or critical blockers.

Advisory identities are normalized through validated GHSA, CVE, and audit-id namespaces. Merge grouping considers every normalized alias and absorbs intersecting groups transitively, including cases where the shared CVE is not the first sorted CVE. Missing or malformed identity and severity evidence fails closed with stable diagnostics.

Merged paths, CVEs, scalar evidence, advisory ordering, and generated reports are deterministic across reversed inputs.

Verification

  • pnpm vitest run scripts/tests/dependency-audit-policy.spec.ts scripts/tests/release-workflow.spec.ts — 49/49 passed
  • Focused policy suite repeated 10 times — 370/370 executions passed
  • Real CLI adversarial harness — both merge orders retained critical, produced byte-identical reports, and malformed or hostile evidence exited nonzero without reflecting payloads
  • pnpm check — passed
  • pnpm typecheck — 226/226 tasks passed across 115 packages
  • pnpm test — 227/227 tasks passed
  • pnpm changeset-required:check — passed; no publishable package changes
  • GitHub CI validate, changes, benchmark, companion, WIP, and CodeRabbit — passed
  • git diff --check — passed

pnpm security:audit-policy cannot currently reach npm's retired legacy audit endpoint; the registry returns HTTP 410 and directs clients to the bulk advisory endpoint. GitHub CI's dependency audit policy report passed, and the local policy suite plus real CLI fixtures cover the changed behavior without network dependency.

Review gates

  • Correctness and regression: pass — all severity pairs are covered in both directions; GHSA, CVE-only, numeric-id, missing-id, malformed, alias-chain, and deterministic-report cases are covered.
  • API, security, and compatibility: pass — independent security review approved the final patch; canonical lowercase GHSA suffixes remain compatible with reviewed metadata.
  • Maintainability and minimality: pass — the change is confined to the policy script and its tests, adds no dependency, and requires no changeset.
  • Architecture review: clear.
  • Adversarial QA: pass.
  • CodeRabbit: approved; all actionable review threads resolved.

No known issue-scoped residual risk remains.

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

감사 advisory의 식별자와 severity를 정규화·검증하고, 중복 결과를 결정적으로 병합하도록 정책 로직과 테스트 픽스처를 확장했습니다. 리포트의 Advisory 표시값은 GHSA, CVE, audit id 순으로 선택됩니다.

Changes

감사 advisory 병합 정책

Layer / File(s) Summary
정규화 및 병합 규칙
scripts/dependency-audit-policy.mts
advisory 식별자와 severity를 canonical 형태로 검증하고, GHSA·CVE·audit id 기준으로 중복 advisory를 병합합니다. severity는 가장 높은 값을 유지하며 CVE와 경로는 정렬합니다.
병합 결과 리포트 출력
scripts/dependency-audit-policy.mts
Advisory 컬럼이 GHSA, 첫 CVE, audit id 순으로 표시값을 선택합니다.
병합 및 입력 검증 테스트
scripts/tests/dependency-audit-policy.spec.ts
severity 보존, 식별자별 병합, 잘못된 입력 거부, 입력 순서 독립성 및 기존 정책 시나리오를 검증하도록 픽스처와 테스트를 확장합니다.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant AuditFiles
  participant normalizeAdvisory
  participant mergeAdvisories
  participant Report
  AuditFiles->>normalizeAdvisory: advisory 데이터 전달
  normalizeAdvisory->>mergeAdvisories: canonical advisory 전달
  mergeAdvisories->>mergeAdvisories: 식별자별 병합 및 최대 severity 선택
  mergeAdvisories->>Report: 병합된 evidence 전달
  Report-->>AuditFiles: Advisory 컬럼 포함 리포트 생성
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed 병합 시 최대 severity 보존, 안정적 식별자 검증, 결정적 정렬 및 관련 테스트 추가가 요구사항과 일치합니다.
Out of Scope Changes check ✅ Passed 테스트 보강과 정규화/리포트 정렬 변경은 모두 severity 병합 문제 해결 범위 안에 있습니다.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목이 advisory 병합 시 최대 severity를 보존하는 핵심 변경을 정확히 요약합니다.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-1340-preserve-advisory-severity

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/dependency-audit-policy.mts`:
- Around line 349-358: Update advisoryKey and its merge lookup flow in
scripts/dependency-audit-policy.mts:349-358 to consider every normalized CVE
alias, rather than only advisory.cves[0], so advisories sharing any CVE resolve
to the same merge group and retain maximum severity. Add a regression test in
scripts/tests/dependency-audit-policy.spec.ts:421-467 where the shared CVE is
not the first sorted CVE on one side, verifying the advisories merge correctly.

In `@scripts/tests/dependency-audit-policy.spec.ts`:
- Around line 950-960: Update expectPolicyProblem so it captures the exception
from run and performs the expected rejection assertion after the try/catch,
removing the generic Error sentinel entirely. Ensure the helper still verifies
the captured exception has category "BadRequest" and the provided message, and
only Problem subclasses are thrown.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: a58612be-4c63-4cc4-8c5d-f0b9389776b2

📥 Commits

Reviewing files that changed from the base of the PR and between 1910ff9 and 296f460.

📒 Files selected for processing (2)
  • scripts/dependency-audit-policy.mts
  • scripts/tests/dependency-audit-policy.spec.ts

Comment thread scripts/dependency-audit-policy.mts Outdated
Comment thread scripts/tests/dependency-audit-policy.spec.ts Outdated
@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown

📊 Benchmark Results

✅ All benchmarks passed

Benchmark p75 Threshold Baseline vs Baseline Status Notes
CrocoApp constructor 6.7μs 30.0ms 8.2μs -17.5% -
CrocoApp lambdaHandler (10 controllers) 178.4μs 50.0ms 258.4μs -31.0% -
Lambda cold-start simulation 313.3μs 80.0ms 418.1μs -25.1% -
Lambda cold-start with headers 266.4μs 80.0ms 369.7μs -27.9% -
Lambda cold-start with binary body 221.2μs 80.0ms 339.1μs -34.8% -
Lambda cold-start with query params 193.6μs 80.0ms 301.3μs -35.7% -
Lambda cold-start with authorizer context 195.9μs 80.0ms 299.8μs -34.7% -
Lambda cold-start realistic scenario 209.3μs 80.0ms 299.2μs -30.0% -
EventBusConfig.start (10 handlers) 1.2μs 10.0ms 1.4μs -18.1% -
EventPublisher.publishNow single event 1.4μs 2.0ms 1.7μs -14.8% -
DefaultHandlerResolver.resolve × 10 0.1μs 5.0ms 0.1μs -22.5% -
Container.get singleton (cold) 49.3μs 5.0ms 70.3μs -29.8% -
Container.register × 50 components 2.6ms 10.0ms 3.2ms -19.6% -
Container.validate (50 components) 2.9ms 20.0ms 3.4ms -15.8% -
Container.get singleton (warm) 1.3μs 500.0μs 1.6μs -21.9% -
TelemetryRuntime.init (lambda preset) 1.6μs 200.0ms 1.1ms -99.9% -
lambdaPreset config creation 1.2μs 2.0ms 1.4μs -17.1% -

Updated: 2026-07-15T11:42:32.917Z · Commit: 64a9171

@kang-heewon

Copy link
Copy Markdown
Member Author

@coderabbitai resolve

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
✅ Action performed

Comments resolved and changes approved.

@kang-heewon
kang-heewon merged commit 967650c into trunk Jul 15, 2026
9 checks passed
@kang-heewon
kang-heewon deleted the issue-1340-preserve-advisory-severity branch July 15, 2026 12:25
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.

[security] Preserve maximum advisory severity when audit results are merged

1 participant