Repository navigation
fix(egress-gateway): start proxy with explicit arguments - #16
Draft
MakerFlyFly wants to merge 5 commits into
Draft
MakerFlyFly wants to merge 5 commits into
MakerFlyFly wants to merge 5 commits into
Conversation
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)
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)
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
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.
Summary
The documented gateway command fails before startup with locked
proxy.py 2.4.10because it passes a positional list toproxy.main(**opts).Use
Proxy(input_args=...)with the zero-argument wait loop supported by the declared>=2.4,<3dependency range.host_allowedexport so this works for the complete import path.Type of change
Review dispositions
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_allowedexport, its function identity, static typing import, and__all__are preserved.The confirmed passthrough
--hostnameoverride and the--host localhostvalidation 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
87d8cfeareproduces the wait-loop TypeError on all four older versions.9dd73b76and fail the added assertions.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-cscript 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-queries1.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
Linked issues
Refs #15.
Notes for reviewers
This remains a draft for human audit within the contributor's fork.
Its base
audit/h11-baseis fixed ate6e5c2e36bcab4d0a09d6da5c26e8060caaea3e2.The commits append to the previously published
87d8cfea,9dd73b76,13a6798b, andc70bf873heads, 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.