Skip to content

Read Semgrep config without following symlinks#20

Merged
haasonsaas merged 1 commit into
mainfrom
fix/semgrep-config-symlink-reads
Jul 18, 2026
Merged

Read Semgrep config without following symlinks#20
haasonsaas merged 1 commit into
mainfrom
fix/semgrep-config-symlink-reads

Conversation

@haasonsaas

Copy link
Copy Markdown
Collaborator

Summary

  • The adapter validated and hashed the operator config with os.Stat/os.ReadFile, both of which follow symlinks — a symlinked room.yml passed startup checks and each analyze-time read could resolve to different bytes than the ones validated.
  • Config reads (startup validation, configMatches, snapshot build) now go through a shared readRegularFD helper: O_NOFOLLOW + O_NONBLOCK open, regular-file + 64 MiB fstat check, and a post-read fstat comparison — the same discipline readRegularBeneath already applied to diff targets.
  • A symlinked or swapped config now fails closed as config_digest_mismatch (analyze) or snapshot_invalid, and startup rejects non-regular configs including FIFOs instead of blocking on them.

Test plan

  • New: symlinked config rejected at startup; config swapped to a same-content symlink mid-run fails closed with config_digest_mismatch (both verified to fail against the pre-fix code)
  • go test ./... and go vet clean
  • ROOM_SEMGREP_CORE=… go test -tags=semgrep_integration ./cmd/room-semgrep — real semgrep-core 1.139.0 suite passes
  • Adversarial review of the diff: approve; its one hardening note (FIFO open could block) addressed with O_NONBLOCK

The adapter validated and hashed the operator config with os.Stat and
os.ReadFile, which follow symlinks. A symlinked config passed startup
validation, and each analyze-time read could resolve to different
bytes than the ones validated. Read the config with O_NOFOLLOW and
verify the descriptor stays a stable regular file instead, so a
symlinked or swapped config fails closed as config_digest_mismatch
or snapshot_invalid.
@haasonsaas
haasonsaas merged commit 699272b into main Jul 18, 2026
4 checks passed
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.

1 participant