fix: routed URL host advertisement and allow-only key normalization#1683
Merged
fix: routed URL host advertisement and allow-only key normalization#1683
Conversation
When the gateway binds to 0.0.0.0 (or ::), clients cannot connect to the advertised URLs since 0.0.0.0 is not a routable address. Fix: In writeGatewayConfig, prefer config.Gateway.Domain if set, and map wildcard addresses (0.0.0.0, ::) to 127.0.0.1 for client-reachable URLs. Also fixes TestBinaryInvocation_PipeOutput to use mock HTTP backend instead of Docker alpine container (same pattern as other test fixes). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Change canonical JSON key from "allowonly" to "allow-only" (with dash) across all serialization, deserialization, map lookups, error messages, tests, and documentation. Accept legacy "allowonly" (no dash) for backward compatibility in: - GuardPolicy.UnmarshalJSON - parsePolicyMap in unified.go - buildStrictLabelAgentPayload in wasm.go Also fix integration test timeouts in difc_config_test.go by adding context.WithTimeout and missing apiKey in JSON stdin configs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes routed DIFC mode URL advertisement and guard policy key normalization to align with schema/docs, plus improves DIFC integration test robustness.
Changes:
- Advertise client-reachable URLs by preferring
gateway.domain, otherwise mapping wildcard bind hosts (0.0.0.0,::) to127.0.0.1. - Canonicalize guard policy JSON key to
"allow-only"while accepting legacy"allowonly"across parsing/adaptation paths. - Harden DIFC integration tests with
context.WithTimeoutand schema-requiredapiKeyin JSON stdin configs.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
internal/cmd/root.go |
Adjusts advertised host selection for routed/unified output config (domain override + wildcard mapping). |
internal/config/guard_policy.go |
Switches canonical JSON key to "allow-only" and accepts legacy "allowonly" on unmarshal. |
internal/guard/wasm.go |
Updates strict policy payload validation to prefer "allow-only" with legacy fallback. |
internal/server/unified.go |
Updates server guard policy parsing to prefer "allow-only" with legacy fallback. |
internal/cmd/flags_difc.go |
Updates CLI help/error strings to use "allow-only" terminology. |
docs/GUARD_RESPONSE_LABELING.md |
Updates documentation examples/field names to "allow-only" + "min-integrity". |
test/integration/difc_config_test.go |
Adds timeouts and apiKey to avoid hangs and satisfy stdin schema validation. |
test/integration/binary_test.go |
Makes pipe-output test Docker-free by using mock HTTP backend; tweaks timing. |
test/integration/auth_config_test.go |
Aligns expected advertised URL host with domain-based output (localhost). |
internal/server/label_agent_test.go |
Updates server guard policy fixture key to "allow-only". |
internal/guard/guard_test.go |
Updates policy payload fixtures/assertions for "allow-only" key and error text. |
internal/config/config_difc_test.go |
Updates DIFC config parsing fixtures to use "allow-only". |
internal/cmd/stdout_config_test.go |
Adds/updates cases for wildcard mapping and domain override in advertised URLs. |
internal/cmd/flags_difc_test.go |
Updates env var JSON fixture to use "allow-only". |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
155
to
160
| if integrityInput == "" { | ||
| return nil, fmt.Errorf("allowonly integrity is required") | ||
| return nil, fmt.Errorf("allow-only integrity is required") | ||
| } | ||
| if !hasIntegrity { | ||
| return nil, fmt.Errorf("allowonly integrity must be one of: none, unapproved, approved, merged") | ||
| return nil, fmt.Errorf("allow-only integrity must be one of: none, unapproved, approved, merged") | ||
| } |
There was a problem hiding this comment.
The new error messages in buildAllowOnlyPolicy say "allow-only integrity ...", but the flag/config field name is "min-integrity" (and the help text uses "min-integrity"). Updating these messages to consistently refer to "min-integrity" will make it clearer to users what value is missing/invalid.
See below for a potential fix:
return nil, fmt.Errorf("min-integrity is required")
}
if !hasIntegrity {
return nil, fmt.Errorf("min-integrity must be one of: none, unapproved, approved, merged")
Collaborator
Author
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Contributor
There was a problem hiding this comment.
Fixed in 94cc639. Updated both error messages in buildAllowOnlyPolicy to use "min-integrity" consistently:
"min-integrity is required""min-integrity must be one of: none, unapproved, approved, merged"
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Contributor
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
This was referenced Mar 8, 2026
4 tasks
This was referenced Mar 10, 2026
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.
buildAllowOnlyPolicyto use "min-integrity" instead of "allow-only integrity"make agent-finished✓)🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.