feat(procore): Track A batch 2 — audit retention, JSONL gate, SBOM/vuln scan (T5,T6,T9) - #25
Conversation
Add migration 008: private.procore_audit, an append-only (UPDATE blocked, DELETE owner-only) audit trail storing minimal metadata only — hashes, rule-pack versions, status, finding counts, write-back metadata, and an append-only human-override trail. Never stores raw SWMS text. SECURITY DEFINER RPCs in public (service_role EXECUTE only): record_procore_audit, purge_procore_audit (retention), and delete_procore_audit_for_company (customer deletion request). Reuses the 004 immutability trigger pattern; table lives in the private schema with RLS. Migration not applied (verify project first). A contract test guards the invariants since the migration can't run in CI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Local JSONL writes under src/data made the 'minimal audit metadata retained' data-handling statement false and aren't durable on an ephemeral container. Gate log_payload, log_review (webhook_handler) and store_artifact (review_store) behind PROCORE_LOCAL_JSONL_ENABLED (default false). When disabled they no-op; the durable record is the Supabase audit trail (T6). Local JSONL stays available for dev/tests via the flag. Tests: TestLocalJsonlGate covers disabled -> no write, enabled -> write, for payload/review/artifact; test_log_creates_file sets the flag. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CI ran flake8 + pytest only, with no SBOM or vulnerability scanning. Add: - Generate SBOM (CycloneDX) via pip-audit, uploaded as an artifact (always). - Vulnerability scan: pip-audit -r requirements.txt, strict — fails CI on known-vuln pinned deps. Unavoidable no-fix advisories must be documented with --ignore-vuln and a reason, never blanket-ignored. - pip-audit[cyclonedx] added to requirements-dev.txt. Note: could not run pip-audit locally (pip install restricted), so CI is the first authoritative scan. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up to T5: TestReviewStore exercises the local store/find roundtrip, which is gated off by default now. Enable PROCORE_LOCAL_JSONL_ENABLED for that class so the roundtrip assertions still run. Caught by the pre-push full-suite gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37f68747c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| create or replace function public.record_procore_audit(p_record jsonb) | ||
| returns bigint | ||
| language plpgsql | ||
| security definer |
There was a problem hiding this comment.
The certification plan's Supabase RPC hardening section says T6 must not create security definer functions in exposed/public schemas (docs/procore/STAGE3_CERTIFICATION_PLAN_V2.md:281-282). This creates public.record_procore_audit as security definer (and repeats the pattern for purge/delete below), so the audit migration fails the hardening requirement it is meant to satisfy even though EXECUTE is restricted to service_role; put the definer implementation in a private/internal schema instead.
Useful? React with 👍 / 👎.
| if not _local_jsonl_enabled(): | ||
| log.debug("Local JSONL disabled; skipping artifact store write") | ||
| return |
There was a problem hiding this comment.
With the default environment this early return prevents store_artifact() from persisting the full review artifact, but _process_procore_v1_webhook still relies on this same store for resubmission comparison via find_previous_artifact() (api/main.py:1412-1429). Since the new Supabase audit migration stores only metadata and there is no Supabase reader/writer for comparison artifacts, normal deployments without PROCORE_LOCAL_JSONL_ENABLED=true will never retain reviews for later resubmission comparisons.
Useful? React with 👍 / 👎.
The new pip-audit gate flagged 19 known vulnerabilities (all with fixes): - cryptography 46.0.7 -> 48.0.1 (GHSA-537c-gmf6-5ccf) - pypdf 6.9.2 -> 6.13.0 (12 CVEs) - python-dotenv 1.1.0 -> 1.2.2 (CVE-2026-28684) - python-multipart 0.0.22 -> 0.0.31 (4 CVEs) All have fix versions, so bump rather than ignore. Bumps validated by CI (could not pip-install locally). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Summary
Track A batch 2 — retention/audit + supply-chain hardening (P1 certification items). Follows PR #24 (T1–T4,T10).
Tickets
feat(procore): append-only audit table with retention + deletion— migration 008:private.procore_audit(append-only, RLS), metadata-only (hashes/versions/counts, never raw text), SECURITY DEFINER RPCs (service-role only) for record / retention-purge / customer-deletion. Contract test guards the invariants (migration not applied in CI).feat(procore): gate local JSONL persistence off by default—log_payload/log_review/store_artifactgated behindPROCORE_LOCAL_JSONL_ENABLED(default false), making the "minimal audit metadata retained" statement true. Durable record is the Supabase audit trail.ci(security): add SBOM + pip-audit vulnerability gate— CycloneDX SBOM artifact + strictpip-auditon every push/PR.Heads-up on CI
This PR introduces the pip-audit vulnerability gate. It could not be run locally (pip install restricted), so this CI run is the first authoritative scan. If the pinned deps carry known advisories, the
Vulnerability scanstep will go red — that's the gate working. We then triage: bump the dep if a fix exists, or add a documented--ignore-vuln <id>with a reason (no blanket-ignore).Deployment prerequisites (not done here)
008_procore_audit.sqlto the Supabase project; the audit RPCs requireservice_role.Tests
pytest tests/test_procore_webhook.py→ 71 passed; audit-migration contract test → 6 passed; full suite green (pre-push gate). flake8 clean.🤖 Generated with Claude Code