Skip to content

fix(postgres): match portless allow patterns for data-plane traffic#53

Merged
andybons merged 1 commit into
mainfrom
fix-postgres-policy-port
Jul 15, 2026
Merged

fix(postgres): match portless allow patterns for data-plane traffic#53
andybons merged 1 commit into
mainfrom
fix-postgres-policy-port

Conversation

@andybons

Copy link
Copy Markdown
Contributor

Problem

Under network.policy: strict, a portless allow pattern like allow: ["*.neon.tech"] never matched Postgres data-plane traffic, so every Postgres connection was denied before the resolver ran (client: FATAL: connection not allowed by network policy, SQLSTATE 28000). The shipped examples/gatekeeper-postgres.yaml uses exactly this pattern under policy: strict and was therefore broken as written.

Root cause: matchesPattern (proxy/hosts.go) treats a portless pattern as 80/443-only (an HTTP-scheme default), but the Postgres plane evaluates on port 5432 — while the resolver matcher (postgresResolverFromEntries) already defaults an unspecified pattern port to 5432. So for the same connection to the same host, the resolver matcher said "I handle *.neon.tech" while the policy matcher said "denied." An internal inconsistency and a footgun.

Fix

Added matchHostPostgres + Proxy.checkNetworkPolicyPostgres — Postgres-specific matchers that default the pattern port to 5432, byte-identical to the resolver's existing logic and to the HTTP matcher except for that port default. The Postgres plane's policy check now uses them. HTTP/CONNECT policy is untouchedmatchHost/matchesPattern/checkNetworkPolicy are not modified and no HTTP path calls the new matchers.

Discovered during a Neon connectivity investigation; this was one of two boundaries collapsing into an opaque failure.

Security

This touches the allow/deny boundary, so it got a dedicated review. The change only alters which port a portless pattern is compared against, and only on the Postgres data-plane path: host matching (wildcard/exact/case) is identical, evil.com still denies, *.neon.tech:5433 still denies a 5432 connection, and reaching the Postgres plane still requires run-token auth + a matching resolver. No new host is reachable that wasn't already reachable via the resolver.

Tests

Test-first (red demonstrated): TestPostgresPolicyAllowsPortlessPattern (end-to-end TLS + SCRAM + policy gate) plus deny/regression cases, and TestCheckNetworkPolicyHTTPPortDefaultsUnchanged pinning the HTTP 80/443-only default. go test -race ./..., vet, gofmt all clean.

CHANGELOG v0.17.3.

hostPattern.port == 0 (an unspecified pattern port) means "match only
ports 80 and 443" -- an HTTP-scheme default enforced by matchesPattern.
The Postgres data plane's network-policy check called that same
HTTP-oriented matchHost/checkNetworkPolicy with the literal Postgres
port (5432), so a portless pattern like "*.neon.tech" could never
match a live Postgres connection: every connection was denied under
strict policy, regardless of the allow list. This left the
network-policy matcher inconsistent with the credential-resolver
matcher for the identical connection -- postgresResolverFromEntries
already defaults an unspecified pattern port to 5432 for resolver
matching, so a host could be accepted for credential resolution and
simultaneously rejected by policy. Both examples/gatekeeper-postgres.yaml
and docs/content/guides/13-postgres-neon.md document exactly this
configuration, so the shipped example was broken as written.

Adds matchHostPostgres (proxy/postgres.go) and
Proxy.checkNetworkPolicyPostgres (proxy/proxy.go), mirroring the
existing HTTP-facing matchHost/checkNetworkPolicy but applying the
same port-0-defaults-to-5432 override postgresResolverFromEntries
already uses, so the two matchers can no longer disagree.

Security-scoped to the Postgres data plane only: matchHost,
matchesPattern, checkNetworkPolicy, and checkNetworkPolicyForRequest
(the shared matcher and its HTTP/CONNECT callers) are untouched -- a
portless allow pattern still means "ports 80 and 443 only" for HTTP
traffic, and a pattern pinned to an explicit non-5432 port still
denies Postgres connections. Covered by new tests proving: the
portless-pattern allow now succeeds, an unlisted host is still denied,
an explicit wrong-port pattern is still denied, an explicit correct
port pattern still works, and checkNetworkPolicy's HTTP port defaults
are unchanged.
@claude

claude Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Claude finished @andybons's task in 2m 13s —— View job


Claude finished the code review

Tasks

  • Read repo instructions (AGENTS.md / CLAUDE.md)
  • Gather context: read diff and changed files
  • Trace policy/resolver matching logic for consistency
  • Verify tests cover the fix and the guard rails
  • Post review findings

Note: go build/go test/go vet were not runnable in this sandbox (Bash tool calls other than read-only git diff/grep required approval that wasn't granted). Review is based on careful static reading of the diff and surrounding code; if you'd like the CI test run reconfirmed, --allowedTools would need to permit go build/go test for future runs.

Summary

This is a clean, well-scoped fix. I traced the logic carefully and didn't find any bugs.

  • Root cause is accurately diagnosed. matchesPattern (proxy/hosts.go:63-84) hard-codes "no port on the pattern → match only 80/443," an HTTP assumption. postgresResolverFromEntries (proxy/postgres.go:132-144, pre-existing) already overrides this by defaulting an unset pattern port to 5432 before calling matchesPattern. The Postgres policy check at serveAuthenticated didn't have the same override, so *.neon.tech (portless) matched for resolver lookup but not for the policy gate — exactly the inconsistency described.
  • matchHostPostgres (proxy/postgres.go:142-153) is byte-identical in intent to postgresResolverFromEntries's override: for each pattern, if pattern.port == 0 it's set to postgresDefaultPort (5432) on the local copy (safe — range over []hostPattern copies each element; no aliasing of the caller's slice), then delegates host/port matching to the untouched matchesPattern. Explicit non-5432 ports are left alone, so *.neon.tech:5433 still never matches a 5432 connection — verified against TestPostgresPolicyExplicitWrongPortDenied.
  • checkNetworkPolicyPostgres (proxy/proxy.go:2039-2053) mirrors checkNetworkPolicy exactly (same RLock/policy != "strict" short-circuit), just swapping in matchHostPostgres. No new locking pattern, no risk of the RLock being taken twice or held across a blocking call.
  • HTTP/CONNECT path is genuinely untouchedmatchHost, matchesPattern, checkNetworkPolicy, checkNetworkPolicyForRequest are unmodified, and TestCheckNetworkPolicyHTTPPortDefaultsUnchanged pins that a portless pattern still means 80/443-only there. Confirmed no HTTP call site was pointed at the new Postgres matchers.
  • RunContextData.AllowedHosts immutability: the struct doc (proxy/proxy.go comment above the type) states these host-keyed fields are read without synchronization and must not be mutated after registration — matchHostPostgres respects that (read-only range, no lock needed), consistent with how matchHost already treats the same field.
  • Test coverage is thorough and matches the AGENTS.md test-first workflow: allow (portless), deny (host not covered by pattern), deny (explicit wrong port), allow (explicit correct port), and the HTTP-default-unchanged regression test. Each closes a specific gap the fix could have opened (e.g., proving the fix doesn't silently widen the allow surface or leak into the HTTP path).
  • Security posture: the change only shifts which port a portless pattern is compared against on the Postgres plane; it doesn't touch host-matching semantics (exact/wildcard/case-fold), doesn't bypass run-token auth, and doesn't add a new reachable host that the resolver wasn't already scoping. Consistent with the PR's own security note.

Minor (non-blocking) observation

matchHostPostgres (proxy/postgres.go:142-153) and postgresResolverFromEntries (proxy/postgres.go:132-144) duplicate the same 5-line "default port 0 → 5432" override. Given the PR's own framing — the bug existed because two matchers had to independently agree on this exact rule — a small shared helper (e.g. pinPortgresPort(pattern hostPattern) hostPattern) would remove the chance of the two drifting apart again in the future. Not a bug as shipped, and a matter of judgment whether it's worth the extra indirection for ~5 lines — flagging for consideration only, not requesting a change.

No inline comments filed — I didn't find any correctness, security, race, or error-handling issues to anchor them to.

@andybons andybons merged commit 47c6c1d into main Jul 15, 2026
2 checks passed
@andybons andybons deleted the fix-postgres-policy-port branch July 15, 2026 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant