fix(egress): restart dead daemons and cap inbound scan body size #466

2026-07-23T18:49:55-04:00

didericis-claude commented

2026-07-23 18:49:55 -04:00

Summary

Fixes #455 — the egress gateway OOM that leaves every bottle on a host without egress.

Two complementary fixes:

Auto-restart dead daemons (gateway_init.py): the supervisor already had restart_daemon() and request_restart() logic; this wires it into tick() so any unexpectedly dead daemon (e.g. an OOM-killed mitmdump) is respawned automatically without operator intervention. Implements the "eventual" failure policy described in the original module docstring.
Cap inbound scan body size (egress_addon.py): response bytes passed to the DLP inbound scan are capped at EGRESS_INBOUND_SCAN_LIMIT_BYTES (default 1 MiB). mitmproxy buffers the full response before the hook fires, but capping at scan time limits the additional amplification from decoded text and regex match strings. Bodies over the limit emit an egress_scan_truncated log event so the trade-off is visible. Set to 0 to disable.

Security trade-off

As the research note documents: the scan cap is a known trade-off — content beyond the threshold is not scanned. The restart fix closes the availability gap regardless.

Test plan

test_gateway_init.py: updated test_child_crash_does_not_initiate_shutdown → test_child_crash_triggers_restart_not_shutdown; verifies crasher gets a new PID after death
test_egress_addon_request_flow.py: added TestInboundScanLimitEnv (env-var parsing) and TestInboundBodyScanCap (truncation behavior, log event, known blind-spot, cap=0 disables)
108 unit tests pass

## Summary Fixes #455 — the egress gateway OOM that leaves every bottle on a host without egress. Two complementary fixes: - **Auto-restart dead daemons** (`gateway_init.py`): the supervisor already had `restart_daemon()` and `request_restart()` logic; this wires it into `tick()` so any unexpectedly dead daemon (e.g. an OOM-killed `mitmdump`) is respawned automatically without operator intervention. Implements the "eventual" failure policy described in the original module docstring. - **Cap inbound scan body size** (`egress_addon.py`): response bytes passed to the DLP inbound scan are capped at `EGRESS_INBOUND_SCAN_LIMIT_BYTES` (default 1 MiB). mitmproxy buffers the full response before the hook fires, but capping at scan time limits the additional amplification from decoded text and regex match strings. Bodies over the limit emit an `egress_scan_truncated` log event so the trade-off is visible. Set to `0` to disable. ## Security trade-off As the research note documents: the scan cap is a known trade-off — content beyond the threshold is not scanned. The restart fix closes the availability gap regardless. ## Test plan - [x] `test_gateway_init.py`: updated `test_child_crash_does_not_initiate_shutdown` → `test_child_crash_triggers_restart_not_shutdown`; verifies crasher gets a new PID after death - [x] `test_egress_addon_request_flow.py`: added `TestInboundScanLimitEnv` (env-var parsing) and `TestInboundBodyScanCap` (truncation behavior, log event, known blind-spot, cap=0 disables) - [x] 108 unit tests pass

didericis-codex requested changes 2026-07-23 18:52:53 -04:00

didericis-codex left a comment

Blocking: tick() queues unexpected-death restarts for the next polling cycle, but then computes done from the still-dead processes and returns True in the current cycle. main() consequently exits before _drain_restart_requests() runs again whenever the crashed child was the only daemon (a supported BOT_BOTTLE_GATEWAY_DAEMONS=egress configuration) or all children happen to be dead. I reproduced this with a one-element _Supervisor: after the child exits, the first tick returns True, retains the original PID, and leaves {'crasher'} in _restart_requested. This means the advertised auto-recovery does not work in that configuration. Please drain newly queued restarts before deciding completion, or make done false while restart requests are pending, and add a single-daemon crash regression test.

I could not run the full pytest selection because pytest is not installed in the review environment; the stdlib-only reproducer directly exercised the PR's _Supervisor implementation.

Blocking: `tick()` queues unexpected-death restarts for the next polling cycle, but then computes `done` from the still-dead processes and returns `True` in the current cycle. `main()` consequently exits before `_drain_restart_requests()` runs again whenever the crashed child was the only daemon (a supported `BOT_BOTTLE_GATEWAY_DAEMONS=egress` configuration) or all children happen to be dead. I reproduced this with a one-element `_Supervisor`: after the child exits, the first tick returns `True`, retains the original PID, and leaves `{'crasher'}` in `_restart_requested`. This means the advertised auto-recovery does not work in that configuration. Please drain newly queued restarts before deciding completion, or make `done` false while restart requests are pending, and add a single-daemon crash regression test. I could not run the full pytest selection because pytest is not installed in the review environment; the stdlib-only reproducer directly exercised the PR's `_Supervisor` implementation.

didericis-codex reviewed 2026-07-23 18:57:06 -04:00

didericis-codex left a comment

Re-reviewed the updated head. The blocking lifecycle issue is fixed: deaths discovered during tick() are restarted before completion is evaluated, so single-daemon and simultaneous all-daemon crashes no longer make the supervisor exit with a queued restart. Regression coverage now exercises both cases.

Validation: python3 -m unittest tests.unit.test_gateway_init -q passes all 31 supervisor tests against the exact published head. Lint is green. The broader remote unit job currently reports 10 unrelated test_cli_start_headless PTY-precondition errors; the changed supervisor test module passes locally and the CI failures do not touch this PR's code paths. No remaining blocking findings.

Re-reviewed the updated head. The blocking lifecycle issue is fixed: deaths discovered during `tick()` are restarted before completion is evaluated, so single-daemon and simultaneous all-daemon crashes no longer make the supervisor exit with a queued restart. Regression coverage now exercises both cases. Validation: `python3 -m unittest tests.unit.test_gateway_init -q` passes all 31 supervisor tests against the exact published head. Lint is green. The broader remote unit job currently reports 10 unrelated `test_cli_start_headless` PTY-precondition errors; the changed supervisor test module passes locally and the CI failures do not touch this PR's code paths. No remaining blocking findings.

didericis-codex reviewed 2026-07-23 18:57:43 -04:00

didericis-codex left a comment

Re-reviewed the updated head. The blocking restart lifecycle issue is fixed and covered by single-daemon and simultaneous-crash regression tests. All 31 gateway supervisor tests pass against the published head; lint is green. The broader remote unit failure is limited to 10 unrelated headless-CLI PTY-precondition tests. No remaining blocking findings. (Gitea will not record my approval because I contributed the fix commits, so this is submitted as a non-blocking review.)

didericis approved these changes 2026-07-23 19:13:38 -04:00

didericis added 3 commits 2026-07-23 19:33:06 -04:00

fix(egress): restart dead daemons and cap inbound scan body size (#455 ) 1fd79dda9a

Two complementary fixes for the egress gateway OOM:

1. gateway_init: auto-restart any daemon that dies unexpectedly. The
   supervisor already had restart_daemon()/request_restart() logic; this
   wires it into tick() so an OOM-killed mitmdump is respawned without
   operator intervention. Implements the "eventual" failure policy
   described in the original module docstring.

2. egress_addon: cap response body bytes passed to the DLP inbound scan
   at EGRESS_INBOUND_SCAN_LIMIT_BYTES (default 1 MiB). mitmproxy buffers
   the full response before the hook fires; capping at scan time limits
   the additional amplification from decoded text and regex strings. Bodies
   over the limit emit an egress_scan_truncated log event. Set to 0 to
   disable the cap.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

fix(gateway): restart dead children before completion check 5ded28896d

test(gateway): cover single and simultaneous daemon crashes

tracker-policy-pr / check-pr (pull_request) Successful in 12s

Details

test / integration-docker (pull_request) Successful in 34s

Details

test / unit (pull_request) Successful in 45s

Details

lint / lint (push) Successful in 53s

Details

test / integration-firecracker (pull_request) Successful in 3m13s

Details

test / coverage (pull_request) Successful in 17s

Details

test / publish-infra (pull_request) Has been skipped

Details

2879dc34cc

didericis force-pushed fix/egress-oom-455 from cd56705177 to 2879dc34cc

2026-07-23 19:33:06 -04:00

Compare

didericis merged commit f24ae45d13 into main

2026-07-23 20:15:21 -04:00

didericis deleted branch fix/egress-oom-455

2026-07-23 20:15:25 -04:00

Sign in to join this conversation.

3 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: didericis/bot-bottle#466