Conversation
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog. This PR will not appear in the changelog. 🤖 This preview updates automatically when you update the PR. |
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
|
|
||
| - name: Install dependencies | ||
| run: npm install |
There was a problem hiding this comment.
npm install executes untrusted code from forked PRs
The workflow runs npm install after checking out PR code, which includes the attacker-controlled package.json from forks. Malicious PRs can execute arbitrary code via npm lifecycle scripts (preinstall, postinstall). While GitHub secrets are not exposed to fork PRs, the workflow could be abused for cryptomining, supply chain attacks on cached dependencies, or other malicious activities within the runner environment.
Verification
Verified by reading the workflow file lines 1-33. The pull_request trigger (line 4) runs on fork PRs, actions/checkout (line 22) checks out the PR head (fork code), and npm install (line 25) executes with that untrusted package.json. Confirmed package.json exists and contains dependencies that would be installed. GitHub's fork PR security model prevents secret exposure but not code execution.
Identified by Warden code-review · T4E-NHS
There was a problem hiding this comment.
We see Warden already in action.
However ... I have no idea how to mitigate this vulnerability ... other than by reviewing changes and consciously manually approving externally triggered runs of our Workflows.
An equivalent vulnerability we're exposed to would be for an attacker just create a PR with
- malicious code directly (
run: ...) - malicious code indirectly
- compromised GitHub Actions: the reason we use commit SHAs
- malicious GitHub Actions: the reason we need to approve external contributions to run on our CI
Although I can't think of a non-review-based mitigation here,
I am in need of feedback.
Also ... I'll check-in with our Frontend/JavaScript team on Monday about best practices and such ... I am quite a noob when it comes to JavaScript based build systems.
My original intent to mitigate this vulnerability was to introduce a ./package.json that requests exact version numbers of npm tools ... without any indeterminism via "latest" or "floating" or "compatible with" (^) or the likes of.
Which is an approach that is consistent with our CI (commit SHAs) and .NET NuGet packages (exact versions, no floating versions) code.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## chore/consolidate-dotagents #5024 +/- ##
============================================================
Coverage 73.94% 73.94%
============================================================
Files 497 497
Lines 17974 17974
Branches 3517 3517
============================================================
Hits 13291 13291
Misses 3825 3825
Partials 858 858 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Backslash path separator inconsistent with all other entries
- Replaced the Windows-style
\.github\workflows\warden.ymlentry with.github/workflows/warden.ymlin both.slnxfiles to match the repository’s path separator convention.
- Replaced the Windows-style
Or push these changes by commenting:
@cursor push 73e211097c
Preview (73e211097c)
diff --git a/.generated.NoMobile.slnx b/.generated.NoMobile.slnx
--- a/.generated.NoMobile.slnx
+++ b/.generated.NoMobile.slnx
@@ -33,7 +33,7 @@
<File Path=".github/workflows/update-deps.yml" />
<File Path=".github/workflows/vulnerabilities.yml" />
<File Path=".github/workflows/playwright-blazor-wasm.yml" />
- <File Path=".github\workflows\warden.yml" />
+ <File Path=".github/workflows/warden.yml" />
</Folder>
<Folder Name="/benchmarks/">
<Project Path="benchmarks/Sentry.Benchmarks/Sentry.Benchmarks.csproj" />
diff --git a/Sentry.slnx b/Sentry.slnx
--- a/Sentry.slnx
+++ b/Sentry.slnx
@@ -33,7 +33,7 @@
<File Path=".github/workflows/update-deps.yml" />
<File Path=".github/workflows/vulnerabilities.yml" />
<File Path=".github/workflows/playwright-blazor-wasm.yml" />
- <File Path=".github\workflows\warden.yml" />
+ <File Path=".github/workflows/warden.yml" />
</Folder>
<Folder Name="/benchmarks/">
<Project Path="benchmarks/Sentry.Benchmarks/Sentry.Benchmarks.csproj" />This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
| <File Path=".github/workflows/update-deps.yml" /> | ||
| <File Path=".github/workflows/vulnerabilities.yml" /> | ||
| <File Path=".github/workflows/playwright-blazor-wasm.yml" /> | ||
| <File Path=".github\workflows\warden.yml" /> |
There was a problem hiding this comment.
Backslash path separator inconsistent with all other entries
Medium Severity
The newly added warden.yml file path uses Windows-style backslashes (.github\workflows\warden.yml) while every other File and Project path in both .slnx files consistently uses forward slashes. This inconsistency can cause cross-platform issues — particularly on Linux (where the CI runs on ubuntu-latest) — as some MSBuild tooling and IDE integrations may fail to resolve the path correctly.



Summary
Add Warden
Uses a subset of Agent Skills added and managed via
dotagents: see #4988See also https://warden.sentry.dev/
Remarks
This PR is based on #5026, but should be merged individually into
main.Skills:
./agents.toml./warden.tomlIt will be a continuous effort to finetune aspects like affected files to scan and severity level to report and break the pipeline.
This changeset serves as a kickoff to add the basic infrastructure needed for Warden.
To test Warden locally, run
We
mayshould improve the scripts in./package.jsonin the future ... I'm just getting started with JavaScript based build systems.Issues
Closes #4987