Skip to content

Commit e8a5b92

Browse files
etrclaude
andcommitted
Merge TASK-074: gate DEBUG raw-body printing behind explicit opt-in
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2 parents 1605f1a + d537c8f commit e8a5b92

11 files changed

Lines changed: 725 additions & 19 deletions

File tree

docs/debug-env-vars.md

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
# Debug-only environment variables
2+
3+
This document inventories environment variables that the libhttpserver
4+
runtime reads. **None of these are meant to be set in production.**
5+
All are silent unless explicitly enabled; the library emits a one-shot
6+
`SECURITY WARNING` to stderr (and to the user's `log_error` callback,
7+
when wired) the first time `webserver::start()` observes a set
8+
variable.
9+
10+
---
11+
12+
## `LIBHTTPSERVER_DEBUG_DUMP_REQUEST_BODY`
13+
14+
| Field | Value |
15+
| --- | --- |
16+
| Introduced | TASK-074 (June 2026) |
17+
| Replaces | The prior compile-time `#ifdef DEBUG` guard in `src/detail/webserver_body_pipeline.cpp` |
18+
| Read by | `httpserver::detail::debug_dump_request_body_opted_in()` (cached in a function-local static) |
19+
| Independent of | Compile-time `DEBUG` / `NDEBUG`. The runtime check is the only gate. |
20+
21+
### Effect
22+
23+
When set to any non-empty, non-`"0"` value, every inbound request body
24+
chunk is written verbatim to `stdout`, prefixed by `Writing content: `,
25+
from the body pipeline in `src/detail/webserver_body_pipeline.cpp`.
26+
27+
### Risk
28+
29+
Raw request bodies routinely contain:
30+
- Basic-auth credentials (when sent in the body rather than the header),
31+
- form-posted passwords (`application/x-www-form-urlencoded` with a
32+
`password=` field),
33+
- session cookies,
34+
- CSRF tokens,
35+
- PII (email addresses, names, IDs, addresses).
36+
37+
The `http_request::operator<<` redaction policy introduced in TASK-057
38+
does **not** cover this code path -- bytes are written verbatim. Treat
39+
any process where this variable is set as a credential-leaking
40+
surface (CWE-532: Insertion of Sensitive Information into Log File).
41+
42+
### Startup signal
43+
44+
The library emits a single `SECURITY WARNING` line to stderr at the
45+
first `webserver::start()` call after the variable is observed. The
46+
exact line is:
47+
48+
```
49+
[libhttpserver] SECURITY WARNING: LIBHTTPSERVER_DEBUG_DUMP_REQUEST_BODY is set. Raw request bodies will be written to stdout, including credentials, session cookies, and PII. Unset the variable for production deployments. See docs/debug-env-vars.md.
50+
```
51+
52+
The same line is forwarded to the owning webserver's `log_error`
53+
callback when one is wired via `create_webserver().log_error(...)`. The
54+
forwarding is best-effort: an exception from the callback is swallowed
55+
so a misconfigured logger cannot abort daemon startup.
56+
57+
Multiple `webserver` instances constructed in the same process share a
58+
process-wide print-once flag, so the warning is emitted exactly once
59+
per process.
60+
61+
### How to disable
62+
63+
```sh
64+
unset LIBHTTPSERVER_DEBUG_DUMP_REQUEST_BODY
65+
```
66+
67+
then restart the process. The value is cached in a function-local
68+
static on first request, so a mid-process `unsetenv()` has no effect.
69+
70+
### Spot-check: release build with `-DDEBUG` accidentally set
71+
72+
The runtime check is the only gate; the env var must be explicitly set
73+
for the dump to fire, regardless of the compile-time `DEBUG` /
74+
`NDEBUG` posture. A release binary built with `-DDEBUG` accidentally
75+
defined therefore still does not leak credentials/PII unless the
76+
operator opted in.
77+
78+
---
79+
80+
## Adding new debug env vars
81+
82+
When introducing a new `LIBHTTPSERVER_*` debug environment variable:
83+
84+
1. Read it once via a function-local static (cached, race-free C++
85+
memory model semantics).
86+
2. Add a one-shot stderr warning in `webserver::start()` naming the
87+
risk explicitly.
88+
3. Document the variable, its risk, and its startup signal here.
89+
4. Cross-reference the entry from `specs/architecture/10-observability.md`.

specs/architecture/10-observability.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,6 @@ The library is a passive provider; callers wire their own logging:
44
- `log_access` callback (already in `create_webserver`): invoked per request with the URI.
55
- `log_error` callback: invoked on internal errors and uncaught handler exceptions.
66
- No metrics or tracing surface added in v2.0.
7+
- Debug-only environment variables (e.g. `LIBHTTPSERVER_DEBUG_DUMP_REQUEST_BODY`): see [`docs/debug-env-vars.md`](../../docs/debug-env-vars.md). None are read on the production code path; all are silent unless explicitly set, and the library emits a one-shot `SECURITY WARNING` to stderr (and to `log_error` when wired) when any of them is detected at `webserver::start()`.
78

89
---

specs/tasks/M7-v2-cleanup/TASK-074.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@
88
`webserver_body_pipeline.cpp:199-208` contains a `#ifdef DEBUG` block that prints the raw request body (including credentials and PII) to stdout. The inline comment explicitly warns it must never be widened to release builds. The risk is contained but the suppression mechanism is fragile (a developer could ship a debug build to production). Make the print opt-in via an explicit env var so the default behaviour even in `DEBUG` builds is safe.
99

1010
**Action Items:**
11-
- [ ] Replace the `#ifdef DEBUG` guard with a runtime check on `getenv("LIBHTTPSERVER_DEBUG_DUMP_REQUEST_BODY")`. The block compiles always but only fires when the env var is set and the build is `DEBUG` (or unconditionally, since prod builds never set the var). Document the env var in `src/httpserver/detail/README.md` (or wherever debug knobs live).
12-
- [ ] Add a warning log at startup if the env var is set, naming the security risk.
13-
- [ ] Update the inline comment to reference the env var rather than `#ifdef DEBUG`.
14-
- [ ] Spot-check that release builds with `-DDEBUG` accidentally set still do not print without the env var.
11+
- [x] Replace the `#ifdef DEBUG` guard with a runtime check on `getenv("LIBHTTPSERVER_DEBUG_DUMP_REQUEST_BODY")`. The block compiles always but only fires when the env var is set; release builds never set the var, so the gate is a one-line runtime check independent of any compile-time `DEBUG` define. Documented in the new `docs/debug-env-vars.md` (a fresh, dedicated home for `LIBHTTPSERVER_*` debug knobs) with a cross-reference from `specs/architecture/10-observability.md`.
12+
- [x] Add a warning log at startup if the env var is set, naming the security risk. Implemented as `httpserver::detail::maybe_warn_debug_dump_request_body()` called from `webserver::start()`. One-shot stderr write (process-wide `std::atomic<bool>` flag) plus best-effort forwarding to `webserver::get_error_logger()` when wired. Message names the env var, the SECURITY WARNING marker, credentials/cookies/PII at risk, and points to `docs/debug-env-vars.md`.
13+
- [x] Update the inline comment to reference the env var rather than `#ifdef DEBUG`. The comment in `webserver_body_pipeline.cpp` now names `LIBHTTPSERVER_DEBUG_DUMP_REQUEST_BODY`, points to `docs/debug-env-vars.md`, and notes that the TASK-057 `operator<<` redaction does not cover this code path.
14+
- [x] Spot-check that release builds with `-DDEBUG` accidentally set still do not print without the env var. Pinned by `test/integ/debug_dump_request_body_unset_test.cpp`, which builds and runs identically on both `--enable-debug` and stock release configurations; the runtime check is the only gate, so the spot-check resolves by construction.
1515

1616
**Dependencies:**
1717
- Blocked by: None
@@ -27,4 +27,4 @@
2727
**Related Requirements:** PRD §2 security NFR (defense-in-depth)
2828
**Related Decisions:** None new
2929

30-
**Status:** Backlog
30+
**Status:** Done

specs/tasks/M7-v2-cleanup/_index.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ TASK-093).
3838
| TASK-071 | Wire `install_not_found_alias_` stub and remove dead `lambda_handler` arm | MED | S | Done |
3939
| TASK-072 | Arena-allocated unescape on the warm path | MED | M | Done |
4040
| TASK-073 | Revisit libmicrohttpd v0.99 unescape workaround | LOW | S | Done |
41-
| TASK-074 | Gate DEBUG raw-body printing behind explicit opt-in | LOW (sec) | S | Backlog |
41+
| TASK-074 | Gate DEBUG raw-body printing behind explicit opt-in | LOW (sec) | S | Done |
4242
| TASK-075 | Propagate `wait_for_server_ready` to sibling hooks integ tests | HIGH | M | Backlog |
4343
| TASK-076 | Replace tautological-pass blocks in TLS test lanes | HIGH | M | Backlog |
4444
| TASK-077 | Restore Windows / Darwin coverage in skipped test suites | HIGH | L | Backlog |

0 commit comments

Comments
 (0)