Skip to content

fix(logs): run PII redaction over HTTP and fix Presidio provisioning#5143

Open
TheodoreSpeaks wants to merge 3 commits into
stagingfrom
fix/pii-redaction-presidio-runtime
Open

fix(logs): run PII redaction over HTTP and fix Presidio provisioning#5143
TheodoreSpeaks wants to merge 3 commits into
stagingfrom
fix/pii-redaction-presidio-runtime

Conversation

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator

Summary

  • Followup to the PII log-redaction feature, which was scrubbing every field to [REDACTION_FAILED] in staging because Presidio is not actually runnable in the deployed runtimes (the guardrails PII block hit the same wall but fail-opens, so it looked fine).
  • Path/fail-fast: resolve the guardrails venv via candidate paths (mirrors isolated-vm.ts) and throw instead of silently falling back to system python3 — that fallback produced the misleading "Presidio not installed".
  • spaCy model: install en_core_web_lg (which the default AnalyzerEngine() loads) in setup.sh and docker/app.Dockerfile; it was never provisioned in the image.
  • Async re-route: redaction now calls an internal POST /api/guardrails/mask-batch endpoint (internal-auth) instead of spawning Python in-process, so Presidio always runs in the app container — including async executions that persist inside the trigger.dev runtime, where there is no Python.

Type of Change

  • Bug fix

Testing

  • Unit: pii-redaction.test.ts + new mask-batch/route.test.ts (9 passing)
  • bun run check:api-validation:strict passes (route baseline 859→860)
  • bun run lint clean
  • sim build: compiled + TypeScript clean
  • Remaining real-env checks before relying on prod: docker image parity (venv+model resolve) and an async trigger.dev run masking via the HTTP path

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

- resolve the guardrails venv via candidate paths and fail fast instead of
  silently falling back to system python3 (the misleading "Presidio not
  installed" that broke redaction and the guardrails block in deployed runtimes)
- install the en_core_web_lg spaCy model in setup.sh and app.Dockerfile
- route log redaction through an internal /api/guardrails/mask-batch endpoint
  so Presidio always runs in the app container, including async executions that
  persist inside the trigger.dev runtime
@vercel

vercel Bot commented Jun 20, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jun 20, 2026 1:37am

Request Review

@cursor

cursor Bot commented Jun 20, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches PII handling and log persistence paths; misconfiguration of internal auth or the app URL could break redaction (fail-safe scrub) or leave masking dependent on a single container endpoint.

Overview
Fixes execution log PII redaction that was scrubbing everything to [REDACTION_FAILED] in staging because Presidio could not run in trigger.dev and the guardrails venv was mis-resolved or incomplete.

Execution log redaction now calls maskPIIBatchViaHttp instead of in-process maskPIIBatch, so masking always hits the app container via internal JWT on POST /api/guardrails/mask-batch (Zod contract, internal auth, in-container maskPIIBatch). The HTTP client chunks large batches (byte/count limits, per-chunk tokens, 45s timeout) and still fails safe to [REDACTION_FAILED] on errors.

Presidio provisioning: resolveGuardrailsPython() probes monorepo vs apps/sim cwd layouts and throws if the venv is missing (no silent system python3 fallback). en_core_web_lg is installed in setup.sh and docker/app.Dockerfile after pip install.

Tests cover the new route and HTTP client; pii-redaction mocks the client; API route baseline 859→860.

Reviewed by Cursor Bugbot for commit d0e573b. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread apps/sim/lib/guardrails/mask-client.ts
@greptile-apps

greptile-apps Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a production regression where PII log-redaction was scrubbing everything to [REDACTION_FAILED] because Presidio (a Python venv) is unavailable in the trigger.dev async runtime. The fix routes redaction through a new internal HTTP endpoint (POST /api/guardrails/mask-batch) so Presidio always executes inside the app container, and hardens provisioning by installing the missing en_core_web_lg spaCy model in both setup.sh and the Dockerfile.

  • New mask-client.ts + /api/guardrails/mask-batch: replaces in-process Python spawning with a chunked HTTP client (byte/count-budgeted, 45s abort signal, per-chunk fresh JWT) that calls back to the app container; the route handler wraps maskPIIBatch in try/catch and returns a structured 500 on venv failure so PII is never leaked.
  • resolveGuardrailsPython refactor: probes both monorepo-root and apps/sim-root candidate layouts and throws on missing venv instead of silently falling back to system python3, eliminating the misleading "Presidio not installed" false-positive.
  • spaCy model provisioning: adds python -m spacy download en_core_web_lg to setup.sh and docker/app.Dockerfile so the default AnalyzerEngine() can load the required NLP model.

Confidence Score: 5/5

Safe to merge — the change reroutes PII masking through a well-guarded HTTP endpoint and tightens venv provisioning without introducing new failure modes.

The HTTP client correctly chunks by bytes and count, mints fresh JWTs per chunk, and carries an abort signal. The fail-safe chain (non-2xx → throw → REDACTION_FAILED) prevents PII from ever being persisted on failure. Unit tests cover auth rejection, chunking, error propagation, and empty input.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/lib/guardrails/mask-client.ts New HTTP client for PII masking; cleanly chunks by bytes (256KB) and count (2,000 per request), mints a fresh JWT per chunk to handle long-running batches, and carries a 45s AbortSignal. Error propagation is correct.
apps/sim/app/api/guardrails/mask-batch/route.ts New internal route protected by internal JWT auth; validates body via contract, wraps maskPIIBatch in try/catch, and returns a structured 500 with error detail on failure so callers can safely fall back to REDACTION_FAILED.
apps/sim/lib/logs/execution/pii-redaction.ts Replaces dynamic in-process maskPIIBatch import with static maskPIIBatchViaHttp; mask-client.ts has no child_process/fs dependencies, so the static import is safe and the fail-safe REDACTION_FAILED path is preserved.
apps/sim/lib/guardrails/validate_pii.ts Consolidates duplicate path-resolution logic into resolveGuardrailsPython(); probes both monorepo layouts and throws on missing venv instead of silently falling back to system python3.
apps/sim/lib/api/contracts/hotspots.ts Adds guardrailsMaskBatchContract with body (texts array max 100k, entityTypes, optional language) and response schema; well-structured and consistent with existing contracts.
docker/app.Dockerfile Adds en_core_web_lg spaCy model download into the venv after Presidio pip install; runs within the same pip cache mount layer so the model is cached across rebuilds.
apps/sim/lib/guardrails/setup.sh Mirrors the Dockerfile by running python -m spacy download en_core_web_lg after pip install so local dev environments provision the required NLP model.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant TD as trigger.dev runtime
    participant NX as Next.js app container
    participant PY as Presidio (Python venv)

    Note over TD,PY: Log persist path (both runtimes)

    TD->>NX: POST /api/guardrails/mask-batch
    Note right of TD: internal JWT, chunked ≤2000 items / 256KB
    activate NX
    NX->>NX: checkInternalAuth()
    NX->>NX: parseRequest() — Zod validation
    NX->>PY: spawn venv/bin/python3 validate_pii.py
    PY-->>NX: "__SIM_RESULT__={masked:[...]}"
    NX-->>TD: "200 { masked: [...] }"
    deactivate NX

    Note over TD: maskPIIBatchViaHttp merges chunk results, preserves order

    Note over NX,PY: On venv missing: resolveGuardrailsPython throws
    Note over NX,PY: route returns structured 500
    Note over NX,PY: caller scrubs to [REDACTION_FAILED]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant TD as trigger.dev runtime
    participant NX as Next.js app container
    participant PY as Presidio (Python venv)

    Note over TD,PY: Log persist path (both runtimes)

    TD->>NX: POST /api/guardrails/mask-batch
    Note right of TD: internal JWT, chunked ≤2000 items / 256KB
    activate NX
    NX->>NX: checkInternalAuth()
    NX->>NX: parseRequest() — Zod validation
    NX->>PY: spawn venv/bin/python3 validate_pii.py
    PY-->>NX: "__SIM_RESULT__={masked:[...]}"
    NX-->>TD: "200 { masked: [...] }"
    deactivate NX

    Note over TD: maskPIIBatchViaHttp merges chunk results, preserves order

    Note over NX,PY: On venv missing: resolveGuardrailsPython throws
    Note over NX,PY: route returns structured 500
    Note over NX,PY: caller scrubs to [REDACTION_FAILED]
Loading

Reviews (4): Last reviewed commit: "fix(guardrails): mint internal token per..." | Re-trigger Greptile

Comment thread apps/sim/lib/guardrails/mask-client.ts
Comment thread apps/sim/app/api/guardrails/mask-batch/route.ts Outdated
@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile review

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@BugBot review

Comment thread apps/sim/lib/api/contracts/hotspots.ts
- chunk maskPIIBatchViaHttp by count (2000) and bytes (256KB) so large
  executions split across requests and never hit the contract's 100k cap
- add AbortSignal.timeout(45s) per request so a slow/unreachable app container
  aborts and the caller scrubs, instead of hanging the trigger.dev job
- catch maskPIIBatch failures in the route: log and return a structured 500
  (broken venv fails loudly server-side; caller still scrubs, no leak)
- add mask-client tests (order across chunks, count split, non-2xx, empty)
@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile review

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@BugBot review

Comment thread apps/sim/lib/guardrails/mask-client.ts Outdated
A single token (5min TTL) could expire mid-batch when a large execution
fans out into many sequential chunk requests; mint one per request instead.
@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile review

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@BugBot review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit d0e573b. Configure here.

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