Skip to content

fix(egress-gateway): start proxy with explicit arguments - #16

Draft
MakerFlyFly wants to merge 5 commits into
audit/h11-basefrom
fix/egress-gateway-cli
Draft

MakerFlyFly wants to merge 5 commits into
audit/h11-basefrom
fix/egress-gateway-cli

Conversation

@MakerFlyFly

@MakerFlyFly MakerFlyFly commented Oct 22, 2026 •

Copy link
Copy Markdown
Owner

Summary

The documented gateway command fails before startup with locked proxy.py 2.4.10 because it passes a positional list to proxy.main(**opts).
Use Proxy(input_args=...) with the zero-argument wait loop supported by the declared >=2.4,<3 dependency range.

  • Preserve CLI arguments, the default bind address, the allowlist policy, and lifecycle cleanup.
  • Isolate CLI test data/cache directories and reject creation of the real user proxy directory.
  • Check help in a fresh interpreter with proxy.py blocked, and defer the parent package's existing host_allowed export so this works for the complete import path.
  • Cover RuntimeError, SystemExit(0), and KeyboardInterrupt cleanup, preserving each exception's behavior.
  • Keep the README focused on setup, configuration, package-scoped automated tests, and manual allowed/denied requests.
  • Assert the default forward-proxy service configuration and document the trust required for additional proxy.py arguments.

Type of change

  • Skill change
  • Tool / bridge contract
  • Python package
  • Groovy reference implementation
  • Cross-cutting
  • Documentation
  • Project template
  • CI / dev loop

Review dispositions

Feedback Decision and reason
Old versions reject the wait-loop argument Accepted: real 2.4.0–2.4.3 installations reproduce it; use a zero-argument call and a strict zero-argument test double. Dependency bounds and lockfile are unchanged.
Tests create the real proxy cache directory Accepted: a write guard rejects the original four startup cases. Shared temporary data/cache arguments now keep these writes isolated.
Help test misses module-level imports Accepted: fresh-process regression and mutation checks detect eager imports in both the CLI and its parent package.
Signal-style exits need coverage Partially accepted: SystemExit(0) retains code 0; KeyboardInterrupt must propagate after cleanup rather than being converted to success.
Live-test documentation is ambiguous Accepted: state that live listening is outside CI, and document a manual curl check expecting 403.
Remove SPDX to match neighboring files Not adopted: Magpie's new-file check requires the SPDX identifier in the first ten lines. The existing SPDX plus complete ASF header is retained.

Default service configuration

The default-startup test verifies the service configuration: the HTTP protocol-handler list contains only the forward-proxy handler, and the Web Server and reverse-route plugin lists are empty.
The README identifies the default allowlisting forward proxy as the supported mode and explains that additional proxy.py arguments are passed through without filtering and must come from a trusted operator.
Extra services, plugins, and replacement handlers are outside the allowlist guarantee.
The default-service assertions and documentation do not restrict arguments or expand the allowlist.
Argument restrictions require a separate follow-up review.

The fresh-process test exposed egress_gateway.__init__ importing the proxy dependency through the allowlist module.
The parent package defers its existing export so dependency-free help works through the complete import path.
The public host_allowed export, its function identity, static typing import, and __all__ are preserved.

The confirmed passthrough --hostname override and the --host localhost validation error are recorded for follow-up.
This PR preserves the default binding; it does not impose a new binding policy.
Windows SIGBREAK behavior, partial setup failures, and dynamic-port display remain upstream behavior.

Test plan

  • Windows CPython 3.12.13 and Docker/Linux CPython 3.12.12: all 37 package tests pass with locked proxy.py 2.4.10.
  • Real proxy.py 2.4.0, 2.4.1, 2.4.2, 2.4.3, 2.4.4, and 2.4.10 startup-contract probes pass on both platforms, using the actual parser, plugin loader, and wait loop with socket setup substituted.
  • Commit 87d8cfea reproduces the wait-loop TypeError on all four older versions.
  • The old help test falsely passes eager-import mutants; the new test rejects both CLI and parent-package mutants. Removing the cache isolation flag is rejected by the user-directory guard.
  • Both installed-command and module entrypoints pass live HTTP/CONNECT tests: local allowed traffic returns 200, denied hosts return 403, and the README curl command form returns 403.
  • Linux live smoke tests directly launch the module and installed-command entrypoints; after startup, sending SIGINT or SIGTERM to the gateway process yields exit status 0 and releases workers and listeners. Windows uses the explicitly test-only SIGBREAK bridge in a hidden console.
  • README revision: all six shell examples pass syntax checks. The documented package-directory command runs all 37 tests on Windows and Docker/Linux.
  • Both platforms and both entrypoints pass the documented curl checks: the allowed HTTPS request receives a final HTTP 200 from PyPI, and the denied HTTP request receives HTTP 403 with the expected body.
  • The newly asserted default plugin contract also passes with real proxy.py 2.4.0 using compatible temporary state directories.
  • Default-service mutation checks: both an unexpected reverse-proxy flag and explicitly loaded extra cores pass the test from 9dd73b76 and fail the added assertions.
  • Docker/Linux full-repository prek: all 40 hooks pass on both the preceding head and the revision, including links, workspace ruff/format/mypy, and pytest across all 50 applicable members.
  • Workflow zizmor checks pass on Windows and Linux.
  • Normal commit hooks pass. Magpie pre-first-PR reports ready across all five commits, with zero blocking or advisory items; the PR template reminder was checked against this description.

The original pre-fix source also fails the CLI startup regression with the original positional-argument TypeError; the original policy suite alone passed 29 cases.
Live listening remains a local verification step, separate from the committed CI-safe tests.

CodeQL follow-up for fork audit

The latest commit addresses the two CodeQL quality comments on upstream PR #1774, with changes confined to tests/test_cli.py:

  • py/implicit-string-concatenation-in-list: make the intentional -c script concatenation explicit, preserving the exact script and its single command-line argument.
  • py/import-and-import-from: import the CLI submodule explicitly while preserving the parent-package and star-export identity checks.

Real local CodeQL 2.27.3 with codeql/python-queries 1.8.13 reproduces both findings before the change and reports neither in the target file afterward.
Across the two selected queries, the whole-tree result count changes from 14 to 12; the other 12 results are identical.
No suppression is added.

Windows and Docker/Linux each pass all 37 package tests, Ruff, format checks, and script/import equivalence probes.
The fork baseline's complete Linux prek run passes all 40 hooks, including whole-tree links, and zizmor passes.
Normal commit hooks and Magpie contribution checks also pass.

Incremental commit: b230ea56 — one file, two insertions and two deletions.
This revision remains in this fork for contributor audit.
The upstream branch remains at 172df519feb08db2540817f53a5ac14830ee19c6; its review threads have not been replied to or resolved by this follow-up.

RFC-AI-0004 compliance

  • HITL — no new mutation interface
  • Sandbox — default bind address and allowlist policy are preserved
  • Vendor neutrality — no new vendor-specific workflow or configuration
  • Conversational + correctable — no new adopter setting
  • Write-access discipline — no new outbound action
  • Privacy LLM — no new model route

Linked issues

Refs #15.

Notes for reviewers

This remains a draft for human audit within the contributor's fork.
Its base audit/h11-base is fixed at e6e5c2e36bcab4d0a09d6da5c26e8060caaea3e2.
The commits append to the previously published 87d8cfea, 9dd73b76, 13a6798b, and c70bf873 heads, retaining the prior review references.
The latest incremental change is limited to the two CLI test syntax adjustments described above; the earlier README revision remains unchanged.
Latest documentation commit: c70bf873.
Repository changes are limited to CLI startup, parent-package lazy export, CLI regression tests, and their README instructions.
Linux validation uses Docker with Git 2.50.1 and a Tini subreaper.
The earlier baseline comparison passed the same full gate configuration; the README revision also passes all 40 hooks.
The CodeQL follow-up validation above was run locally; it is not a hosted CI result for this fork head.

Generative AI disclosure: AI-generated implementation, tests, and description prepared with Codex (GPT-6), submitted as a draft for human review.

The locked proxy.py main function accepts keyword arguments and reads
sys.argv, so passing the gateway's argument list prevents startup.
Start the existing Proxy context with explicit input_args and use its
wait loop, preserving lifecycle cleanup and the gateway's CLI options.
Cover argument parsing, plugin loading, cleanup, and help without
opening sockets in the regression tests.

Refs: #15
Generated-by: Codex (GPT-6)
@MakerFlyFly MakerFlyFly added bug Something isn't working family:tools substrate:sandbox Tool substrate: agent isolation, egress control, settings audit labels Oct 22, 2026 — with ChatGPT Codex Connector
Use the zero-argument wait loop supported throughout the declared proxy.py
range. Isolate CLI test storage, check help in a fresh interpreter, and
cover signal-style exits without changing exception propagation.
The fresh-process check also exposed an eager parent-package import, so
defer the existing host_allowed export while preserving its identity and
star-import compatibility. Clarify the manual proxy smoke check.

Refs: #15
Generated-by: Codex (GPT-6)
Assert that default CLI startup loads only the forward proxy handler
and no web-server or reverse-route plugins. Document the supported mode
and the trust required for additional proxy.py arguments.

Keep runtime parsing and dependency constraints unchanged.

Generated-by: Codex (GPT-6)
Keep the guide focused on running and configuring the gateway. Replace
test implementation details with a concise coverage summary, scope the
automated test command to the package directory, and document allowed
and denied manual requests.

Generated-by: Codex (GPT-6)
Address two CodeQL quality findings, py/import-and-import-from and
py/implicit-string-concatenation-in-list, by importing the CLI submodule
explicitly and making the help script concatenation explicit. Preserve
the script argument, dependency-free help, and package export coverage.

Generated-by: Codex (GPT-6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working family:tools substrate:sandbox Tool substrate: agent isolation, egress control, settings audit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant