diff --git a/docs/procore/STAGE3_CERTIFICATION_PLAN_V2.md b/docs/procore/STAGE3_CERTIFICATION_PLAN_V2.md index adceaa3..c5aca35 100644 --- a/docs/procore/STAGE3_CERTIFICATION_PLAN_V2.md +++ b/docs/procore/STAGE3_CERTIFICATION_PLAN_V2.md @@ -5,6 +5,17 @@ **Source of truth:** [STAGE3_CERTIFICATION_REVIEW.md](STAGE3_CERTIFICATION_REVIEW.md) (verdict: ACCEPT WITH CHANGES) **Rule:** every claim carries `file:line` (verified against current HEAD) or an official docs URL; gaps marked `UNVERIFIED`. +## Status Update - 2026-06-18 + +This plan has now advanced beyond the original read-only planning state. `main` includes PRs #26-#29: + +- **PR #26:** live write-back is gated by `PROCORE_LIVE_WRITEBACK_ENABLED=false` by default. +- **PR #27:** migration 008 is hardened with pinned `search_path` and a controlled delete guard. +- **PR #28:** PDF/JWT dependency smoke coverage exists for the upgraded extraction/auth dependencies. +- **PR #29:** metadata-only Procore audit payload builder and graceful Supabase RPC wiring are implemented. + +The remaining blockers are still real: Procore auth/signature scheme, OAuth/DMSA grant/scopes, write-back resource/path/API version, and `delivery_id`/`ulid` payload location remain `UNVERIFIED`; migrations 007/008 remain `UNAPPLIED`; production env/migration application remains `OPS-GATED`. + --- ## Deliverable 1 — Stale `file:line` Correction Table @@ -130,44 +141,45 @@ Each phase lists objective, key evidence, and exit gate. Per `CLAUDE.md`, any ph **Safe Method — Procore integration data-handling statement (draft):** -> Procore remains the system of record. Safe Method does not create a permanent copy of customer project data. For each SWMS review, Safe Method retains a **minimal audit record** in its managed database (Supabase): the Procore event/delivery identifier, a SHA-256 hash of the payload, project and company identifiers, the rule-pack version and the criteria checked, structured finding summaries and the advisory status recommendation, write-back metadata, and human-override records. Safe Method **does not retain** the raw SWMS document text, full Procore document content, full webhook payload bodies, or any OAuth tokens/secrets in that audit record. +> Procore remains the system of record. Safe Method does not create a permanent copy of customer project data. For each SWMS review, Safe Method retains a **minimal audit record** in its managed database (Supabase): the Procore event/delivery identifier, project and company identifiers, a document fingerprint/hash generated from the extracted SWMS review text (not from the full webhook payload), rule-pack/library versions, review status fields, finding and hard-fail counts, write-back metadata, and human-override records. Safe Method **does not retain** the raw SWMS document text, full Procore document content, full webhook payload bodies, finding prose, comment bodies, or any OAuth tokens/secrets in that audit record. > > Audit records are stored in a database with append-only (no-update/no-delete) controls and are retained for **[12 months — Alan to confirm]**, after which they are purged. Idempotency/delivery records are retained for **[30 days — Alan to confirm]** (covering Procore's documented retry window) and then purged. Customers may request deletion of their records by company or project identifier; Safe Method will action the request and confirm completion. > > SWMS document content is processed transiently by the Anthropic API for the review step. The applicable data-processing terms (including a Zero-Data-Retention arrangement) are **[status — Alan to confirm; ZDR is an open action item, `PROCORE_TECHNICAL_FEASIBILITY_REPORT.md:135,169`]** and must be locked before this statement is submitted. -**Engineering precondition for this language to be true:** local JSONL persistence (`webhook_handler.py:136-165`, `review_store.py:22-27`, written at `main.py:1330,1410,1413`) must be removed or gated off in production (Phase 4). Until then, the statement above is aspirational, not factual. +**Engineering status for this language:** local JSONL persistence is gated off by default (`PROCORE_LOCAL_JSONL_ENABLED=false`), and PR #29 added metadata-only audit payload/RPC wiring. The statement is still conditional on applying migrations 007/008 after Supabase preflight and confirming final retention/ZDR terms. --- -## Deliverable 6 — Implementation Tickets (draft, for later approval) +## Deliverable 6 — Implementation Tickets / Current Status -> Not implemented. Each ticket is bounded, pytest-gated, manually committed (no auto-push) per `CLAUDE.md`. **Only the final wiring of T2 (auth scheme) and T8 (write-back resource) depends on Phase 1 Procore answers; all others are Track A and can start now.** +> Current status after PRs #24-#29. Several confirmation-independent tickets are implemented; the final T2/T7/T8 production wiring still depends on Procore answers and ops-controlled Supabase migration application. | ID | Title | Scope (evidence) | Depends on | Acceptance | |---|---|---|---|---| -| T1 | Disable legacy `/procore` route registration | comment/guard `main.py:67,157`; keep `api/procore.py` for tests; env flag `ENABLE_LEGACY_PROCORE_ROUTE` (action_05) | — | `POST /procore/webhook` → 404; legacy direct-import tests still pass | -| T2 | Fail-closed webhook auth (**scheme-agnostic**) | pluggable `verify(headers, body) -> bool` replacing optional check `main.py:1314-1320`; reject before any side-effect (currently `log_payload` at `:1330` runs first); state replay/forgery mitigations (HTTPS-only, secret rotation) | Track A now; scheme config from Phase 1 | no secret/bad sig → 401; `log_payload`/`fetch_attachment`/`run_prescreen_review`/`post_submittal_comment` not called; verifier swappable by config | -| T3 | Durable idempotency | replace in-memory `webhook_handler.py:27,103-109`; Supabase `procore_webhook_deliveries` + reserve RPC (action_03); **derived fallback key = SHA-256(raw body)** when no `delivery_id`/`ulid` (current reads `metadata` only, `webhook_handler.py:93`) | T2 | duplicate → single processing; survives restart; reservation before side-effects; works with no delivery_id | -| T4 | Async 202 + dead-letter/failure surface | extract `_process_procore_v1_webhook` from `main.py:1367-1459`; use `BackgroundTasks` as interim async (pattern only — do **not** import deprecated `api/procore.py` helpers into canonical `/v1`); durable status row post-202; **extract Slack/failure logic into a canonical `core/procore/alerts.py`** rather than reusing `api/procore.py:32,87` directly | T3 | 202 returns fast; failed async job recorded + alerted; `/v1` has no import dependency on deprecated `api/procore.py`; operator can see failures | -| T5 | Remove/gate local JSONL in production | gate `webhook_handler.py:136-165`, `review_store.py:22-27` behind non-prod flag | T6 | no Procore-derived files written on disk in prod | -| T6 | Supabase audit tables w/ immutability + retention | new `procore_audit` + delivery tables; reuse triggers (`004_job_states_immutable.sql`); retention purge job | — | UPDATE/DELETE rejected; retention window enforced; deletion-by-company works | -| T7 | OAuth / DMSA token model | replace static `api_client.py:27,36-48`; secret-store; refresh/rotation | Phase 1 (grant) | no static prod token; least-privilege scopes; rotation documented | -| T8 | Advisory comment quality gate + write-back idempotency | `ReviewArtifactV1`/`StoredAuditArtifactV1`/`ProcoreAdvisoryCommentV1`; deterministic gate; goldens; **once-per-`review_run_id` write-back guard** (no duplicate comment on Procore 12h retry); verified write-back path | Phase 1 (write-back resource) | banned approval words blocked; evidence refs required; no duplicate comment on replay; goldens pass | -| T9 | SBOM / vulnerability scan in CI | add `pip-audit` (+ SBOM, e.g. cyclonedx) step to `.github/workflows/ci.yml:37-41` (currently flake8+pytest only; none present — verified) | — | CI fails on known-vuln dep; SBOM artifact produced | -| T10 | Baseline review must not be gated by rule pack | replace the `no_rule_pack` early return (`api/main.py:1351-1356`) with an **empty fallback rule pack carrying `project_id=event.project_id`**, then run baseline/structural review anyway (engine already supports pack-less, `prescreen_reviewer.py:455-457`); pack only adds project-specific criteria | — | project with no pack still receives baseline WHS + structural review; artifact carries `project_id=event.project_id` (**not blanked**); `project_review_status=UNAVAILABLE` returned and stored, not blocked | +| T1 | Disable legacy `/procore` route registration | Implemented in PR #24; legacy route disabled by default behind `PROCORE_LEGACY_ROUTE_ENABLED` | Done | `POST /procore/webhook` returns 404 by default; direct-import legacy tests remain supported | +| T2 | Fail-closed webhook auth (**scheme-agnostic**) | Implemented in PR #24; default `PROCORE_AUTH_SCHEME=unverified` rejects before side effects | Final scheme UNVERIFIED | Candidate HMAC/Bearer verifiers exist; production scheme/header must be set only after Procore confirms | +| T3 | Durable idempotency | Implemented in PR #24; Supabase reserve RPC with raw-body hash fallback | Migrations UNAPPLIED; payload key UNVERIFIED | Duplicate processing is guarded; migration 007 must be applied after ops preflight | +| T4 | Async 202 + dead-letter/failure surface | Implemented in PR #24; `/v1` returns 202 and runs background pipeline with failure alerting | Done | Failures recorded/alerted; worker/queue can remain a later durability upgrade | +| T5 | Gate local JSONL in production | Implemented in PR #25; `PROCORE_LOCAL_JSONL_ENABLED=false` by default | Done | No local Procore-derived JSONL files in production default | +| T6 | Supabase audit tables w/ immutability + retention | Implemented in PR #25 and hardened in PR #27 | Migrations UNAPPLIED; ops preflight required | Migration 008 has private table, RLS, service-role RPCs, pinned trigger search path, controlled retention/delete guard | +| T7 | OAuth / DMSA token model | Not implemented; static token model remains interim | Procore grant/scopes UNVERIFIED | Replace `PROCORE_ACCESS_TOKEN` with confirmed OAuth/DMSA model before certification | +| T8 | Advisory comment quality gate + write-back idempotency | Partially implemented: PR #26 gated live write-back; PR #29 added metadata-only audit builder/RPC wiring | Write-back resource/path UNVERIFIED | Final endpoint/path, write-back idempotency, and comment quality gate still require Procore confirmation | +| T9 | SBOM / vulnerability scan in CI | Implemented in PR #25; PR #28 added PDF/JWT dependency smoke coverage | Done | CI fails on vulnerable pinned deps; SBOM artifact produced; extraction/auth smoke coverage exists | +| T10 | Baseline review must not be gated by rule pack | Implemented in PR #24 | Done | Missing rule pack still produces baseline structural review with `project_review_status=UNAVAILABLE` | --- ## Residual UNVERIFIED items (must resolve before submission) 1. Webhook signature/auth scheme — Procore confirmation (D2 Q1). -2. Write-back resource + REST path/version — Procore confirmation (D2 Q3); do **not** invent. +2. Write-back resource + REST path/version — Procore confirmation (D2 Q3); do **not** invent. Live write-back is safely gated off by default, but the final resource is still `UNVERIFIED`. 3. OAuth grant + DMSA support + scopes — Procore confirmation (D2 Q4). 4. `delivery_id`/`ulid` location in real payloads — Procore confirmation (D2 Q2); current code reads `metadata` only (`webhook_handler.py:93`). 5. `tests/test_procore_webhook.py` line citations in action_01 — not re-opened this pass; re-pin on edit. 6. Anthropic ZDR status — Alan (`report:135,169`). 7. Retention windows (12 mo / 30 day) — Alan to set. +8. Supabase migrations 007/008 — `UNAPPLIED`; apply only after project/schema/service-role preflight. --- diff --git a/docs/procore/TRACK_A_COMPLETION_REPORT.md b/docs/procore/TRACK_A_COMPLETION_REPORT.md index 07426a0..43292b4 100644 --- a/docs/procore/TRACK_A_COMPLETION_REPORT.md +++ b/docs/procore/TRACK_A_COMPLETION_REPORT.md @@ -1,110 +1,118 @@ -# Track A — Completion Report (for Codex review) +# Procore Track A - Evidence Checkpoint -**Date:** 2026-06-17 -**Branches merged to `main`:** PR #24 (T1–T4, T10), PR #25 (T5, T6, T9) -**`main` HEAD after merge:** `1447c27` +**Date:** 2026-06-18 +**Current `main` HEAD:** `0d6160f` (merged PR #29) **Source of truth / plan:** `docs/procore/STAGE3_CERTIFICATION_PLAN_V2.md` -**Scope:** Track A = confirmation-independent P0/P1 hardening. Procore-gated work (T7/T8) is intentionally NOT done. +**Scope:** Track A plus the confirmation-independent post-review hardening through PRs #26-#29. -This report is for Codex to **review** the implemented Track A work. Every claim cites `file:line` or a commit. Nothing here requires re-implementation; verify and flag. +This report replaces the older Track A review snapshot that stopped at PR #24/#25 and `1447c27`. It is an evidence checkpoint for Stage 3 certification readiness. It does not claim the Procore-gated items are complete. --- -## 1. What was delivered (ticket by ticket) - -### T1 — Disable legacy `/procore` route registration (`f0e34fc`) -- `api/main.py`: removed the top-level `from api.procore import router` and made registration conditional + lazy — `if os.getenv("PROCORE_LEGACY_ROUTE_ENABLED", "").lower() == "true":` then import + `include_router`. By default the module isn't imported and the route isn't exposed. -- `api/procore.py` and flat `core/procore_*.py` remain on disk for the direct-import tests in `tests/test_procore_stage2.py`. -- Test: `tests/test_procore_webhook.py::TestWebhookEndpoint::test_legacy_procore_route_not_registered_by_default` → `POST /procore/webhook` returns **404** by default. -- **Review focus:** confirm no other code path imports `api.procore` at module load; confirm the rollback flag path still works. - -### T10 — Baseline review not gated by rule pack (`1b3a104`) -- `api/main.py`: replaced the `no_rule_pack` early return with `if rule_pack_path.exists(): load else: rule_pack = {"project_id": event.project_id}`. Baseline/structural review always runs; the pack only ADDS project-specific criteria. -- `artifact.project_id` stays populated because `run_prescreen_review` reads it from the pack (`core/procore/prescreen_reviewer.py:396`); `project_review_status` resolves to `UNAVAILABLE` for an empty pack (`prescreen_reviewer.py:375`). -- **Review focus:** confirm the empty fallback pack can't accidentally suppress baseline structural findings (it can't — it has no `rules`/`structural_expectations`). - -### T2 — Scheme-agnostic, fail-closed webhook auth (`15a3e55`) -- `api/main.py`: new `_verify_procore_request(headers, body)` selected by env at call-time. Replaced the old optional `if _PROCORE_WEBHOOK_SECRET:` block (removed that constant + the route's unused `validate_signature` import). -- Env contract: `PROCORE_REQUIRE_AUTH` (default `true`), `PROCORE_AUTH_SCHEME` (default `unverified` → reject), schemes `hmac_sha256` / `authorization_bearer`, configurable `PROCORE_SIGNATURE_HEADER`. Unknown/unset scheme → fail closed. -- Auth runs **before any side effect**. Tests: `TestWebhookAuth` (unverified→401 with no side effects; no-secret→401; bad bearer→401; valid bearer/HMAC→202). -- **Review focus / UNVERIFIED:** the real Procore scheme is unconfirmed — `validate_signature` (HMAC) and the `Authorization: Bearer` path are both *candidates*. Do NOT treat either as the confirmed scheme until Procore replies (Deliverable 2). Production must set `PROCORE_AUTH_SCHEME` or the endpoint stays 401. - -### T3 — Durable idempotency reserved before side effects (`49909a0`) -- `core/procore/webhook_handler.py`: `delivery_key(delivery_id, raw_body)` (prefers delivery_id, else `sha256:`+hash of body); `reserve_delivery(key, correlation_id)` (Supabase RPC when configured, in-memory fallback, logged); `_reserve_supabase` / `_supabase_configured`. -- `api/main.py`: reservation runs right after `parse_event`, **before `log_payload`** and the pipeline. Duplicate → `already_processed`. -- `supabase/migrations/007_procore_webhook_deliveries.sql`: table in **private** schema (RLS), `reserve_procore_webhook_delivery` RPC in `public` (`SECURITY DEFINER`, `EXECUTE` to `service_role` only). -- Tests: body-hash fallback dedupes; reservation precedes `log_payload`; durable path used when configured; falls back to memory on error. -- **Review focus:** the durable key today is `metadata.delivery_id` only (`webhook_handler.py:93`) — confirm against real Procore payloads whether `ulid` should be primary (UNVERIFIED, Deliverable 2). Migration **not applied** (CLAUDE.md). - -### T4 — Async 202 + dead-letter/failure surface (`4265868`) -- `core/procore/alerts.py` (new): `alert_failure()` — Slack if `SLACK_WEBHOOK_URL`, else logs; never raises. Keeps `/v1` off the deprecated `api/procore.py`. -- `api/main.py`: extracted the pipeline into `async _process_procore_v1_webhook(payload, event, correlation_id)`; route now returns **202 `accepted`** after auth→reserve→`log_payload`, scheduling the pipeline via `BackgroundTasks`. Failures are recorded (`record_state`) + alerted, never raised. `BackgroundTasks` is flagged **interim** (worker/queue later). -- Tests: `TestProcorePipeline` (reviewed; baseline-UNAVAILABLE; ignored; failure→alerted-not-raised); endpoint tests assert 202. -- **Review focus:** the review outcome is no longer in the HTTP response — it's produced in the background. Confirm that's acceptable for the Procore contract (it matches "respond 2xx fast"). Confirm `record_state` calls are safe no-ops offline. - -### T6 — Append-only audit table + retention/deletion (`27194fa`) -- `supabase/migrations/008_procore_audit.sql` (new): `private.procore_audit` (append-only, RLS), **metadata only** (`document_hash`, versions, status, counts, `writeback` jsonb, `reviewer_override` jsonb) — never raw SWMS text. Immutability trigger (UPDATE blocked always; DELETE owner-only). RPCs (`SECURITY DEFINER`, service-role only): `record_procore_audit`, `purge_procore_audit` (retention), `delete_procore_audit_for_company` (customer deletion). -- `tests/test_procore_audit_migration.py`: contract test guarding the invariants (migration can't run in CI). -- **Review focus / not done:** app-side **wiring to emit audit rows** via `record_procore_audit` is deferred to T8 (artifact shape finalized there). T6 delivers schema + guarantees only. Migration **not applied**. - -### T5 — Gate local JSONL persistence off by default (`2954468`, fix `37f6874`) -- `core/procore/webhook_handler.py` + `core/procore/review_store.py`: `log_payload` / `log_review` / `store_artifact` gated behind `PROCORE_LOCAL_JSONL_ENABLED` (default `false`); no-op when disabled. Makes the "minimal audit metadata retained" statement true; durable record is the Supabase audit trail. -- Tests: `TestLocalJsonlGate`; the review-store roundtrip tests in `tests/test_resubmission_comparison.py::TestReviewStore` now enable the flag (regression caught by the pre-push full suite — see §3). -- **Review focus:** when disabled, resubmission comparison finds no prior *local* artifact (acceptable; durable comparison moves to the audit store with T8 wiring). - -### T9 — SBOM + pip-audit vulnerability gate (`10f76aa`, deps `91d5ec4`) -- `.github/workflows/ci.yml`: CycloneDX SBOM artifact (`pip-audit … --format cyclonedx-json`, uploaded `if: always()`) + strict `pip-audit -r requirements.txt` gate. `requirements-dev.txt`: `pip-audit[cyclonedx]`. -- **The gate found 19 real CVEs on first run** (all fixable). Bumped in `91d5ec4`: `cryptography 46.0.7→48.0.1`, `pypdf 6.9.2→6.13.0` (12 CVEs), `python-dotenv 1.1.0→1.2.2`, `python-multipart 0.0.22→0.0.31` (4 CVEs). -- **Review focus:** confirm the bumped deps behave correctly in areas Codex knows (pypdf is used in extraction; cryptography is a major bump). CI full suite passed on the upgraded deps, but targeted manual spot-checks of PDF extraction are worth it. +## 1. Implemented And Verified + +### PR #24 - T1/T2/T3/T4/T10 core hardening +- T1: legacy `/procore` route disabled by default behind `PROCORE_LEGACY_ROUTE_ENABLED`; flat modules remain for direct-import tests. +- T2: `/v1/procore/webhook` is fail-closed by default with `PROCORE_AUTH_SCHEME=unverified`; auth runs before parsing, idempotency, logging, fetch, review, or comment. +- T3: durable idempotency reservation is attempted before side effects; fallback key is `sha256:` + raw body hash when no delivery id is available. +- T4: `/v1/procore/webhook` returns fast `202 accepted` and runs the review pipeline in `BackgroundTasks`; failures are recorded and alerted without raising to Procore. +- T10: baseline/structural review runs even when a project rule pack is missing; project-specific criteria only add to the baseline review. + +### PR #25 - T5/T6/T9 audit foundation, local-retention reduction, supply-chain gate +- T5: local JSONL persistence for payloads, reviews, and artifacts is gated behind `PROCORE_LOCAL_JSONL_ENABLED=false` by default. +- T6: migrations `007_procore_webhook_deliveries.sql` and `008_procore_audit.sql` define private-schema delivery/audit storage, RLS, service-role RPCs, retention, and deletion support. +- T9: CI now produces an SBOM artifact and runs `pip-audit`; dependency CVEs found during Track A were patched. + +### PR #26 - P0 live write-back blocker resolved +- `PROCORE_LIVE_WRITEBACK_ENABLED` now gates live Procore comment write-back and defaults to `false`. +- Even with live credentials and `retrieval_mode == "live_api"`, no comment is posted unless the flag is explicitly `true`. +- The write-back resource/path itself was not changed and remains `UNVERIFIED` pending Procore confirmation. + +### PR #27 - migration 008 hardened before apply +- `private.prevent_procore_audit_mutation()` now pins `search_path`. +- Audit retention/customer deletion no longer depends on the migration owner being `postgres` or `audit_admin`; controlled delete functions use a transaction-local guard. +- Migration 008 remains **UNAPPLIED**. + +### PR #28 - PDF/JWT dependency smoke tests +- Added smoke coverage for real PDF extraction and textless-PDF fallback when `pypdf` is installed. +- Added a real ES256 JWT/JWKS decode smoke through `core.auth.get_current_user`. +- This specifically covers the `pypdf`, `cryptography`, and `python-jose[cryptography]` upgrade risk from PR #25. + +### PR #29 - metadata-only audit builder and RPC wiring +- Added a metadata-only Procore audit builder and graceful Supabase RPC writer. +- `/v1/procore/webhook` now attempts `record_procore_audit` after review/comment metadata is known. +- Audit payload excludes raw SWMS text, review prose, amendment reasons, comment body, attachment bytes, and full webhook payloads. +- Audit write degrades to no-op when Supabase is unconfigured or migration 008/RPC is absent. +- This is partial T8: audit-row wiring is done; final write-back resource/path is still Procore-gated. --- -## 2. Verification status +## 2. Verification Status -| Check | Result | +| Check | Latest result | |---|---| -| `pytest tests/test_procore_webhook.py` | 71 passed | -| `pytest tests/test_procore_audit_migration.py` | 6 passed | -| Full suite (pre-push gate) | green on every push | -| CI on PR #24 / #25 | green (PR #25 green **on upgraded deps**, validating the bumps) | -| flake8 | clean | -| pip-audit | clean after dep bumps (was 19 CVEs) | +| `pytest tests/test_procore_webhook.py` | 74 passed after PR #26 | +| `pytest tests/test_procore_audit_migration.py` | 7 passed after PR #27 | +| `pytest tests/test_dependency_smoke.py` | CI passed after PR #28; local Python skipped PDF smokes where `pypdf` was absent | +| `pytest tests/test_procore_audit.py tests/test_procore_webhook.py` | 79 passed before PR #29 | +| Full suite pre-push hook | Passed for PRs #26, #27, #28, #29 | +| GitHub CI | Passed before merging PRs #26, #27, #28, #29 | +| flake8 | Clean on changed files for each PR | +| pip-audit | Clean after dependency bumps from PR #25 | --- -## 3. Process / tooling added during Track A (also for review) +## 3. Current Data-Handling Position -- **GitHub branch protection on `main`**: requires the `test` CI check (strict), blocks direct/force pushes, requires conversation resolution; admin break-glass left on. (Used `--admin` to merge #23/#24/#25 when GitHub reported the strict-policy `BLOCKED` formality with CI green + no required review.) -- **Commit automation** (PR #23, merged): commit-msg Conventional-Commits hook (`scripts/check_commit_msg.py`), pre-push full-suite hook, `.gitmessage` template, `default_install_hook_types`. Activate per clone with `pre-commit install`. -- **Pre-existing CI fix** (PR #22, merged): SSA audit-report tests now skip when gitignored `pims/*.docx` fixtures are absent — `main` CI was red before this. -- The pre-push full suite **caught a real T5 regression** (`test_resubmission_comparison.py`) before it reached CI — the automation working as intended. +Use the precise claim: ---- +> Procore remains the system of record. Safe Method retains minimal audit metadata only: event/delivery identifiers, company/project identifiers, a document fingerprint/hash generated from extracted SWMS review text, rule-pack/library versions, status fields, finding and hard-fail counts, and write-back metadata. Safe Method does not retain raw SWMS text, full Procore document content, full webhook payload bodies, attachment bytes, finding prose, comment bodies, or OAuth tokens/secrets in the audit record. + +Do **not** use an unqualified "no data retained" claim. The correct position is minimal metadata retained in Supabase audit tables, with raw document content processed transiently for review. -## 4. What remains (NOT in Track A) — needs Procore confirmation or ops +--- -1. **Procore confirmation email** (`STAGE3_CERTIFICATION_PLAN_V2.md` Deliverable 2) — gates the below. UNVERIFIED items: auth scheme, write-back resource/path, OAuth grant, `delivery_id`/`ulid` payload location. -2. **T7** — OAuth/DMSA token model (replace static `PROCORE_ACCESS_TOKEN`, `core/procore/api_client.py:27`). Needs grant confirmation. -3. **T8** — verified write-back resource + advisory comment quality gate + **wire `record_procore_audit`** to emit audit rows. Needs resource confirmation (current `submittal_logs/.../comments` is deprecated/unverified, `api_client.py:88`). -4. **Ops / deploy prerequisites:** - - Apply migrations `007_procore_webhook_deliveries.sql` and `008_procore_audit.sql` to the Supabase project (verify project URL/tables first, per CLAUDE.md). - - Set app env: `SUPABASE_SERVICE_ROLE_KEY` (durable idempotency + audit), `PROCORE_AUTH_SCHEME` + secret (else endpoint is fail-closed 401), `PROCORE_LEGACY_ROUTE_ENABLED=false`, `PROCORE_LIVE_WRITEBACK_ENABLED=false`, `PROCORE_LOCAL_JSONL_ENABLED=false`. +## 4. Still Open / Gated + +### Procore-gated (`UNVERIFIED`) +1. Webhook auth/signature scheme and exact header/algorithm. +2. OAuth/DMSA grant model and least-privilege scopes. +3. Correct write-back resource/path/API version. +4. `delivery_id` / `ulid` payload location in real webhook deliveries. +5. PM/SM directed visibility or two-tier comment support, if required. + +### Ops-gated +1. Supabase migration preflight: verify project URL, schema state, existing functions/tables, and service-role access. +2. Apply migrations `007_procore_webhook_deliveries.sql` and `008_procore_audit.sql` only after the preflight is clean. +3. Set production env safely: + - `PROCORE_REQUIRE_AUTH=true` + - `PROCORE_AUTH_SCHEME` set only after Procore confirms the scheme + - `PROCORE_WEBHOOK_SECRET` / configured signing secret + - `PROCORE_LEGACY_ROUTE_ENABLED=false` + - `PROCORE_LIVE_WRITEBACK_ENABLED=false` until write-back resource is verified + - `PROCORE_LOCAL_JSONL_ENABLED=false` + - `SUPABASE_SERVICE_ROLE_KEY` for durable idempotency/audit + +### Still not complete +1. T7 OAuth/DMSA token model; static `PROCORE_ACCESS_TOKEN` remains a non-certification-ready interim model. +2. Final T8 write-back resource/path replacement and advisory comment quality gate. +3. Anthropic ZDR / data-processing status and final retention windows, if not already confirmed externally. --- -## 5. Target architecture — now in place (confirmation-independent parts) +## 5. Current Target Architecture ``` Procore Submittal event -> /v1/procore/webhook --> fail-closed scheme-agnostic auth (T2) [scheme UNVERIFIED] --> durable idempotency reservation, pre-side-effect (T3) --> 202 Accepted (T4) --> async pipeline: gate -> fetch -> review -> store -> compare -> comment - -> failure recorded + alerted, never raised (T4) --> minimal immutable audit + retention/deletion (T6) [schema only; wiring in T8] --> [verified write-back] [T8, UNVERIFIED] +-> fail-closed scheme-agnostic auth [scheme UNVERIFIED] +-> durable idempotency reservation, pre-side-effect [migrations UNAPPLIED until ops preflight] +-> 202 Accepted +-> async pipeline: fetch -> extract -> review -> compare -> optional comment + -> live write-back gated off by default + -> failures recorded + alerted + -> metadata-only audit RPC attempted, no-op if Supabase/RPC absent -> human Safety Manager decides in Procore ``` -Plus: legacy route disabled (T1), baseline review always runs (T10), local JSONL gated off (T5), supply chain scanned + patched (T9). - + +The confirmation-independent foundation is now in place. The remaining work is deliberately blocked on Procore answers and controlled Supabase operations, not more speculative implementation.