Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 3 additions & 12 deletions .github/workflows/codeql.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,9 @@ on:
- 'adrs/**'
- 'plans/**'
- 'coverage/**'
pull_request:
branches: [main]
paths-ignore:
- '**/*.md'
- 'docs/**'
- 'LICENSE'
- '.gitignore'
- 'PLAN*.md'
- 'REPORT*.md'
- 'adrs/**'
- 'plans/**'
- 'coverage/**'
# No pull_request trigger: CodeQL output goes to the Security tab only
# (never inline PR annotations), so running it on every PR adds ~25 min of
# CI cost with no faster feedback. Weekly schedule + push-to-main is enough.
schedule:
- cron: '0 9 * * 1'
workflow_dispatch:
Expand Down
10 changes: 3 additions & 7 deletions .github/workflows/semgrep.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,9 @@ on:
- 'sql/**'
- '.semgrep/**'
- '.github/workflows/semgrep.yml'
pull_request:
branches: [main]
paths:
- 'src/**'
- 'sql/**'
- '.semgrep/**'
- '.github/workflows/semgrep.yml'
# No pull_request trigger: Semgrep rules are advisory-only and won't block
# a merge, so running on every PR provides no gate value. Weekly schedule
# + push-to-main covers main-branch drift.
schedule:
- cron: '0 10 * * 1'
workflow_dispatch:
Expand Down
55 changes: 55 additions & 0 deletions .github/workflows/unsafe-inventory.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# =============================================================================
# Unsafe Inventory Workflow — surface unsafe block growth in PRs.
#
# Counts `unsafe {` blocks per .rs file and compares against the committed
# baseline in .unsafe-baseline. Fails if any file exceeds its baseline count.
#
# This is an advisory signal: the workflow posts a per-file table as a step
# summary so reviewers can see unsafe growth at a glance. It is not a required
# check for PR merge (add it to branch protection once the team adopts the
# policy).
#
# To update the baseline after a justified unsafe addition:
# scripts/unsafe_inventory.sh --update > .unsafe-baseline
# git add .unsafe-baseline && git commit -m "chore: update unsafe baseline"
# =============================================================================
name: Unsafe inventory

on:
push:
branches: [main]
paths:
- 'src/**'
- '.unsafe-baseline'
- 'scripts/unsafe_inventory.sh'
- '.github/workflows/unsafe-inventory.yml'
pull_request:
branches: [main]
paths:
- 'src/**'
- '.unsafe-baseline'
- 'scripts/unsafe_inventory.sh'
- '.github/workflows/unsafe-inventory.yml'
schedule:
- cron: '15 9 * * 1'
workflow_dispatch:

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

permissions:
contents: read

jobs:
unsafe-inventory:
name: Unsafe block inventory
runs-on: ubuntu-latest
timeout-minutes: 5
steps:
- uses: actions/checkout@v4

- name: Run unsafe inventory
run: |
chmod +x scripts/unsafe_inventory.sh
scripts/unsafe_inventory.sh
73 changes: 70 additions & 3 deletions .semgrep/pg_trickle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,78 @@ rules:
- generic
severity: INFO
message: >-
SECURITY DEFINER requires explicit review. Pin search_path and verify the
function body cannot be influenced by attacker-controlled identifiers or
SQL fragments.
SECURITY DEFINER requires explicit review. Always pin the search path:
SET search_path = pgtrickle, pg_catalog, pg_temp; — otherwise an
unprivileged user can pre-create objects in a schema earlier on the path
and intercept calls made by the privileged function.
paths:
include:
- src/**
- sql/**
exclude:
- "**/*.md"
- plans/**
- docs/**
pattern-regex: SECURITY\s+DEFINER

# ── Phase 2: Extension-specific privilege-context rules ─────────────────────

- id: sql.row-security.disabled
languages:
- generic
severity: WARNING
message: >-
SET LOCAL row_security = off disables PostgreSQL row-level security for
the current transaction. In a SECURITY DEFINER extension context this can
allow privileged code to bypass per-user RLS policies. Verify the context
is appropriate and this path is not reachable from an unprivileged caller
via the extension API.
paths:
include:
- src/**
- sql/**
exclude:
- "**/*.md"
- plans/**
- docs/**
pattern-regex: 'SET\s+LOCAL\s+row_security\s*=\s*off'

- id: sql.set-role.present
languages:
- generic
severity: WARNING
message: >-
SET ROLE or RESET ROLE modifies the current session role context. In a
SECURITY DEFINER extension context this can widen or narrow privileges
unexpectedly. Verify the role transition is intentional, bounded by a
matching RESET ROLE or transaction boundary, and that the target role is
the one you intend.
paths:
include:
- src/**
- sql/**
exclude:
- "**/*.md"
- plans/**
- docs/**
pattern-regex: '(SET\s+ROLE\b|RESET\s+ROLE\b)'

- id: rust.panic-in-sql-path
languages:
- rust
severity: WARNING
message: >-
.unwrap(), .expect(), or panic!() will crash the PostgreSQL backend
process if reached from a SQL-callable function. Propagate errors via
Result<T, PgTrickleError> and convert at the API boundary with
pgrx::error!() or ereport!() instead. If this code path is genuinely
unreachable, use pgrx::error!("unreachable: ...") so PostgreSQL can
clean up the session gracefully rather than aborting the backend.
patterns:
- pattern-either:
- pattern: $X.unwrap()
- pattern: $X.expect($MSG)
- pattern: panic!(...)
paths:
include:
- src/**
14 changes: 14 additions & 0 deletions .unsafe-baseline
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# pg_trickle unsafe { block baseline — generated 2026-03-10
# Tracks `grep -c "unsafe {"` counts per source file.
# Files with 0 unsafe blocks are omitted.
#
# To update after a justified increase:
# scripts/unsafe_inventory.sh --update
#
# Files and their unsafe block counts:
src/api.rs 10
src/dvm/parser.rs 1286
src/lib.rs 1
src/scheduler.rs 5
src/shmem.rs 3
src/wal_decoder.rs 4
41 changes: 41 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,47 @@ run without `#[ignore]`:

### Added

- **SAST Phase 2 + 3 — Privilege-context rules and unsafe inventory**
(`sast-review-1` branch):

- **Phase 2 — Extension-specific Semgrep rules** (`.semgrep/pg_trickle.yml`):
- `sql.row-security.disabled` — flags `SET LOCAL row_security = off` in
`src/**` and `sql/**`. Alerts reviewers when the extension disables RLS,
which can bypass per-user policies in a `SECURITY DEFINER` context.
- `sql.set-role.present` — flags `SET ROLE` / `RESET ROLE` patterns. Role
transitions in extension code can widen or narrow privileges unexpectedly.
- `rust.panic-in-sql-path` — flags `.unwrap()`, `.expect(…)`, and
`panic!(…)` in `src/**`. These crash the PostgreSQL backend process if
reached from a SQL-callable function. Hits ~37 existing callsites
(mostly `expect("unreachable after error!()")` idiom in `monitor.rs` and
`api.rs`); all advisory, documented as Phase 2 triage backlog.
- Updated `sql.security-definer.present` message to explicitly require
`SET search_path = schema, pg_catalog, pg_temp` alongside every
`SECURITY DEFINER` declaration. Prevents search-path hijacking.
- All three rules are advisory (WARNING/INFO, SARIF upload only, no CI gate).
- No current hits in source — rules are proactive, will fire when these
patterns are added during the RLS and privilege-hardening work in v0.3.0.

- **Phase 3 — Unsafe block inventory** (`scripts/unsafe_inventory.sh`):
- New `scripts/unsafe_inventory.sh` counts `unsafe {` blocks per `.rs` file
and compares against the committed baseline in `.unsafe-baseline`.
- `.unsafe-baseline` records the 2026-03-10 snapshot: 1309 total `unsafe {`
blocks across 6 files (`api.rs`:10, `dvm/parser.rs`:1286, `lib.rs`:1,
`scheduler.rs`:5, `shmem.rs`:3, `wal_decoder.rs`:4).
- New `.github/workflows/unsafe-inventory.yml` runs the script on PRs that
touch `src/**`. Posts a per-file delta table to `GITHUB_STEP_SUMMARY`.
Fails if any file exceeds its baseline count.
- New `just unsafe-inventory` recipe for local use.
- `clippy::undocumented_unsafe_blocks` deferred: `src/dvm/parser.rs` has
1286 mechanical pgrx FFI blocks needing a `#![allow]` first.

- **CI workflow trigger tightening** (`.github/workflows/codeql.yml` and
`semgrep.yml`): removed `pull_request` trigger from both CodeQL and
Semgrep. CodeQL output lands in the Security tab only (no inline PR
annotations), so running it on every PR added ~25 min cost with no
faster feedback. Semgrep is advisory-only and never blocks merge.
Both workflows still run on push-to-main and weekly Monday schedule.

- **SAST / static security analysis baseline** — new security tooling on the
`codeql-workflow` branch, now merged:
- **GitHub CodeQL** (`.github/workflows/codeql.yml`) — runs 16 Rust security
Expand Down
28 changes: 28 additions & 0 deletions ROADMAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,34 @@ DIFFERENTIAL baseline. No `src/` changes — test-only additions.

---

## SAST Program (Phases 1–3)

**Branch:** `sast-review-1`
**Status:** Phases 1–3 complete.
**Ref:** [plans/testing/PLAN_SAST.md](plans/testing/PLAN_SAST.md)

Incremental SAST (Static Application Security Testing) rollout. Phases 0–1
(CodeQL, cargo deny, initial Semgrep, first alert triage) merged via the
`codeql-workflow` branch and were detailed in the v0.2.3 changelog. This
branch implements Phases 2–3 and CI workflow tuning.

| Item | Description | Status |
|------|-------------|--------|
| S1 | Remove `pull_request` trigger from CodeQL + Semgrep workflows | ✅ Done |
| S2 | `sql.row-security.disabled` Semgrep rule | ✅ Done |
| S3 | `sql.set-role.present` Semgrep rule | ✅ Done |
| S4 | Updated `sql.security-definer.present` message: explicit `SET search_path` guidance | ✅ Done |
| S5 | `scripts/unsafe_inventory.sh` — per-file `unsafe {` counter | ✅ Done |
| S6 | `.unsafe-baseline` — committed baseline (1309 blocks across 6 files) | ✅ Done |
| S7 | `.github/workflows/unsafe-inventory.yml` — advisory CI workflow | ✅ Done |
| S8 | `just unsafe-inventory` recipe | ✅ Done |

**Next (Phase 4):** promote high-confidence Semgrep rules from advisory to
blocking after the privilege-context rules have been validated against v0.3.0
RLS/SECURITY DEFINER work.

---

## v0.3.0 — Parallel Refresh, Security & Partitioning

**Goal:** Deliver true parallel refresh first, then harden security (RLS) and
Expand Down
5 changes: 5 additions & 0 deletions justfile
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ clippy:
[group: "lint"]
lint: fmt-check clippy

# Audit unsafe block counts against the committed baseline (.unsafe-baseline)
[group: "lint"]
unsafe-inventory:
bash scripts/unsafe_inventory.sh

# ── Tests ─────────────────────────────────────────────────────────────────

# Run pure-Rust unit tests (no Docker needed)
Expand Down
Loading