[Feature]: add opt-in sensitive-data Guardrails #3705

Activity

1. github-actions

   added

   enhancementNew feature or requestNew feature or request

   proxyHTTP proxy, routing, reverse-proxy / management authHTTP proxy, routing, reverse-proxy / management auth

   streamingSSE, WebSocket, terminal stream framesSSE, WebSocket, terminal stream frames

   toolstool_calls, MCP, web-search / sidecar toolstool_calls, MCP, web-search / sidecar tools

   on Sep 5, 2026
2. lidge-jun commented on Sep 5, 2026

   

   lidge-jun

   on Sep 5, 2026

   Owner

   More actions

   리뷰 · 우선순위 34 / 80

   이 이슈는 OpenCodex가 실수로 자격 증명·개인 데이터를 LLM 제공자에게 넘기지 않도록, 기본 꺼짐(opt-in)인 프로세스 안 Guardrails를 넣자는 설계 제안입니다. 제공자별 또는 전체 적용, 제공자 I/O 직전에 민감값을 결정적 플레이스홀더로 바꾸고, 성공한 어시스턴트 본문에서만 원래 값을 되돌리며, 도구 인자·실패 응답·늦은 스트림 실패·조기 EOF·용량 폴백은 계속 가린 채로 두자는 내용입니다. JSON / SSE / WebSocket / continuation 경로를 모두 타고, 설정·규칙·테스터·가져오기 미리보기·메타데이터만 보는 활동은 기존 Management API와 대시보드에 붙이며, 별도 서버·DB·기증 UI는 없다는 범위도 분명합니다. 라벨 proxy / streaming / tools가 맞습니다. 프록시 라우팅·스트림 수명주기·tool_calls 인자가 한 축으로 묶이는 기능입니다.

   지금 dev HEAD는 7e7ab281c(패키지 2.44.0)이고, 릴리스 열차의 초점은 C-lane OAuth(방금 3–4층 착지)·A-lane 런타임·카탈로그 스택입니다. 프리뷰 배포는 계획에 없습니다. 이 제안은 그 열차의 출하 차단 버그가 아니라 큰 설계 확장이라, 우선순위는 낮게 잡았습니다. 작성자(rmsff)는 이미 작업 브랜치와 로컬 게이트·테스트 영수증을 갖고 있으며, 큰 draft PR을 열기 전에 방향만 확인해 달라고 했습니다. 외부 필터 프록시(cloud-ru-tech/guardrails-llm-filter)를 안으로 들이되 re2-wasm 추가와 보안 검토·maintainer-sponsored가 필요하다는 점도 스스로 적었습니다.

   현재 HEAD를 짧게 찾아 보면, 요청 본문을 제공자 보내기 전에 플레이스홀더로 바꾸는 Guardrails 기능은 없습니다. 있는 것은 로그·에러·헤더용 비밀 가리기입니다.

   경로/심볼 src/lib/redact.ts / redactSecretString / SENSITIVE_KEY_PATTERN - 로그·에러 메시지·헤더에서 시크릿을 [REDACTED]로 지우는 쪽입니다. 제공자 I/O 전 본문 치환·성공 응답만 복원하는 Guardrails와는 역할이 다릅니다.

   경로 src/server/management/ (route-registry.ts, config-routes.ts, provider-routes.ts 등) - Management API 표면은 이미 있습니다. Guardrails 설정·규칙·테스터용 라우트/대시보드 페이지는 아직 없습니다.

   경로 src/server/chat-native-sse.ts, src/router.ts, src/bridge.ts - 스트림·라우팅 뼈대는 있습니다. 이 이슈가 말하는 “모든 JSON/SSE/WS/continuation에 동일 마스크 정책”은 여기 전 구간을 가로지르는 새 레이어입니다.

   경로 gui/src - Guardrails / sensitive-data 전용 UI 컴포넌트는 보이지 않았습니다. Dashboard → Guardrails는 신규 면입니다.

   경로 src/adapters/kiro.ts의 GUARDRAIL_INTERVENED - 상류 Kiro 중단 사유 문자열일 뿐, 제품 Guardrails 기능이 아닙니다.

   메인테이너의 판단이 필요한 지점

   • 기본 꺼짐·제공자 선택·성공 본문만 복원(도구 인자·실패는 계속 마스크) 정책을 제품 기본으로 받아들일지
   • Management API + 대시보드만 쓰고 별도 서버/DB를 금지하는 범위가 유지 가능한지
   • re2-wasm·외부 규칙 자산(Apache-2.0) 도입을 보안 검토 전제에서 허용할지
   • 큰 draft PR을 지금 받을지, 아니면 짧은 설계/RFC 합의 뒤에만 받을지 (현재 C-lane·카탈로그 열차와의 순서)

   너의 추천
   라벨은 그대로 두고, 댓글 기준으로는 needs-design / help wanted 성격으로 보는 것이 맞습니다. 큰 draft PR은 C-lane OAuth·카탈로그 열차가 더 안정될 때까지 미루고, 먼저 이 이슈에 **짧은 설계 메모(RFC 댓글)**를 부탁하세요. 최소로 (1) 마스크/복원이 끼는 정확한 훅 지점(요청 직전·SSE 청크·tool_calls·continuation), (2) 실패·폴백 시 마스크 유지 불변식, (3) Management API 경로 초안, (4) re2-wasm 보안·번들 영향 한 단락이면 됩니다. 합의가 나면 전면 PR 대신 프록시 한 경로(예: JSON non-stream)만의 spike PR을 초대하는 쪽이 리뷰 부담이 작습니다. 기존 src/lib/redact.ts와 이름을·책임을 섞지 말고, Guardrails는 별 모듈로 두라고 명시해 주세요.

   이 댓글은 grok-bot이 작성했습니다
3. rmsff commented on Sep 6, 2026

   

   rmsff

   on Sep 6, 2026

   Author

   More actions

   Thanks, the RFC-first ordering makes sense. I will not open the current large branch as-is.

   One freshness note first: dev has moved again since this issue was opened and is now at 00d0cc140 (still 2.44.0). My working branch is based on 22da7a4bc, so any PR would be rebased and fully reverified before it is opened.

   1. Request and response hook points

   Guardrails stays in its own src/guardrails/ module. It does not change the responsibility of src/lib/redact.ts, which continues to handle logs, errors and headers.

   At request ingress, OpenCodex captures one immutable policy. Once the provider is classified, the request leases one immutable compiled-rule generation. An unknown provider is treated as protected until routing can classify it. A known excluded provider bypasses the Guardrails runtime and never imports RE2/WASM.

   Masking runs after any OpenCodex normalization that can materialize plaintext, such as replay expansion or recovered task input, and before adapter serialization or the first provider send. The current call sites are:

   • src/server/responses/core.ts for Responses and translated paths;
   • src/server/chat-completions.ts for native Chat Completions;
   • src/server/claude-messages.ts for native Anthropic Messages;
   • src/server/responses/compact.ts for compaction.

   Each call uses the same prepareGuardrailsTurn contract and produces a cloned masked body plus one request-scoped mapping. Retries, combo children and policy fallbacks reuse that prepared state rather than rescanning and renumbering placeholders. Text introduced later by a tool or sidecar is appended to the same turn state and scanned before the next provider send.

   The response boundary is outside provider adapters in demaskGuardrailsResponse. JSON restoration is limited to allowlisted assistant text fields. Tool arguments and provider-controlled failure envelopes remain masked.

   SSE and client WebSocket use the same frame rewriter. Once a frame would restore a placeholder, both its masked and restored forms are held under a bounded queue. The restored form is released only after a protocol success terminal (response.completed, Chat [DONE], or Anthropic message_stop). A failure terminal, malformed frame, premature EOF or capacity limit releases the masked form instead.

   That does delay restored streaming text after the first placeholder until the terminal event. I do not see a safe way to expose the original token immediately and still retract it if a later response.failed arrives. The simpler alternative would be to never restore streaming output; the working implementation uses terminal gating.

   2. Failure, fallback and continuation invariants

   The default failure policy is block. Registry, scan or capacity failure returns locally before the affected provider send. An explicit passthrough policy can roll back only from a complete pre-mask clone before the next provider I/O; a partially mutated body is never forwarded. A late tool or media failure blocks if that complete rollback is unavailable.

   A retry keeps the same masked body and mapping. If fallback changes the request from an excluded provider to a protected provider after preparation, OpenCodex returns a local 409 guardrails_policy_changed rather than sending an unprotected body.

   Continuation mappings stay in process memory. They are scoped by the admission identity and a digest of the client thread, expire after one hour, and are capped at 1,000 entries / 8 MiB. Collisions poison the entry. If a mapping is missing after restart or expiry, placeholders remain masked.

   3. Management API and dashboard

   The proposed surface is the authenticated /api/guardrails namespace registered in the existing Management API. It contains overview and settings, rule catalog and CRUD, a synthetic-value tester, import/export with a preview step, plus metadata-only activity.

   Mutations require If-Match and use the existing atomic config coordinator. Import has a dry-run security diff before apply. Tester input is not persisted. Activity stores counts, category/rule IDs, mode, result and latency; it does not retain request text, response text, originals or provider error messages.

   The dashboard is a normal #guardrails page built from existing OpenCodex components. There is no second server, database or donor UI.

   4. RE2/WASM, rules and package impact

   The two direct dependencies added by the working branch are re2-wasm@1.0.2 and yaml@2.8.1. The WASM runtime is dynamically imported only after Guardrails is enabled for the selected provider. The disabled path has a regression test proving there is no static route to re2-wasm.

   The registry contains 266 rules from cloud-ru-tech/guardrails-llm-filter@bbd6f274 plus six OpenCodex-owned supplemental rules. Donor files, generated fixtures, npm integrity, WASM loader/binary hashes and license texts are recorded in src/guardrails/rules/provenance.json and checked during packaging.

   The main work limits are 100 custom rules, a 4 KiB pattern, 128 KiB per scanned leaf, 2 MiB scanned text per turn, 4,096 findings, and a 128 MiB aggregate regex-input budget. Locally, re2-wasm occupies about 1.1 MiB installed; src/guardrails is about 400 KiB, including roughly 144 KiB of rule assets.

   Proposed review shape

   If a maintainer wants to validate the seam before the current release lanes settle, I can rebase onto current dev and open a draft-only spike limited to the registry/config foundation plus one non-streaming JSON path. I would keep that spike non-mergeable and default-off so it cannot imply protection for Chat, SSE or WebSocket before those paths exist.

   If this boundary and spike shape look reasonable, I will refresh the branch and open only that smaller draft after a human maintainer confirms the direction.

   Technical draft prepared with OpenAI Codex (gpt-5.6-sol, xhigh reasoning); reviewed and posted by the contributor.
4. rmsff commented on Sep 8, 2026

   

   rmsff

   on Sep 8, 2026

   Author

   More actions

   Hi @lidge-jun and @Ingwannu, I've refreshed the branch and opened draft #4022 with the implementation, screenshots and verification details.

   It includes dev@29bb221c3 (2.49.0; latest stable 2.48.0). The Cloud.ru donor is still at v0.1.2.1 / bbd6f274, with no merged rule updates to bring in. I also updated the YAML dependency from the RFC's 2.8.1 to 2.8.3 and refreshed its provenance.

   Local verification includes 22,085 backend tests passed with 40 existing platform/runtime skips, 1,994 dashboard tests passed, package checks and isolated desktop/mobile testing. The PR distinguishes the full-suite baseline from the final version-only merge and its fresh checks; GitHub CI is separate.

   I previously offered a smaller spike. I've made the full branch available as a draft so you can inspect the integration and UI, but I'm happy to split the landing if that's easier to review. It remains default-off and supports selecting which providers to protect.

   Could you take a look at the proposed boundaries and let me know which review shape you prefer? The security review and maintainer-sponsored requirement are still pending; I'm leaving the PR in draft while we agree on that.
5. Ingwannu commented on Sep 8, 2026

   

   Ingwannu

   on Sep 8, 2026

   Owner

   More actions

   Thanks for the refreshed draft and for separating final-head checks from the full-suite baseline. My review preference is split landing, with #4022 retained as an integration reference draft rather than approving the entire feature at once.

   Please outline a small stack here before doing another large rewrite: (1) inert policy/types and the core-owned activation seam, (2) one non-streaming path with fail-closed behavior and synthetic tests, then (3) remaining transports/continuations and finally management/UI. A partial implementation must stay non-shipping or explicitly unavailable on uncovered routes; a toggle must not imply protection that only one path actually provides. Disabled users must avoid WASM loading, timers and per-request scans, consistent with the current optional-subsystem rule.

   The RFC's terminal-gated restoration tradeoff is explicit and useful. The review still needs bounded retention of originals, identity-scoped continuation state, fallback/retry ownership, and tests proving tool arguments and failed/premature streams stay masked. I have not audited the full implementation or its dependencies and am not asserting those properties already hold.

   @lidge-jun please confirm the product/sponsorship decision before a mergeable stack is prepared. This comment is review-shape guidance, not security approval, a maintainer-sponsored grant, or approval of the GUI. No need to close the existing draft or repeat unchanged full-suite runs just for this reply.
6. rmsff commented on Sep 8, 2026

   

   rmsff

   on Sep 8, 2026

   Author

   More actions

   I've prepared a smaller core-only review branch and kept #4022 open as the full reference. The core is inert: no proxy hooks, settings, API or dashboard, and no network protection claim. I have not opened a second PR ahead of the design agreement requested here.

   Here is the contract I'd like to get agreement on before the integration goes further. The hook and failure matrix links each protocol boundary to its regression tests.

   Boundary	Proposed contract
   Admission and outbound text	Off by default; one immutable policy per logical turn, canonical provider scope, masking before provider I/O, shared budgets across retries and newly materialized sidecar text.
   Response restoration	Only allowlisted successful assistant prose. SSE/WebSocket restoration waits for the protocol's success terminal. Errors, premature EOF, capacity fallback and executable fields stay masked.
   Lost mappings	Bounded process-local state, no disk recovery. Restart/expiry/eviction leaves unresolved placeholders. The operator guide explains how to restart a session with source context without disabling protection or guessing executable values.
   Failure policy	Block by default. Explicit request passthrough is allowed only with complete rollback; it never turns a restoration failure into unmasked output.

   The proposed review order is inert core + pinned rules/tests → complete data-plane and lifecycle → Management API → dashboard and operator docs. Rules and provenance stay with the scanner because they determine its behavior. Enablement would arrive with the full advertised protocol coverage, not a global switch backed by one protected route.

   The contribution docs now record the 33 historical synthetic Push Protection matches and the per-value checks. Repository-wide protections remain unchanged. Security sponsorship still needs a maintainer to review the rule update channel, RE2/WASM/package hashes, Management API imports and fixture exceptions.

   Would you prefer this core-first boundary, or the earlier non-streaming JSON spike as a test-only integration exercise? This is a proposed freeze, not an approval record. Once a maintainer confirms the direction, the smaller PR can be opened from the prepared branch. I won't chase new dev commits while that decision is pending.
7. rmsff commented on Sep 10, 2026

   

   rmsff

   on Sep 10, 2026

   Author

   More actions

   Hi @lidge-jun, @Ingwannu — a quick follow-up on the proposed split. The inert core branch is prepared and tested. Would you prefer a core-first PR or the smaller JSON-only spike? I’m holding off on opening another PR until we agree on the direction. Thanks!
8. rmsff commented on Sep 19, 2026

   

   rmsff

   on Sep 19, 2026

   Author

   More actions

   Bringing this back up, and tagging you individually in case notifications route differently.

   @lidge-jun — a one-line answer is enough here: core-first PR, or the smaller JSON-only spike? I'm keeping the second PR unopened until we have a direction.

   @Ingwannu — if you are the right reviewer for the security-facing parts, could you say what you'd need to see before maintainer-sponsored? I'd rather adjust the boundary now than open a PR that cannot pass the gate.

   The inert core branch is ready and tested; #4022 stays as the full reference either way. No rush — either answer unblocks the next step.
9. devin-ai-integration

   added

   priority: P3Low: new provider/client integration, large or experimental feature (>2000 LOC or >50 files), RFC/roLow: new provider/client integration, large or experimental feature (>2000 LOC or >50 files), RFC/ro

   on Sep 24, 2026
10. devin-ai-integration commented on Sep 24, 2026

    

    devin-ai-integrationbot

    on Sep 24, 2026 – with Devin.ai Integration

    Contributor

    More actions

    Triage: priority: P3 — sensitive-data Guardrails feature.

    Criteria (P3): Low: new provider/client integration, large or experimental feature (>2000 LOC or >50 files), RFC/roadmap, or long-stale branch.

    Matched pull requests:

    • feat: add opt-in sensitive-data Guardrails #4022 (would resolve this issue) — feat: add opt-in sensitive-data Guardrails [priority: P3]