Skip to content

perf(ci): gate local semgrep jobs on changed paths - #74230

Merged
rnegron merged 4 commits into
masterfrom
posthog-code/gate-cross-cutting-semgrep-scans
Jul 28, 2026
Merged

perf(ci): gate local semgrep jobs on changed paths#74230
rnegron merged 4 commits into
masterfrom
posthog-code/gate-cross-cutting-semgrep-scans

Conversation

@rnegron

@rnegron rnegron commented Jul 28, 2026

Copy link
Copy Markdown
Member

Problem

The Semgrep devex and rule-test jobs run on PRs that cannot affect their results.

Changes

  • Gate semgrep-devex on its scan targets and local rules.
  • Gate semgrep-test-rules on .semgrep/**.
  • Keep semgrep-general unconditional because it loads live security packs.
  • Run every path-scoped scan when a PR exceeds the GitHub 3,000-file API limit.
  • Keep all scans unconditional on master pushes.

How did you test this code?

  • bin/hogli lint:workflows
  • actionlint -config-file .github/actionlint.yaml .github/workflows/ci-security.yaml
  • git diff --check

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

Docs update

No user-facing behavior changes.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Claude measured the original proposal. Codex narrowed it to locally defined checks and added the oversized-PR safeguard. Skills: /code-review, /authoring-ci-workflows, /writing-code-comments, /writing-user-facing-copy, and /wt.

semgrep-general, semgrep-devex and semgrep-test-rules run on every PR.
Measured against the file lists of all 136 PRs merged in a trailing
24-hour window, they have nothing in scope on 89%, 13% and 99% of them
respectively, which is roughly 1,600 job-minutes a day.

Each new filter mirrors what its job actually reads:

- general: '**' minus the same twelve trees the job passes as --exclude
  flags. Deliberately expressed as excludes rather than positive
  includes so a new top-level directory fails open and still triggers
  the scan.
- devex: the directory list the job hands to semgrep.
- test-rules: '.semgrep/**' only, since `semgrep --test .semgrep/`
  exercises the rules against their own committed fixtures and cannot
  be affected by anything outside that tree.

Coverage this deliberately preserves:

- master pushes are untouched. The filter step is gated on
  `github.event_name != 'push'`, so every scan still runs
  unconditionally on master, which is what covers repo-wide drift and
  newly published rules against unchanged code.
- .github/** always triggers general. That is the surface CODEOWNERS
  singles out as untrusted, and the job loads p/github-actions to scan
  it.
- editing this workflow triggers every scan, the mitigation from #68640.
- outputs keep their `|| 'true'` default, so a skipped or failed filter
  step fails open.

Raising this as a proposal rather than folding it into a CI cost pass:
#68640 left these three unconditional on purpose, and #70907 hit the
same cost problem and chose `--jobs 4` over path filtering, explicitly
to avoid weakening the check. Narrowing them is team-security's call.

Generated-By: PostHog Code
Task-Id: 7bad9f9c-04b8-40b0-b44c-e967a3f8c731
rnegron and others added 2 commits July 28, 2026 10:14
The comments added earlier in this PR were verbose, and the truncation
NOTE contradicted the pre-existing lines it was appended to: those said
the `|| 'true'` default makes scans run unconditionally, then the new
text immediately qualified that it only covers a missing output. Merged
into one statement, 9 lines to 6, with every fact intact.

The `general` comment now also records why the filter is shaped as '**'
plus negations rather than positive includes: a new top-level directory
then fails open and still triggers the scan. That rationale was in the
PR description but had never made it into the file, which is the one
place a future editor would look before restructuring the filter. The
one-for-one warning about semgrep-general's --exclude flags is
preserved, including which direction of drift is the dangerous one.

Comments only. Verified by parsing the workflow, re-parsing the
`filters:` literal block (its comments are part of the string the
action receives), and deep-comparing against the previous commit:
byte-identical. Separately re-asserted that the `general` negations are
still one-for-one with the job's --exclude flags.

Generated-By: PostHog Code
Task-Id: 7bad9f9c-04b8-40b0-b44c-e967a3f8c731
@rnegron rnegron changed the title perf(ci): gate the three cross-cutting semgrep scans on changed paths perf(ci): gate local semgrep jobs on changed paths Jul 28, 2026
@rnegron
rnegron marked this pull request as ready for review July 28, 2026 15:11
@rnegron
rnegron requested a review from a team as a code owner July 28, 2026 15:11
@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team July 28, 2026 15:12
@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Reviews (1): Last reviewed commit: "fix(ci): harden semgrep path filters" | Re-trigger Greptile

@rnegron
rnegron enabled auto-merge (squash) July 28, 2026 18:00
@rnegron
rnegron merged commit 3e92e9e into master Jul 28, 2026
173 of 174 checks passed
@rnegron
rnegron deleted the posthog-code/gate-cross-cutting-semgrep-scans branch July 28, 2026 18:15
@deployment-status-posthog

deployment-status-posthog Bot commented Jul 28, 2026

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-07-28 18:56 UTC Run
prod-us ✅ Deployed 2026-07-28 19:13 UTC Run
prod-eu ✅ Deployed 2026-07-28 19:15 UTC Run

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.

2 participants