Skip to content

fix: body dumps off by default (ACP_DUMP_BODY) + reuse log-mask in dumps (#276) - #278

Merged
ranxianglei merged 7 commits into
masterfrom
2026-08-27_disable-default-body-dumps
Aug 27, 2026
Merged

fix: body dumps off by default (ACP_DUMP_BODY) + reuse log-mask in dumps (#276)#278
ranxianglei merged 7 commits into
masterfrom
2026-08-27_disable-default-body-dumps

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Part B of #255, split out of PR #274 (which covered logs only). Stacked on 2026-08-27_mask-sensitive-logs — depends on src/log-mask.ts, so rebase to master once #274 lands.

Changes

1. Body dumps off by default — every plaintext body dump was gated on --debug, and the launchers hardcode debug: true (src/launcher.ts:1035), so every bili <client> user silently wrote full conversation bodies to disk. All body dumps are now opt-in via ACP_DUMP_BODY=1, decoupled from --debug:

  • dumps/req-*.json (src/server.ts)
  • raw/*-REQ.txt / raw/*-RES.txt (src/server.ts)
  • raw/*-INCOMING.txt (src/server.ts)
  • req-*-REREQUEST.json (src/loop/core.ts)

--debug / ACP_DEBUG=1 now only produces verbose (already-masked, per #255) logs — no body dumps. Per #255 floor 5 the launcher keeps debug: true for troubleshooting; the fix is that dumps no longer follow it.

2. Dump header filtering reuses maskHeadersForLog (src/log-mask.ts) — the old /key|auth|token/i regex missed cookie / set-cookie; the shared rule covers them plus non-public hosts.

3. Non-public hosts masked in dumpsupstreamUrl in raw/*-REQ.txt via maskUrlForLog; the incoming path in raw/*-INCOMING.txt via maskUrlsInText (keeps the path's debug value, masks the embedded host).

4. INCOMING dump honors ACP_RAW_DUMP_DIR (was hardcoded to stateDir()/raw).

5. Teststests/dump-mask.test.ts: dumps off by default under --debug; with ACP_DUMP_BODY=1 dumps carry no credentials (authorization / x-api-key / cookie) and no non-public host; dumps still work without --debug.

Verification

  • npm run typecheck — clean
  • npm test — 671 pass (668 + 3 new)
  • npm run build — success

Response-body SSE dump (ACP_DUMP_SSE) is untouched — already opt-in.

ework-agent added 3 commits August 27, 2026 09:40
bili.log and the launcher tmp log (bili-proxy-${port}.log) received
plaintext secrets and non-public upstream endpoints:

- 'forward GET/POST -> <upstreamUrl>' logged the full upstream URL on
  every request (always-on, not debug-gated)
- '-> upstream headers:' / '<- upstream response headers:' logged
  authorization / x-api-key / cookie / set-cookie values verbatim
- formatUpstreamError embedded the raw upstream URL and the failing
  host (ECONNREFUSED 192.168.x.x, ENOTFOUND relay.internal)
- mitm/CONNECT logs exposed tunneled upstream host:port

New src/log-mask.ts:
- credential headers (key|auth|token|cookie) -> '<masked N chars>'
- non-public API hosts -> '<private-host>' (port kept); well-known
  public endpoints (openai, chatgpt, anthropic, deepseek, googleapis,
  azure, mistral, groq, ...) stay verbatim for debuggability
- userinfo/query/hash stripped from logged URLs (key-leak vectors)
- error message/address fields scrubbed of non-public hostnames

debug: true in the launcher is intentionally kept (per #255 triage);
dump-file gating and GC are tracked in separate issues.
tunnelThrough logged err.message verbatim; OS/undici error text embeds
the endpoint ("connect ECONNREFUSED 10.0.0.5:8443", "getaddrinfo
ENOTFOUND relay.internal"), re-leaking the host the template just
masked. Add maskHostInText() to log-mask.ts and apply it to the two
tunnel error log lines; formatUpstreamError now reuses the same helper
(also fixes the bracketed/bare IPv6 mismatch in its scrub).

+2 tests: unit coverage of maskHostInText, e2e CONNECT tunnel failure
asserting no raw target host in any captured log line.
…mps (#276)

- Decouple all plaintext body dumps (dumps/req-*.json, raw/*-REQ.txt,
  raw/*-RES.txt, raw/*-INCOMING.txt, req-*-REREQUEST.json) from --debug:
  they are now opt-in via ACP_DUMP_BODY=1, so `bili <client>` users
  (launcher hardcodes debug:true) no longer leak conversation bodies to
  disk by default.
- Reuse maskHeadersForLog from src/log-mask.ts for dump header filtering:
  now covers cookie/set-cookie (the old /key|auth|token/i regex missed
  them) and non-public hosts.
- Mask the upstream URL in raw/*-REQ.txt (maskUrlForLog) and the incoming
  path in raw/*-INCOMING.txt (maskUrlsInText) so non-public hosts don't
  land in dump files.
- INCOMING dump now honors ACP_RAW_DUMP_DIR (was hardcoded to stateDir/raw).
- Add tests/dump-mask.test.ts: dumps off by default under --debug; with
  ACP_DUMP_BODY=1 dumps carry no credentials and no non-public host; dumps
  work without --debug.
@ranxianglei

Copy link
Copy Markdown
Owner Author

[bot] Reviewed 2026-08-27_disable-default-body-dumps (3060764). All five changes verified in the diff, and the local pre-flight is green:

  • npm run typecheck — clean
  • npm test — 671/671 pass (matches the 668 + 3 new claim)
  • npm run build — success

Change-by-change verification:

  1. Dumps off by defaultbodyDumpEnabled() (src/server.ts:72) gates all four dump sites: dumps/req-*.json (src/server.ts:1636), raw/*-REQ.txt (src/server.ts:1690), raw/*-RES.txt (src/server.ts:1741), raw/*-INCOMING.txt (src/server.ts:657-667), plus req-*-REREQUEST.json (src/loop/core.ts:450). The [debug] log lines (INCOMING summary, tools=, RAW path notices) stay under --debug; launcher debug: true (src/launcher.ts:1035) untouched. I grepped every writeFileSync/ACP_DUMP* site in src/ — no other plaintext body dump exists, and ACP_DUMP_SSE (src/server.ts:1894) is indeed untouched/opt-in.
  2. Header masking reuse — both raw dumps and the INCOMING dump now go through maskHeadersForLog, so cookie/set-cookie (missed by the old /key|auth|token/i regex) and non-public host headers are covered.
  3. Host maskingmaskUrlForLog(upstreamUrl) on the raw REQ line, maskUrlsInText(req.url) on INCOMING (path kept, embedded host masked). Confirmed by the new tests asserting <private-host> present and no 127.0.0.1:<port> anywhere in any dump.
  4. ACP_RAW_DUMP_DIR honored by the INCOMING dump (src/server.ts:659).
  5. Teststests/dump-mask.test.ts covers off-by-default under --debug, credential/host scrubbing when on, and decoupling from --debug.

Stacking note (expected, no action needed now): the branch is based on 1e5a0f9 only — it's missing 39be0fe (tunnel CONNECT scrub) from #274. I dry-ran the rebase onto the full 2026-08-27_mask-sensitive-logs branch: applies cleanly (no file overlap), and the rebased tree passes 673/673. So once #274 lands, rebase to master and this is good to merge.

One minor observation (non-blocking): ACP_DUMP_REQ=0 still suppresses dumps/req-*.json (src/server.ts:1636) but no longer affects the REREQUEST dump in src/loop/core.ts — that one is now controlled solely by ACP_DUMP_BODY. Slight semantic drift in the legacy knob; fine if intentional (it's an undocumented env anyway).

🤖 ework agent · vllm-qwen/qwen3.8-27b

@ranxianglei
ranxianglei changed the base branch from 2026-08-27_mask-sensitive-logs to master August 27, 2026 14:10
@github-actions

Copy link
Copy Markdown

📦 Built Package Artifact

Branch: 2026-08-27_disable-default-body-dumps (fdc8723)

Option A — Install from npm PR tag (recommended)

npm install -g billion-context@pr-278

Each push to this PR publishes a new version under the pr-278 npm tag.

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf billion-context-pr278.tgz
npm install -g package

This comment is automatically updated on each push.

@ranxianglei
ranxianglei merged commit 181918e into master Aug 27, 2026
6 checks passed
@ranxianglei ranxianglei mentioned this pull request Aug 27, 2026
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