ci: add semgrep job in the ci workflow by anodar · Pull Request #3869 · near/mpc

anodar · 2026-07-20T20:17:41Z

closes: #594

claude · 2026-07-20T20:20:09Z

Pull request overview

Adds a Semgrep job to the CI workflow that runs the p/trailofbits, p/rust, and p/secrets rule sets on every non-docs PR, plus a .semgrepignore and a nosemgrep annotation for the intentional dummy TLS private key.

Changes:

New semgrep job in .github/workflows/ci.yml, pinned by digest, gated on the existing changes detector, running in the official semgrep/semgrep container.
New .semgrepignore excluding docs, localnet, and test crates.
nosemgrep comment on DUMMY_ISSUER_PRIVATE_KEY to silence the detected-private-key rule.

Reviewed changes

Per-file summary

File	Description
`.github/workflows/ci.yml`	Adds the `semgrep` job (digest-pinned container, minimal `contents: read` permission, `persist-credentials: false`, `--severity WARNING/ERROR --error`, excludes `panic-in-function-returning-result`).
`.semgrepignore`	Replaces the default ignore list; skips `docs/`, `*.md`, `localnet/`, `tests/`, `crates/e2e-tests/`, `crates/test-utils/`.
`crates/tls/src/constants.rs`	`nosemgrep` annotation on the intentional dummy PKCS#8 private key used only to satisfy the TLS handshake.

Findings

Non-blocking (nits, follow-ups, suggestions):

.github/workflows/ci.yml:280-281 — The stated rationale ("intentional panics on invariant violations are allowed by docs/engineering-standards.md") slightly overstates what the standard permits. docs/engineering-standards.md §Don't panic actually says most code should be panic-free, with only three narrow exceptions (top-level main, smart-contract entry points, and provably-dead branches). Wholesale-excluding the rule matches current practice but forfeits future signal — consider re-enabling later and using inline // nosemgrep: annotations at the ~small number of legitimate call sites, so new unjustified panics still trip CI. Even a lighter tweak — reword the comment to "...allowed in narrowly-defined cases; excluded because per-site annotation isn't in place yet" — would keep the intent honest.
.semgrepignore:11-15 — Excluding tests/ only skips integration-test directories. Per AGENTS.md, unit tests live inline in src/ under #[cfg(test)] and will still be scanned. If the intent (per the comment) is "panics and dummy data are expected in test code," you'll get false positives from those inline #[cfg(test)] blocks — worth verifying by running the job against the current tree before merge, and either narrowing the comment or adding a semgrep path-based exclude for #[cfg(test)] modules.
.semgrepignore:2 — Overriding semgrep's default ignore list means the built-in exclusions for things like vendor/, node_modules/, minified assets, etc. are no longer applied. For this Rust repo it's likely fine (target/ is gitignored), but if the repo later grows a vendored dep or JS asset directory it will be scanned. Consider re-adding the defaults you care about, or documenting the trade-off.
.github/workflows/ci.yml:262-291 — Findings surface only in the CI log. Uploading SARIF via semgrep scan --sarif --output results.sarif and github/codeql-action/upload-sarif would light up the Security → Code scanning tab and inline PR annotations. Not blocking, but a low-cost improvement for developer UX.
.github/workflows/ci.yml:283-291 — semgrep scan runs against the whole tree, not the diff. On PRs this can be reduced with --baseline-ref origin/${{ github.base_ref }} to only report newly-introduced findings, which also keeps the job fast as the codebase grows. Optional.

Overall the job is well-scoped: digest-pinned container, least-privilege contents: read, persist-credentials: false, sensible timeout, and consistent gating with the rest of CI. Good addition.

✅ Approved

anodar · 2026-07-21T09:48:42Z

+          --config p/trailofbits
+          --config p/rust
+          --config p/secrets
+          --exclude-rule trailofbits.rs.panic-in-function-returning-result.panic-in-function-returning-result


We have quite a bit of places where we do this, so decided to exclude it instead of adding annotation to over 20 places.

Should we maybe annotate those call sites properly as a follow up?

Created #3877

gilcu3

Thank you!

gilcu3 · 2026-07-21T11:49:01Z

+        uses: ./.github/actions/restore-nix-cache
+
+      - name: Run opengrep
+        run: nix shell .#opengrep -c bash scripts/run-opengrep.sh


nit: we could use cargo-make here, but this is also fine ofc

Leaving as is as it's marked nit.

Also, I assume you mean nix develop --command cargo make opengrep but that will need to build everything (in case of cache miss, if I understand it correctly) hereas this will be lot more isolated.

gilcu3 · 2026-07-21T11:50:13Z

+  # Upstream publishes standalone signed binaries per platform, no docker image.
+  assets = {
+    x86_64-linux = {
+      name = "opengrep_manylinux_x86";
+      hash = "sha256-msSuu0e6P3sNj8ZBrIdJy2wvJT9hYTGmfZYx4A1L6jM=";
+    };
+    aarch64-linux = {
+      name = "opengrep_manylinux_aarch64";
+      hash = "sha256-/UASQnLQBggqVZSxmuzuB7Ad1Qkz2K3XpP1cVX0r5fY=";
+    };
+    aarch64-darwin = {
+      name = "opengrep_osx_arm64";
+      hash = "sha256-NUP8q66dsq5byXSjt1QmNT8KPjaRgbIVfvJ/RoZ5lsg=";
+    };
+  };


are we fine with downloading binaries for this one? IIRC we were leaning towards the hard core build everything in nix stuff :)

I've looked a bit into it now, the ones that we build is different because it's rust tools published in crates.io and we can pin hash and build with locked versions. I don't see any non-rust tool that we build here.

On the other hand, while opengrep publishes their flake, they use opam package manager for OCalm (which is apparently language opengrep is written on). To nixify their opam deps they use opam-nix but somewhere down the dependency graph something's not pinned and it can't be built without --impure:

nix build "github:opengrep/opengrep/v1.25.0?submodules=1#opengrep" error: … while calling a functor (an attribute set with a '__functor' attribute) at «github:opengrep/opengrep/27b5fba1b315fbc63d0fd474d01f5d8235cc86e2»/flake.nix:413:11: 412| pyopengrep = with pythonPackages; 413| buildPythonApplication { | ^ 414| # thanks to @06kellyjac … while evaluating a branch condition at «github:nixos/nixpkgs/4bd9165a9165d7b5e33ae57f3eecbcb28fb231c9?narHash=sha256-l/iNYDZ4bGOAFQY2q8y5OAfBBtrDAaPuRQqWaFHVRXM%3D»/pkgs/development/interpreters/python/python-packages-base.nix:32:7: 31| in 32| if lib.isAttrs result then | ^ 33| result (stack trace truncated; use '--show-trace' to show the full, detailed trace) error: [opam-nix] a git dependency without an explicit sha1 is not supported in pure evaluation mode; try with --impure

Maybe we can work around this but not sure it's worth going down the rabbit hole.

yeah, ocaml makes it much harder here, thank you for taking a look

ci: add semgrep job in the ci workflow

7da9d0d

cleanup

7a920ef

anodar commented Jul 21, 2026

View reviewed changes

drop trailing whitespace

a2061a1

anodar mentioned this pull request Jul 21, 2026

Add annotations for semgrep exceptions instead of excluding the entire rule #3877

Open

anodar added 2 commits July 21, 2026 13:13

Switch to opengrep, add to check-all-fast

2701e79

Run opengrep in ci in nix

553c842

gilcu3 approved these changes Jul 21, 2026

View reviewed changes

haiyuechen-nearone approved these changes Jul 21, 2026

View reviewed changes

anodar added this pull request to the merge queue Jul 21, 2026

Merged via the queue into main with commit 6be87ac Jul 21, 2026
16 checks passed

anodar deleted the 594-add-semgrep-to-ci-checks branch July 21, 2026 12:50

Navigation Menu

Uh oh!

ci: add semgrep job in the ci workflow - #3869

ci: add semgrep job in the ci workflow#3869
anodar merged 5 commits into
mainfrom
594-add-semgrep-to-ci-checks

anodar commented Jul 20, 2026 •

edited

Loading

Uh oh!

claude Bot commented Jul 20, 2026

Uh oh!

anodar Jul 21, 2026

Uh oh!

haiyuechen-nearone Jul 21, 2026

Uh oh!

anodar Jul 21, 2026

Uh oh!

gilcu3 left a comment

Uh oh!

gilcu3 Jul 21, 2026

Uh oh!

anodar Jul 21, 2026

Uh oh!

gilcu3 Jul 21, 2026

Uh oh!

anodar Jul 21, 2026

Uh oh!

gilcu3 Jul 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

anodar commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot commented Jul 20, 2026

Pull request overview

Reviewed changes

Findings

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gilcu3 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

anodar commented Jul 20, 2026 •

edited

Loading