HARMONY-2268: Change audit to ignore dev dependencies as best as we can#861
HARMONY-2268: Change audit to ignore dev dependencies as best as we can#861chris-durbin merged 4 commits intomainfrom
Conversation
…an. For dev dependencies that can't be ignored because they are pulled in from subdependencies add a note and remove the expiration.
chris-durbin
requested review from
flamingbear,
indiejames and
ygliuvt
as code owners
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRoot-level audit scripting was reorganized and many package test/better-audit scripts were updated to delegate to a new audit script with a progress flag. Multiple Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 8: The current "test" script in package.json uses "npm run audit-deps &&
lerna run test --load-env-files=false" which causes audit failures to
short-circuit and skip tests; modify the script so the audit runs but does not
block tests (e.g., run audit as a separate command that does not return
non‑zero, or use a shell separator/ignore-failure pattern so lerna always runs)
and ensure known/accepted root-level vulnerabilities are listed in .nsprc;
update the "test" script entry and verify behavior of the "audit-deps" task and
.nsprc suppression so CI shows test failures separately from audit results.
In `@services/harmony/.nsprc`:
- Around line 17-20: Update the GHSA entry for GHSA-2g4f-4pwh-qvx6 to match the
dev-dependency convention: remove the "expiry" property and change the "notes"
value to "Dev dependency in packages/util" while leaving "active": true intact;
locate the GHSA-2g4f-4pwh-qvx6 object in the .nsprc and make those edits so it
matches the other services' entries.
In `@services/work-scheduler/.nsprc`:
- Around line 2-5: Remove the "expiry" field from the advisory entry keyed by
"GHSA-p8p7-x288-28g6" in the .nsprc file: keep "active": true and the existing
"notes" value ("Will fix in HARMONY-1729") but delete the "expiry" property so
the entry complies with HARMONY-2268 for dev-dependency advisories.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
.nsprcpackage.jsonpackages/util/.nsprcpackages/util/package.jsonservices/cron-service/.nsprcservices/cron-service/package.jsonservices/harmony/.nsprcservices/harmony/package.jsonservices/query-cmr/.nsprcservices/query-cmr/package.jsonservices/service-runner/.nsprcservices/service-runner/package.jsonservices/work-failer/.nsprcservices/work-failer/package.jsonservices/work-scheduler/.nsprcservices/work-scheduler/package.jsonservices/work-updater/.nsprcservices/work-updater/package.json
| "scripts": { | ||
| "test": "better-npm-audit audit -l moderate && lerna run test --load-env-files=false", | ||
| "audit-deps": "better-npm-audit audit -l moderate -p", | ||
| "test": "npm run audit-deps && lerna run test --load-env-files=false", |
There was a problem hiding this comment.
Heads-up: audit failure now hard-blocks all tests via &&.
If a new vulnerability surfaces in root-level production dependencies (or a stale .nsprc entry expires), lerna run test will be skipped entirely. This is intentional per the PR's fail-fast goal, but it means CI red won't distinguish "tests failed" from "audit blocked tests". Make sure any known/accepted root-level vulnerabilities are suppressed in .nsprc to avoid spurious CI blocks.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 8, The current "test" script in package.json uses "npm
run audit-deps && lerna run test --load-env-files=false" which causes audit
failures to short-circuit and skip tests; modify the script so the audit runs
but does not block tests (e.g., run audit as a separate command that does not
return non‑zero, or use a shell separator/ignore-failure pattern so lerna always
runs) and ensure known/accepted root-level vulnerabilities are listed in .nsprc;
update the "test" script entry and verify behavior of the "audit-deps" task and
.nsprc suppression so CI shows test failures separately from audit results.
| "GHSA-2g4f-4pwh-qvx6": { | ||
| "active": true, | ||
| "notes": "Will fix in HARMONY-2261", | ||
| "expiry": "2026-06-01" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n --glob '*.nsprc' 'GHSA-2g4f-4pwh-qvx6' -C 3Repository: nasa/harmony
Length of output: 1610
Align GHSA-2g4f-4pwh-qvx6 with the dev-dependency policy used across all other services.
This advisory is treated as a dev dependency in services/work-updater, services/service-runner, services/cron-service, and services/query-cmr—all with the note "Dev dependency in packages/util" and no expiry. The harmony service should follow the same pattern. Remove the expiry date and update the note to match the established convention.
✏️ Suggested fix
"GHSA-2g4f-4pwh-qvx6": {
"active": true,
- "notes": "Will fix in HARMONY-2261",
- "expiry": "2026-06-01"
+ "notes": "Dev dependency in packages/util"
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "GHSA-2g4f-4pwh-qvx6": { | |
| "active": true, | |
| "notes": "Will fix in HARMONY-2261", | |
| "expiry": "2026-06-01" | |
| "GHSA-2g4f-4pwh-qvx6": { | |
| "active": true, | |
| "notes": "Dev dependency in packages/util" | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/harmony/.nsprc` around lines 17 - 20, Update the GHSA entry for
GHSA-2g4f-4pwh-qvx6 to match the dev-dependency convention: remove the "expiry"
property and change the "notes" value to "Dev dependency in packages/util" while
leaving "active": true intact; locate the GHSA-2g4f-4pwh-qvx6 object in the
.nsprc and make those edits so it matches the other services' entries.
| "GHSA-p8p7-x288-28g6": { | ||
| "active": true, | ||
| "notes": "Will fix in HARMONY-1729", | ||
| "expiry": "2026-06-01" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Examine the .nsprc file to see the full context
cat -n services/work-scheduler/.nsprcRepository: nasa/harmony
Length of output: 227
🏁 Script executed:
# Search for any PR or commit context about this advisory
git log --oneline -20 -- services/work-scheduler/.nsprcRepository: nasa/harmony
Length of output: 266
🏁 Script executed:
# Search for documentation or context about dev-dep advisory policy in the PR
git log -p --all -5 -S "dev-dep" -- ".nsprc*" | head -200Repository: nasa/harmony
Length of output: 38
🏁 Script executed:
# Search for references to GHSA-p8p7-x288-28g6 to identify the affected package
rg "GHSA-p8p7-x288-28g6" --type json --type yamlRepository: nasa/harmony
Length of output: 38
🏁 Script executed:
# Examine package.json in services/work-scheduler to understand dependencies
cd services/work-scheduler && cat -n package.json | head -100Repository: nasa/harmony
Length of output: 4640
🏁 Script executed:
# Search for npm audit or security scan output that mentions this advisory
git log --all -p -S "GHSA-p8p7-x288-28g6" | head -200Repository: nasa/harmony
Length of output: 142
Remove expiry per HARMONY-2268 policy for dev-dependency advisories.
The expiry field conflicts with the policy established in HARMONY-2268: "For dev dependencies that can't be ignored because they are pulled in from subdependencies add a note and remove the expiration." The entry already includes the required note; remove the expiry field.
Suggested change
"GHSA-p8p7-x288-28g6": {
"active": true,
"notes": "Will fix in HARMONY-1729",
- "expiry": "2026-06-01"
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "GHSA-p8p7-x288-28g6": { | |
| "active": true, | |
| "notes": "Will fix in HARMONY-1729", | |
| "expiry": "2026-06-01" | |
| "GHSA-p8p7-x288-28g6": { | |
| "active": true, | |
| "notes": "Will fix in HARMONY-1729" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/work-scheduler/.nsprc` around lines 2 - 5, Remove the "expiry" field
from the advisory entry keyed by "GHSA-p8p7-x288-28g6" in the .nsprc file: keep
"active": true and the existing "notes" value ("Will fix in HARMONY-1729") but
delete the "expiry" property so the entry complies with HARMONY-2268 for
dev-dependency advisories.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
services/cron-service/.nsprc (1)
1-1: LGTM — same rationale asservices/query-cmr/.nsprc.The removed entries (
1113429,1113371) were dev-dependency advisories now handled at the audit script level. No prod-dependency advisories remain for this service, so{}is correct.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/cron-service/.nsprc` at line 1, The .nsprc file should remain an empty object {} because the removed advisory entries 1113429 and 1113371 were dev-dependency advisories handled by the audit script; ensure the .nsprc in this change contains only {} (no prod advisories), and confirm any dev advisory handling is delegated to the audit script rather than re-adding those IDs.services/work-updater/.nsprc (1)
1-1: LGTM — same rationale asservices/query-cmr/.nsprc.Consistent with the other emptied service configs. No further suppressions are required here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/work-updater/.nsprc` at line 1, No changes required—leave the .nsprc for the work-updater service empty to match the same intentional suppression pattern used for the query-cmr service; approve and keep the consistent empty config (no code edits to functions or classes are needed).
🧹 Nitpick comments (1)
services/query-cmr/.nsprc (1)
1-1: LGTM — removal of npm-ID-based dev dependency suppressions is consistent with the PR objective.Entries
1113429and1113371were dev-dependency advisories; since the audit script now filters dev dependencies upstream, explicit suppressions here are redundant. Keeping{}is valid and maintains the file's presence.As a minor optional cleanup, these files (and the equivalently-emptied
cron-serviceandwork-updaterones) could be deleted rather than left as empty stubs, sincebetter-npm-audittreats a missing.nsprcfile identically to an empty{}configuration—both result in no exceptions being loaded.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/query-cmr/.nsprc` at line 1, The .nsprc file currently contains only an empty object ("{}"); as an optional cleanup, remove the empty .nsprc from query-cmr (and the equivalent empty .nsprc files in cron-service and work-updater) since better-npm-audit treats a missing .nsprc the same as an empty one; delete those files, run the audit/CI to confirm no behavior changes, and commit the removals.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@services/cron-service/.nsprc`:
- Line 1: The .nsprc file should remain an empty object {} because the removed
advisory entries 1113429 and 1113371 were dev-dependency advisories handled by
the audit script; ensure the .nsprc in this change contains only {} (no prod
advisories), and confirm any dev advisory handling is delegated to the audit
script rather than re-adding those IDs.
In `@services/work-updater/.nsprc`:
- Line 1: No changes required—leave the .nsprc for the work-updater service
empty to match the same intentional suppression pattern used for the query-cmr
service; approve and keep the consistent empty config (no code edits to
functions or classes are needed).
---
Nitpick comments:
In `@services/query-cmr/.nsprc`:
- Line 1: The .nsprc file currently contains only an empty object ("{}"); as an
optional cleanup, remove the empty .nsprc from query-cmr (and the equivalent
empty .nsprc files in cron-service and work-updater) since better-npm-audit
treats a missing .nsprc the same as an empty one; delete those files, run the
audit/CI to confirm no behavior changes, and commit the removals.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
services/cron-service/.nsprcservices/query-cmr/.nsprcservices/service-runner/.nsprcservices/work-updater/.nsprc
🚧 Files skipped from review as they are similar to previous changes (1)
- services/service-runner/.nsprc
3 participants
Jira Issue ID
HARMONY-2268
Description
New plan for dependency monitoring
Local Test Steps
npm run better-audit
PR Acceptance Checklist
Summary by CodeRabbit