[freeqaz]

Problem

#2425 and #2427 removed three copies of a strict decimal-IPv4 check that only ran on a URL-parser-written host after isIP(...) === 4, so it could never fail. Two more copies remained:

• apps/controller/src/drivers/compute/kubernetes/index.ts, channel proxy URL: writtenHost is tested against the strict pattern and an octet > 255 check, but only after writtenHost !== parsed.hostname already threw.
• scripts/render-installation-profile.mjs, ipv4HostKept: written === url.hostname && strict.test(url.hostname).

isIP accepts only four decimal octets from 0 to 255 with no leading zeros, and the URL parser serializes every IPv4 host that way. At both sites the tested string equals parsed.hostname, so these terms never change an outcome.

This round swept the whole repo (apps, packages, scripts, deploy, internal, cmd) at once, so no further copies are left behind.

Change

• Compute: drop the strict pattern and the octet > 255 term; the condition is now writtenHost === undefined || writtenHost !== parsed.hostname.
• Renderer: ipv4HostKept returns written === url.hostname.
• Fold the reasoning into both comments. Error messages are unchanged.

No behaviour change.

Sweep

Searched for 25[0-5], \d{1,3}, [0-9]{1,3}, [1-9][0-9]{0,2}, (?:0|[1-9], isIP(, isIPv4(, > 255 / <= 255, octet, leading zero, dotted-quad shapes, and Go ParseIP/ParseAddr/Is4.

Site	What it tests	Verdict
kubernetes/index.ts channel proxy (strict pattern + octet > 255)	writtenHost, already equal to parsed.hostname, after isIP === 4	dead, removed
render-installation-profile.mjs ipv4HostKept	url.hostname after isIP === 4	dead, removed
auth/configuration.ts refuseRewrittenIpv4Host, occ/backends.ts gatewayEndpointRewritesIpv4, openai-endpoint.ts openAiBaseUrlRewritesIpv4	written-vs-parsed comparison only	already cleaned (#2425, #2427)
channel/slack.ts:17 PROXY_ENDPOINT	raw proxy URL text	live
render-installation-profile.mjs:68 isLiteralIpv4ProxyUrl (+ > 255)	raw profile value	live
controller/src/index.ts ipv4() / cidrContains prefix	raw dev trusted CIDR text	live
auth/client-address.ts:72 prefix pattern	raw CIDR prefix	live (not an IPv4 host)
docker/index.ts loopbackHost	raw host:port text	live
scripts/ci/logging.mjs assertIpv4, assertIpv4Cidr, EndpointSlice filter	raw CI inputs, no isIP gate	live
runtime-entrypoints.ts:3264 loopback peer_addr	raw log field	live
occ/runtime-logs/sanitize.ts:402 CODEX_SOCKET_ADDRESS	redaction pattern over log text	live (not a check)
client-address.ts:42, render-installation-profile.mjs:800 embedded IPv4 tail	IPv6 group conversion, no refusal	not a check
render-installation-profile.mjs isIpv4Cidr, isCidr, sourceCidrs, OIDC host; server.mjs, metrics/listener.ts, oidc.ts, postgres-pool.ts, openshell-service-transport.ts, openshell-gateway-client.ts, check-api-database-ca.mjs; scripts/ci/prepare.mjs isIPv4	isIP / isIPv4 gate only, no extra pattern	no copy
Helm _helpers.tpl (ipv4Host32, ipv4Cidr, trusted proxy, auth.baseUrl written host, IPv4-mapped tails), networkpolicies.yaml:105, observability-demo _helpers.tpl:16	chart values; Go's URL parser keeps the written host	live
internal/occdev/*.go	netip.ParseAddr / ParsePrefix / Is4 only	no copy
tests/** dotted-quad assertions	test expectations on output	out of scope

Verification

• Probe (local script, reusing refactor(occ): drop two more dead strict ipv4 patterns #2427's): 12.3M dotted strings checked, 340,736 pass isIP(...) === 4, none fails the strict pattern or has an octet above 255. Old vs new bodies of both functions compared on a fixed list (octal, hex, shorthand, single integer, trailing dot, percent and fullwidth escapes, userinfo, backslashes, missing slashes, leading controls, tabs and newlines inside the host, uppercase scheme, IPv6) plus ~619k fuzzed URLs: 242,534 refusals, 0 differences.
• Suites, pinned to 2 CPUs, unchanged and passing: profile-renderer 67/67, profile-preflight-chart-parity 30/30, kubernetes-compute 354/354, url-authority 1/1, configuration-occ 27/27.
• tsc --build for apps/controller, Prettier and ESLint on the touched files: clean.

Risks

Low. Two dead terms removed where the tested string provably equals a canonical isIP IPv4 host. If a future change applied either check to a raw, unparsed host, it would need its own strictness check.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs maintainer review before merge.

What this changes

This PR removes redundant strict IPv4 checks from Kubernetes channel-proxy validation and installation-profile rendering.

Example: An Installation configures channels.proxyUrl as http://0177.0.0.1:8080.

• Before: The written-versus-parsed hostname comparison rejects the rewritten IPv4 spelling.
• After: The same comparison rejects it with the same diagnostic; the redundant decimal-pattern and octet checks are removed.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The patch is focused and source-equivalent; empirical evidence is reported through local probes and suites rather than inspectable shipped-entry-point output.
Proof confidence	🦐 gold shrimp (3/6)	The supplied host policy identifies this as maintainer-authored and outside the ordinary contributor proof gate; source equivalence and reported differential probes support unchanged behavior, with no material authority-chain change.
Patch quality	🦞 diamond lobster (5/6)	No actionable review findings were identified.

Product

Kind: Refactor · Worth it: Yes
Reason: Removing provably redundant checks reduces maintenance burden while preserving the security-sensitive written-host contract; the supplied host policy establishes maintainer-approved direction.

Merge readiness

✅ Ready for maintainer review

Keep this PR open for maintainer triage; the pinned diff removes redundant checks without changing accepted inputs, diagnostics, or egress destinations, and no actionable defect was found.

Priority: P3
Reviewed head: 2c1fa6e5165e925cfe24df063ed7bb76c55c7d4e

Before merge

None.

Findings

None.

Tests

• Missing end-to-end proof: The supplied validation describes local differential probes and existing suites rather than an independently inspectable shipped-entry-point run.

Agent review details

How this fits together

Installation profiles validate operator URLs before rendering configuration, while Kubernetes Compute validates the channel proxy endpoint before using its address and port in gateway egress NetworkPolicies.

flowchart LR
  A[Operator URLs] --> B[Profile validation]
  B --> C[Installation configuration]
  C --> D[Kubernetes Compute]
  D --> E[Channel proxy validation]
  E --> F[Gateway egress NetworkPolicy]

Loading

Technical review

Best possible solution:

Retain the written-host comparisons and accept the focused removal of conditions already guaranteed by isIP.

Do we have a high-confidence way to reproduce the issue?

No user-facing defect is claimed; source inspection confirms the removed conditions are redundant under the retained isIP guards.

Is this the best way to solve the issue?

The focused deletion preserves the original validation contract without adding a helper, configuration option, or parallel implementation.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against b891e5479589.

Provenance checked

• Kubernetes channel proxy IPv4 validation keeps the original intent (c386614: Merged fix(kubernetes): refuse channel proxy hosts that parse as another address #2334 rejected written IPv4 hosts that URL parsing rewrites before generating egress policy.)
• Installation-profile IPv4 host validation keeps the original intent (6b54ffa: Merged fix(auth): refuse IPv4 auth origins the URL parser rewrites #1636 aligned profile auth-origin refusal with startup and chart behavior; fix(profiles): refuse an @ before url hosts and uppercase image repository paths #2358 subsequently removed redundant userinfo parsing after earlier rejection.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

• Pinned introduction: The complete introduced diff changes only two validators and their explanatory comments; error messages and written-host comparisons remain intact. (2c1fa6e5165e)
• Compute equivalence: The removed terms are reached only when node:net isIP(address) returns 4 and writtenHost equals parsed.hostname, so neither can reject an additional input. (apps/controller/src/drivers/compute/kubernetes/index.ts:1919, 2c1fa6e5165e)
• Renderer equivalence: ipv4HostKept gates on isIP(url.hostname) before comparing the written host; origin and observability callers retain their existing validation. (scripts/render-installation-profile.mjs:428, 2c1fa6e5165e)
• Final egress consumer: channelNetworkPolicy still obtains the validated proxy and constructs the same exact-address ipBlock and TCP port; the refactor introduces no alternate authorization or network path. (apps/controller/src/drivers/compute/kubernetes/index.ts:12230, 2c1fa6e5165e)
• Original security intent: Merged fix(kubernetes): refuse channel proxy hosts that parse as another address #2334 added the written-host comparison to prevent URL normalization from selecting another egress address; this PR preserves that protection. (apps/controller/src/drivers/compute/kubernetes/index.ts:1913, c386614d2c5c)
• Related merged cleanup: Merged refactor(configuration): drop the dead ipv4 pattern and pin the deploy-time gateway password check #2425 and refactor(occ): drop two more dead strict ipv4 patterns #2427 remove the same redundant pattern from different validators; neither includes these two remaining sites. (b891e5479589)

Likely related people:

• Eric Cai: Raw commit c386614 adds apps/controller/src/drivers/compute/kubernetes/index.ts:1851 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: c386614d2c5c; files: apps/controller/src/drivers/compute/kubernetes/index.ts)

Labels

Label changes:

• add P3: This is a small maintenance refactor with no change to supported inputs or user-visible behavior.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦞 diamond lobster.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

• P3: This is a small maintenance refactor with no change to supported inputs or user-visible behavior.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦞 diamond lobster.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 14, 2026, 10:05 AM ET / 14:05 UTC.