Repository navigation
fix(logging,sse): redact sensitive log fields and default SSE comments to disabled - #10539
Merged
diegosouzapw merged 3 commits intoAug 18, 2026
Merged
Conversation
Owner
|
The SSE default change is reasonable, but privacy is not safe to merge yet: debugMode still defaults true, ProxyEgress exposes proxy.host, and the cache does not track setting changes. Establish one opt-in contract, add behavioral tests, update docs, and rerun gates. |
HouMinXi
added a commit
to HouMinXi/OmniRoute
that referenced
this pull request
Aug 17, 2026
…eload debugMode defaulted to true, exposing client IPs, account prefixes, and proxy hosts in ProxyEgress logs by default. Change default to false, redact proxy.host when debugMode is off, and wire debugMode into applyRuntimeSettings so the cached flag updates when the operator changes the setting at runtime. Addresses maintainer feedback on PR diegosouzapw#10539 Signed-off-by: Minxi Hou <houminxi@gmail.com>
ProxyEgress and AUTH logs exposed client IPs, egress IPs, and account prefixes at info level — a privacy leak in multi-tenant/shared-log environments. Now redacted by default, only shown when debugMode=true. Fixes diegosouzapw#10348
Strict SSE clients (WorkBuddy, etc.) JSON.parse every SSE line and crash on comment lines. Changed OMNIROUTE_SSE_COMMENTS default from enabled to disabled. Operators can opt in with OMNIROUTE_SSE_COMMENTS=on. Fixes diegosouzapw#10524
…t debugMode The proxy-log redaction half of diegosouzapw#10348 is superseded by an already-merged fix (PROXY_LOG_INCLUDE_IPS, decoupled from debugMode). The remaining gap was the chat.ts AUTH log line ("Using <provider> account: <prefix>..."), which this PR gated on the broad `debugMode` setting. `debugMode` is a general dashboard-visibility toggle unrelated to log privacy — coupling redaction to it means any future, unrelated change to debugMode's default silently changes whether account prefixes leak into logs. Add a dedicated AUTH_LOG_INCLUDE_ACCOUNT_ID feature flag (default off, security category) and gate the AUTH log line on it via isFeatureFlagEnabled(), which reads the DB override synchronously on every call (no stale in-memory cache to invalidate) and fails safe to redacted on any lookup error. Also update the SSE-comments tests/docs that still asserted the old enabled-by-default behavior (tests/unit/sseHeartbeat.test.ts, tests/unit/sse-comments-optout-9305.test.ts, docs/reference/ENVIRONMENT.md) to match the new default-off behavior from this PR's earlier commit. Refs diegosouzapw#10348, diegosouzapw#10524 Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
diegosouzapw
force-pushed
the
review/log-sse-defaults
branch
from
August 18, 2026 00:14
59d5ef1 to
700cb1d
Compare
This was referenced Aug 19, 2026
diegosouzapw
added a commit
that referenced
this pull request
Aug 19, 2026
… a locale gap (#10704) * fix(tests): drain three base-reds left by the SSE-comment default and a locale gap All three reproduce on a pristine tip; none is caused by the branch that found them. 1. i18n vi — six keys landed in en.json without a Vietnamese counterpart (settings.reasoningTokenBuffer*, settings.zeroLatencyOptimizations*, settings.compressionOutputStyle.i-have-adhd.*). The vi locale is held to strict parity, so the whole i18n-vi suite went red. Translated; no existing key reordered. 2. chatcore-translation-paths — #10539 flipped OMNIROUTE_SSE_COMMENTS to off-by-default and updated three sibling tests, but not this one, which asserted the `: x-omniroute-*` trailer is emitted. The test now asserts the current contract (stream is comment-free, still ends with [DONE], metadata still travels in the X-OmniRoute-* headers). The opt-in half stays covered by sse-comments-optout-9305.test.ts, which drives the env var through all three states. Enabling the flag inside this file instead leaks process.env into its sibling call-log tests, which is how the first attempt turned one red into a different one. 3. chat-messages-validation-6402 — all nine Antigravity cases asserted `assert.match(body, /ok/)` against the mocked model output. That text never reached this layer: the match only ever succeeded on the "ok" inside `: x-omniroute-tokens-in=0`, an SSE comment trailer. When the trailers stopped being emitted the coincidence broke, not the behavior — bisected to 6b823aa, whose parent 6d99a46 passes. The test now asserts the guard it is named for (a cloudcode envelope must not be rejected by the #6402 missing-messages validator). Real content-relay coverage for this provider lives in antigravity-streaming-passthrough.test.ts, which passes. Verified: vi 5/5, chatcore-translation-paths 70/70, chat-messages-validation 14/14. * chore(changelog): correct the fragment to the real PR number (#10704) --------- Co-authored-by: Xiangzhe <bakryun0718@proton.me>
HouMinXi
added a commit
to HouMinXi/OmniRoute
that referenced
this pull request
Aug 20, 2026
…s to disabled (diegosouzapw#10539) * fix(logging): redact client IPs and account prefixes by default ProxyEgress and AUTH logs exposed client IPs, egress IPs, and account prefixes at info level — a privacy leak in multi-tenant/shared-log environments. Now redacted by default, only shown when debugMode=true. Fixes diegosouzapw#10348 * fix(sse): default SSE comment lines to disabled Strict SSE clients (WorkBuddy, etc.) JSON.parse every SSE line and crash on comment lines. Changed OMNIROUTE_SSE_COMMENTS default from enabled to disabled. Operators can opt in with OMNIROUTE_SSE_COMMENTS=on. Fixes diegosouzapw#10524 * fix(logging): gate AUTH account-prefix redaction on a narrow flag, not debugMode The proxy-log redaction half of diegosouzapw#10348 is superseded by an already-merged fix (PROXY_LOG_INCLUDE_IPS, decoupled from debugMode). The remaining gap was the chat.ts AUTH log line ("Using <provider> account: <prefix>..."), which this PR gated on the broad `debugMode` setting. `debugMode` is a general dashboard-visibility toggle unrelated to log privacy — coupling redaction to it means any future, unrelated change to debugMode's default silently changes whether account prefixes leak into logs. Add a dedicated AUTH_LOG_INCLUDE_ACCOUNT_ID feature flag (default off, security category) and gate the AUTH log line on it via isFeatureFlagEnabled(), which reads the DB override synchronously on every call (no stale in-memory cache to invalidate) and fails safe to redacted on any lookup error. Also update the SSE-comments tests/docs that still asserted the old enabled-by-default behavior (tests/unit/sseHeartbeat.test.ts, tests/unit/sse-comments-optout-9305.test.ts, docs/reference/ENVIRONMENT.md) to match the new default-off behavior from this PR's earlier commit. Refs diegosouzapw#10348, diegosouzapw#10524 Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
giauphan
pushed a commit
to giauphan/OmniRoute
that referenced
this pull request
Aug 20, 2026
…s to disabled (diegosouzapw#10539) * fix(logging): redact client IPs and account prefixes by default ProxyEgress and AUTH logs exposed client IPs, egress IPs, and account prefixes at info level — a privacy leak in multi-tenant/shared-log environments. Now redacted by default, only shown when debugMode=true. Fixes diegosouzapw#10348 * fix(sse): default SSE comment lines to disabled Strict SSE clients (WorkBuddy, etc.) JSON.parse every SSE line and crash on comment lines. Changed OMNIROUTE_SSE_COMMENTS default from enabled to disabled. Operators can opt in with OMNIROUTE_SSE_COMMENTS=on. Fixes diegosouzapw#10524 * fix(logging): gate AUTH account-prefix redaction on a narrow flag, not debugMode The proxy-log redaction half of diegosouzapw#10348 is superseded by an already-merged fix (PROXY_LOG_INCLUDE_IPS, decoupled from debugMode). The remaining gap was the chat.ts AUTH log line ("Using <provider> account: <prefix>..."), which this PR gated on the broad `debugMode` setting. `debugMode` is a general dashboard-visibility toggle unrelated to log privacy — coupling redaction to it means any future, unrelated change to debugMode's default silently changes whether account prefixes leak into logs. Add a dedicated AUTH_LOG_INCLUDE_ACCOUNT_ID feature flag (default off, security category) and gate the AUTH log line on it via isFeatureFlagEnabled(), which reads the DB override synchronously on every call (no stale in-memory cache to invalidate) and fails safe to redacted on any lookup error. Also update the SSE-comments tests/docs that still asserted the old enabled-by-default behavior (tests/unit/sseHeartbeat.test.ts, tests/unit/sse-comments-optout-9305.test.ts, docs/reference/ENVIRONMENT.md) to match the new default-off behavior from this PR's earlier commit. Refs diegosouzapw#10348, diegosouzapw#10524 Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
giauphan
pushed a commit
to giauphan/OmniRoute
that referenced
this pull request
Aug 20, 2026
… a locale gap (diegosouzapw#10704) * fix(tests): drain three base-reds left by the SSE-comment default and a locale gap All three reproduce on a pristine tip; none is caused by the branch that found them. 1. i18n vi — six keys landed in en.json without a Vietnamese counterpart (settings.reasoningTokenBuffer*, settings.zeroLatencyOptimizations*, settings.compressionOutputStyle.i-have-adhd.*). The vi locale is held to strict parity, so the whole i18n-vi suite went red. Translated; no existing key reordered. 2. chatcore-translation-paths — diegosouzapw#10539 flipped OMNIROUTE_SSE_COMMENTS to off-by-default and updated three sibling tests, but not this one, which asserted the `: x-omniroute-*` trailer is emitted. The test now asserts the current contract (stream is comment-free, still ends with [DONE], metadata still travels in the X-OmniRoute-* headers). The opt-in half stays covered by sse-comments-optout-9305.test.ts, which drives the env var through all three states. Enabling the flag inside this file instead leaks process.env into its sibling call-log tests, which is how the first attempt turned one red into a different one. 3. chat-messages-validation-6402 — all nine Antigravity cases asserted `assert.match(body, /ok/)` against the mocked model output. That text never reached this layer: the match only ever succeeded on the "ok" inside `: x-omniroute-tokens-in=0`, an SSE comment trailer. When the trailers stopped being emitted the coincidence broke, not the behavior — bisected to 6b823aa, whose parent 6d99a46 passes. The test now asserts the guard it is named for (a cloudcode envelope must not be rejected by the diegosouzapw#6402 missing-messages validator). Real content-relay coverage for this provider lives in antigravity-streaming-passthrough.test.ts, which passes. Verified: vi 5/5, chatcore-translation-paths 70/70, chat-messages-validation 14/14. * chore(changelog): correct the fragment to the real PR number (diegosouzapw#10704) --------- Co-authored-by: Xiangzhe <bakryun0718@proton.me>
Merged
5 tasks
diegosouzapw
pushed a commit
that referenced
this pull request
Aug 20, 2026
…and the egress API (#10876) Obrigado — expõe onde o operador realmente olha (sweep periódico de saúde de proxy + resposta da API de egress) o sinal de compartilhamento anônimo de IP de egress entre contas de um mesmo rotation group, que já existia (analyzeEgressSharing) mas só era acessível via curl autenticado. Fecha #10677. Respeita a decisão de redação do #10348/#10539: apenas contagens por padrão, nenhum IP/identidade de conta a menos que PROXY_LOG_INCLUDE_IPS=true. Validação (worktree combinado a partir de origin/release/v3.8.50, 0 conflitos): - typecheck:core limpo, complexity/cognitive-complexity dentro do baseline - tests/unit/proxy-egress-route-summary.test.ts + proxy-egress-summary.test.ts + proxy-health-egress-line.test.ts — 23/23 passando (agregado, formatter, linha do sweep com output real capturado, rota completa com auth de management)
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…s to disabled (diegosouzapw#10539) * fix(logging): redact client IPs and account prefixes by default ProxyEgress and AUTH logs exposed client IPs, egress IPs, and account prefixes at info level — a privacy leak in multi-tenant/shared-log environments. Now redacted by default, only shown when debugMode=true. Fixes diegosouzapw#10348 * fix(sse): default SSE comment lines to disabled Strict SSE clients (WorkBuddy, etc.) JSON.parse every SSE line and crash on comment lines. Changed OMNIROUTE_SSE_COMMENTS default from enabled to disabled. Operators can opt in with OMNIROUTE_SSE_COMMENTS=on. Fixes diegosouzapw#10524 * fix(logging): gate AUTH account-prefix redaction on a narrow flag, not debugMode The proxy-log redaction half of diegosouzapw#10348 is superseded by an already-merged fix (PROXY_LOG_INCLUDE_IPS, decoupled from debugMode). The remaining gap was the chat.ts AUTH log line ("Using <provider> account: <prefix>..."), which this PR gated on the broad `debugMode` setting. `debugMode` is a general dashboard-visibility toggle unrelated to log privacy — coupling redaction to it means any future, unrelated change to debugMode's default silently changes whether account prefixes leak into logs. Add a dedicated AUTH_LOG_INCLUDE_ACCOUNT_ID feature flag (default off, security category) and gate the AUTH log line on it via isFeatureFlagEnabled(), which reads the DB override synchronously on every call (no stale in-memory cache to invalidate) and fails safe to redacted on any lookup error. Also update the SSE-comments tests/docs that still asserted the old enabled-by-default behavior (tests/unit/sseHeartbeat.test.ts, tests/unit/sse-comments-optout-9305.test.ts, docs/reference/ENVIRONMENT.md) to match the new default-off behavior from this PR's earlier commit. Refs diegosouzapw#10348, diegosouzapw#10524 Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
… a locale gap (diegosouzapw#10704) * fix(tests): drain three base-reds left by the SSE-comment default and a locale gap All three reproduce on a pristine tip; none is caused by the branch that found them. 1. i18n vi — six keys landed in en.json without a Vietnamese counterpart (settings.reasoningTokenBuffer*, settings.zeroLatencyOptimizations*, settings.compressionOutputStyle.i-have-adhd.*). The vi locale is held to strict parity, so the whole i18n-vi suite went red. Translated; no existing key reordered. 2. chatcore-translation-paths — diegosouzapw#10539 flipped OMNIROUTE_SSE_COMMENTS to off-by-default and updated three sibling tests, but not this one, which asserted the `: x-omniroute-*` trailer is emitted. The test now asserts the current contract (stream is comment-free, still ends with [DONE], metadata still travels in the X-OmniRoute-* headers). The opt-in half stays covered by sse-comments-optout-9305.test.ts, which drives the env var through all three states. Enabling the flag inside this file instead leaks process.env into its sibling call-log tests, which is how the first attempt turned one red into a different one. 3. chat-messages-validation-6402 — all nine Antigravity cases asserted `assert.match(body, /ok/)` against the mocked model output. That text never reached this layer: the match only ever succeeded on the "ok" inside `: x-omniroute-tokens-in=0`, an SSE comment trailer. When the trailers stopped being emitted the coincidence broke, not the behavior — bisected to d5c4a54, whose parent 639f810 passes. The test now asserts the guard it is named for (a cloudcode envelope must not be rejected by the diegosouzapw#6402 missing-messages validator). Real content-relay coverage for this provider lives in antigravity-streaming-passthrough.test.ts, which passes. Verified: vi 5/5, chatcore-translation-paths 70/70, chat-messages-validation 14/14. * chore(changelog): correct the fragment to the real PR number (diegosouzapw#10704) --------- Co-authored-by: Xiangzhe <bakryun0718@proton.me>
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…and the egress API (diegosouzapw#10876) Obrigado — expõe onde o operador realmente olha (sweep periódico de saúde de proxy + resposta da API de egress) o sinal de compartilhamento anônimo de IP de egress entre contas de um mesmo rotation group, que já existia (analyzeEgressSharing) mas só era acessível via curl autenticado. Fecha diegosouzapw#10677. Respeita a decisão de redação do diegosouzapw#10348/diegosouzapw#10539: apenas contagens por padrão, nenhum IP/identidade de conta a menos que PROXY_LOG_INCLUDE_IPS=true. Validação (worktree combinado a partir de origin/release/v3.8.50, 0 conflitos): - typecheck:core limpo, complexity/cognitive-complexity dentro do baseline - tests/unit/proxy-egress-route-summary.test.ts + proxy-egress-summary.test.ts + proxy-health-egress-line.test.ts — 23/23 passando (agregado, formatter, linha do sweep com output real capturado, rota completa com auth de management)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two privacy/compatibility fixes from issue triage:
1. #10348: Redact client IPs and account prefixes in default logs
ProxyEgress and AUTH logs exposed client IPs, egress IPs, and account prefixes at info level — a privacy leak in multi-tenant/shared-log environments. Now redacted by default, only shown when
debugMode=true.Changes:
src/lib/proxyLogger.ts: cachedebugModefrom settings, redactclientIp/egressIp/accountin ProxyEgress log linesrc/sse/handlers/chat.ts: redact account prefix in "Using provider account" log line2. #10524: Default SSE comment lines to disabled
Strict SSE clients (WorkBuddy, etc.) JSON.parse every SSE line and crash on
: x-omniroute-*comment lines. ChangedOMNIROUTE_SSE_COMMENTSdefault from enabled to disabled. Operators can opt in withOMNIROUTE_SSE_COMMENTS=on.Changes:
open-sse/utils/sseHeartbeat.ts:sseCommentsEnabled()defaults tofalse, acceptson/true/1/yesto enabletests/unit/sse-comments-default-10524.test.ts: 3 tests (default off, explicit on, explicit off)Verification:
npm run typecheck:coreclean