ci(security): add Muninn security scan with fail-on info#7251
ci(security): add Muninn security scan with fail-on info#7251sg0nzalez wants to merge 13 commits into
Conversation
Introduce the Muninn GitHub Action as a merge-gating security check, harden checkout credential persistence and action pins, bump vulnerable transitive dependencies, and document justified suppressions for intentional PRT trust models and test fixtures. Signed-off-by: Santiago Gonzalez <santiago.gonzalez33@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Muninn security scanning configuration and workflow coverage, hardens GitHub Actions credentials and permissions, improves workflow value handling and Node.js installation, pins selected actions, and updates dependency and Dependabot settings. ChangesCI security hardening
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant Checkout
participant Muninn
participant UploadSARIF
GitHubActions->>Checkout: Checkout repository and trusted base policy
Checkout->>Muninn: Provide resolved Muninn configuration
Muninn->>GitHubActions: Produce comments and muninn.sarif
GitHubActions->>UploadSARIF: Upload SARIF when present
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
wscurran
added
area: ci
|
✨ Thanks for the fix. This adds a merge-gating Muninn security scan with SARIF upload and PR comments, plus CI hardening. Ready for maintainer review. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
muninn.yml (1)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
fail-on: infois specified in two places — pick one source of truth.The setting appears in both
muninn.yml(config file) and.github/workflows/muninn.yaml(workflow input). If the action uses the workflow input as the override, the config value is dead; if someone changes one but not the other, behavior diverges. Choose one location and remove the other.
muninn.yml#L5-L5: Removefail-on: infoif the workflow input is the source of truth, or keep it and remove the workflow input..github/workflows/muninn.yaml#L41-L41: Removefail-on: infofrom the workflow input if the config file is the source of truth, or keep it and remove the config file entry.🤖 Prompt for 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. In `@muninn.yml` at line 5, Choose a single source of truth for the fail-on setting and remove the duplicate declaration: update muninn.yml lines 5-5 and .github/workflows/muninn.yaml lines 41-41 so only one retains fail-on: info, with the other explicitly removed; preserve the resulting action behavior.
🤖 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 `@package.json`:
- Around line 94-96: Regenerate package-lock.json after adding the root axios
override in package.json, ensuring the lockfile records the updated axios
resolution policy and dependency tree. Commit the regenerated lockfile without
changing the override itself.
---
Nitpick comments:
In `@muninn.yml`:
- Line 5: Choose a single source of truth for the fail-on setting and remove the
duplicate declaration: update muninn.yml lines 5-5 and
.github/workflows/muninn.yaml lines 41-41 so only one retains fail-on: info,
with the other explicitly removed; preserve the resulting action behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a524ebf1-c53a-460d-8fa0-e15d4c82b48e
⛔ Files ignored due to path filters (2)
nemoclaw/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
.github/actionlint.yaml.github/workflows/base-image.yaml.github/workflows/code-scanning.yaml.github/workflows/commit-lint.yaml.github/workflows/docker-pin-check.yaml.github/workflows/docs-cli-parity-pr.yaml.github/workflows/docs-links-pr.yaml.github/workflows/e2e-branch-validation.yaml.github/workflows/macos-e2e.yaml.github/workflows/muninn.yaml.github/workflows/pr-self-hosted.yaml.github/workflows/wsl-e2e.yaml.gitignoremuninn.ymlpackage.json
PR Review Advisor — InformationalAdvisor assessment: Informational / medium confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 1 warning · 0 suggestionsWarningsWarnings do not block.
|
Keep the lockfile root dependencies aligned with package.json after moving axios to an overrides-only pin via @j178/prek. Signed-off-by: Santiago Gonzalez <santiago.gonzalez33@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Drop the redundant workflow fail-on input so severity gating is owned by muninn.yml alone and cannot drift from the workflow. Signed-off-by: Santiago Gonzalez <santiago.gonzalez33@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Load muninn.yml from the PR base SHA so PRs cannot weaken suppressions, fix template-injection / permissions / Node install checksum issues, and drop suppressions that are no longer needed for fail-on: info. Signed-off-by: Santiago Gonzalez <santiago.gonzalez33@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Gate severity lives in muninn.yaml; root muninn.yml is scanners and suppressions only. Base-SHA config checkout still protects the allowlist. Signed-off-by: Santiago Gonzalez <santiago.gonzalez33@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
muninn.yml (1)
51-64: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winScope these suppressions to exact findings.
muninn.yml#L51-L145These selectors are broad and permanent:
tool/rule-idcan hide whole classes of findings,idmatches by path substring, and emptyexpiresmakes each entry non-expiring. Replace them with finding fingerprints so only the reviewed false positives stay suppressed.🤖 Prompt for 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. In `@muninn.yml` around lines 51 - 64, Replace the broad, non-expiring suppressions throughout muninn.yml ranges 51-64, 67-72, 75-100, 103-128, and 130-143 with exact finding fingerprint selectors. Preserve suppressions only for the reviewed false positives, remove broad tool/rule-id or path-substring matching, and assign an explicit expiration to each retained suppression.
🧹 Nitpick comments (1)
.github/dependabot.yml (1)
10-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a regression assertion for the new cooldown.
The
cooldown.default-dayssetting is valid for GitHub Actions version updates, but the existingtest/code-scanning-workflow.test.tscontract does not verify it. Assert that this update entry retainsdefault-days: 7so future edits cannot silently remove or change the seven-day delay. GitHub notes that cooldown does not affect security updates. (docs.github.com)🤖 Prompt for 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. In @.github/dependabot.yml around lines 10 - 11, Add a regression assertion in the existing test/code-scanning-workflow.test.ts contract for the Dependabot update configuration, verifying that cooldown.default-days remains exactly 7 for GitHub Actions version updates; preserve the existing security-update behavior and surrounding assertions.Source: Coding guidelines
🤖 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 @.github/workflows/muninn.yaml:
- Around line 58-71: Update the config path emitted by the trusted-policy branch
in the workflow’s Muninn setup to be workspace-relative and accessible inside
Docker, rather than using RUNNER_TEMP or a host absolute path. Preserve the
existing copy and cleanup behavior, and ensure the config output points to the
copied trusted muninn.yml relative to the workspace so Muninn loads the intended
policy.
---
Outside diff comments:
In `@muninn.yml`:
- Around line 51-64: Replace the broad, non-expiring suppressions throughout
muninn.yml ranges 51-64, 67-72, 75-100, 103-128, and 130-143 with exact finding
fingerprint selectors. Preserve suppressions only for the reviewed false
positives, remove broad tool/rule-id or path-substring matching, and assign an
explicit expiration to each retained suppression.
---
Nitpick comments:
In @.github/dependabot.yml:
- Around line 10-11: Add a regression assertion in the existing
test/code-scanning-workflow.test.ts contract for the Dependabot update
configuration, verifying that cooldown.default-days remains exactly 7 for GitHub
Actions version updates; preserve the existing security-update behavior and
surrounding assertions.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 31d72322-9cb5-4858-ad31-0f4d28f6972d
📒 Files selected for processing (15)
.github/dependabot.yml.github/workflows/candidate-compatibility.yaml.github/workflows/code-scanning.yaml.github/workflows/docs-links-pr.yaml.github/workflows/e2e-branch-validation.yaml.github/workflows/e2e.yaml.github/workflows/macos-e2e.yaml.github/workflows/muninn.yaml.github/workflows/platform-vitest-main.yaml.github/workflows/pr-e2e-gate.yaml.github/workflows/pr-review-advisor.yaml.github/workflows/regression-e2e.yaml.github/workflows/release-latest-tag.yaml.github/workflows/wsl-e2e.yamlmuninn.yml
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/docs-links-pr.yaml
- .github/workflows/e2e-branch-validation.yaml
- .github/workflows/code-scanning.yaml
Use a workspace-relative trusted muninn.yml for the Docker action, replace broad suppressions with fingerprints (path ids only where Semgrep lacks stable fingerprints), and assert Dependabot Actions cooldown stays at 7 days. Signed-off-by: Santiago Gonzalez <santiago.gonzalez33@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
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 `@muninn.yml`:
- Around line 230-244: Replace the path-scoped suppressions for
agents/hermes/Dockerfile, src/lib/onboard/created-sandbox-failure.test.ts, and
src/lib/resources-cmd.ts with finding-specific Semgrep fingerprints once stable
fingerprints are available, preserving only the intended current findings and
their expiration metadata.
- Around line 44-48: Update the Poutine suppression in muninn.yml for
github_action_from_unverified_creator_used to use the specific finding
fingerprint key, matching the structure of the other fingerprint-based
suppressions, instead of suppressing the entire rule globally. Preserve the
existing reason and expiration.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ded98892-465f-40d4-b554-da783165adb9
📒 Files selected for processing (4)
.github/workflows/muninn.yaml.gitignoremuninn.ymltest/code-scanning-workflow.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- .gitignore
- .github/workflows/muninn.yaml
Replace the global github_action_from_unverified_creator_used rule suppression with per-finding fingerprints so new Actions uses still fail the gate until explicitly reviewed. Signed-off-by: Santiago Gonzalez <santiago.gonzalez33@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
dfernandez365-rgb
left a comment
dfernandez365-rgb
left a comment
There was a problem hiding this comment.
Independent dependency review of exact head 6aee1dceb1d66c133608fbbdf8a5ad89490ed261: FAIL. The new scanner reports no unsuppressed findings, but the PR's own production dependency graph still contains a known high-severity advisory.
- HIGH — The production graph remains vulnerable. On the exact PR head:
npm audit --omit=dev --json
high: 1
brace-expansion 2.0.0 - 2.1.1
node_modules/filelist/node_modules/brace-expansion: 2.1.0
GHSA-3jxr-9vmj-r5cp
fixAvailable: true
exit: 1
The root brace-expansion was refreshed to 5.0.7, but the production filelist copy remains 2.1.0. Refresh that nested resolution to the maintained 2.x fix (2.1.2) and require npm audit --omit=dev to return zero before this security gate is described as passing.
- MEDIUM — The direct
js-yamldeclaration retains the vulnerable floor.package-lock.jsonresolves 4.3.0, butpackage.jsonstill declares^4.1.1. This weakens the dependency contract and permits regeneration or downstream no-lock installation from the vulnerable range implicated by GHSA-52cp-r559-cp3m/GHSA-h67p-54hq-rp68. Keep the v4-compatible line and raise the declared floor to^4.3.0; no v5 migration is required.
A narrow two-file resolution (package.json plus package-lock.json) using js-yaml 4.3.0, root brace-expansion 5.0.7, and nested brace-expansion 2.1.2 produced zero production findings and passed 96 manifest/config tests, compatibility probes, CLI build, both typechecks, Biome, file-scoped hooks, and git diff --check. The shrinkwrapped 5.0.6 copy inside the development-only coding-agent package is separate debt and must not be “fixed” by a dishonest outer-lock edit.
This PR already owns the overlapping dependency refresh. Please repair it here rather than opening a duplicate dependency PR. The fact that Muninn is green while npm audit --omit=dev is high also deserves an explicit checked-in gate or documented coverage boundary; otherwise “unified dependency findings” overstates what this workflow proves.
3 participants
Summary
Adds Muninn as a merge-gating security scan (
fail-on: info) so PRs andmainget unified CI/CD, secret, SAST, dependency, and IaC findings with SARIF upload and PR comments. Also hardens checkout credential persistence / action pins and bumps vulnerable transitive dependencies so the new gate can pass.Changes
.github/workflows/muninn.yaml(pinnedskaldlab/muninn@v0.3.5, SARIF + PR comment)muninn.ymlwithfail-on: infoand justified suppressions for intentional PRT trust models, hash-pinned Actions creator noise, test-fixture fake secrets, and sandbox Docker/Checkov policy exceptions.github/actionlint.yamlfor NVIDIA self-hosted runner labelspersist-credentials: falseon flagged checkout steps; pinactions/setup-nodewhere it was unpinnedaxiosto^1.18.1and refresh nested lockfile advisoriesmuninn.json/muninn.sarifType of Change
Quality Gates
muninn --fail-on info(exit 0)muninn.ymlsuppressions and PRT-related justificationsVerification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailable/tmp/muninn --target . --config muninn.yml --fail-on info→ exit 0 (0 unsuppressed findings)npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: not applicable (CI config only)npm run docsbuilds without warnings (doc changes only)Test plan
Security / Muninnruns on this PR and uploads SARIFfail-on: info)muninn.ymlsuppressions (especially PRT / untrusted-checkout justifications)persist-credentials: falsewhere expectedSigned-off-by: Santiago Gonzalez santiago.gonzalez33@gmail.com
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Security
Chores