Skip to content

feat(postgres): capture application_name in the request log for connection tracing - #58

Merged
andybons merged 1 commit into
mainfrom
postgres-application-name
Jul 15, 2026
Merged

feat(postgres): capture application_name in the request log for connection tracing#58
andybons merged 1 commit into
mainfrom
postgres-application-name

Conversation

@andybons

Copy link
Copy Markdown
Contributor

What

Captures the client-supplied Postgres application_name startup parameter into the canonical request log, so a Postgres connection can be traced to the originating box via a client-chosen slug — the Postgres analogue of HTTP capture_headers.

Two tracing signals now sit side by side on the Postgres log line:

  • run_id — the trusted, authenticated identity (from the run token), already present.
  • application_name — a free-form, client-set correlation slug (via PGAPPNAME, application_name=…, or options), new here. It also already flows through to Neon, so the same slug surfaces in pg_stat_activity.

How

  • New RequestLogData.ApplicationName, set once at the Postgres log-entry construction so it feeds every exit path (success, policy-deny, no-resolver, upstream-fail) — denied connections stay traceable.
  • Emitted as an application_name slog attribute, non-empty-gated like run_id/client_ip.
  • Sanitized through a new shared proxy.SanitizeLogValue (256-byte UTF-8-safe bound, strips control chars, fixes invalid UTF-8) — it's client-controlled text, so this closes the log-injection vector. Sanitization applies to the log only; the value forwarded upstream to Neon is untouched.
  • Deduplication: the existing HTTP capture_headers truncation now routes through the same SanitizeLogValue. Behavior change there is strictly an improvement — same 256-byte bound, now also strips control chars (the same injection vector it previously passed through).

application_name is a correlation slug, not identity — nothing keys auth/policy off it; run_id remains the trusted identity.

Review

Opus-reviewed (ship): sanitization verified against adversarial inputs (embedded \n+forged log line, NUL, C0/C1 controls, invalid UTF-8, 10KB → 256 bytes at a rune boundary); confirmed the capture_headers refactor is a non-regressing improvement; confirmed the upstream-forwarded value is the raw one while the log gets the sanitized one (load-bearing test). Mutation-checked (neutering the control-char strip turns the tests red).

Tests

Test-first: clean/absent/sanitized+truncated capture, denied-connection capture, forwarding-unaffected guard, and SanitizeLogValue unit tests — the sanitization test encodes a forged run_id=fake-admin-run injection payload. go test -race ./..., vet, gofmt clean.

CHANGELOG v0.20.0.

@claude

claude Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Claude finished @andybons's task in 3m 52s —— View job


Review complete

  • Gather context (PR body, changed files list)
  • Review the diff against origin/main (proxy/proxy.go, proxy/postgres.go, gatekeeper.go, tests, docs, CHANGELOG)
  • Post review findings

Note: go build/go vet/go test were not runnable in this sandboxed session (the go toolchain required interactive approval I couldn't obtain), so this review is static/manual only — I read every changed file rather than running the test suite. Given the PR description states go test -race ./..., vet, and gofmt were already run clean, I did not re-verify that mechanically.

Summary

Solid, well-scoped change. RequestLogData.ApplicationName is populated once at Postgres log-entry construction (proxy/postgres.go:796-811) so it naturally covers every exit path — success, network-policy deny, no-resolver deny, upstream failure — since they all share the same logEntry and call s.log(logEntry). Confirmed via TestPostgresDeniedConnectionLogsApplicationName and the code path itself (deny() closures mutate/log the same struct).

Confirmed the security-relevant separation holds throughout: application_name only ever feeds slog.String(...) for logging (gatekeeper.go:315-318) and is never consulted by authenticate(), network policy, or resolver selection — run_id (from the authenticated run token) remains the sole trust signal. startupParams["application_name"] (the raw, unsanitized value) is what's actually forwarded upstream via connectWithRetrySanitizeLogValue is applied only when building logEntry, so the sanitization never touches what Neon sees. TestPostgresLogsApplicationName's upstream-forwarding assertion (fake.lastApplicationName()) correctly guards this.

proxy.SanitizeLogValue (proxy/proxy.go:820-853) is correct: fixes invalid UTF-8 via strings.ToValidUTF8 first, strips unicode.IsControl runes (covers C0/C1 controls including DEL 0x7F), then bounds to 256 bytes truncating at a valid rune boundary — order of operations avoids re-introducing invalid UTF-8 or double-processing. The capture_headers dedup (gatekeeper.go:352-361) is a genuine improvement: same 256-byte bound as before, now also strips control chars it previously let through untouched.

One observation, not a blocking issue: the PR frames this as closing a "log-injection vector," but the two handlers this repo actually wires up (slog.NewTextHandler / slog.NewJSONHandler in gatekeeper.go) already quote/escape control characters like \n in slog.String values (Go's stdlib needsQuoting + strconv.AppendQuote for text, JSON string escaping for JSON) — so a raw newline wouldn't already forge a second line in the log file under either configured handler. SanitizeLogValue's control-char stripping is still worthwhile defense-in-depth (protects any downstream consumer that doesn't re-quote — e.g. a custom handler, or a system that concatenates the value into free text elsewhere) and the 256-byte bound is independently useful regardless. Just flagging that the "forged run_id=fake-admin-run injection" scenario in the SanitizeLogValue unit test (proxy/proxy_test.go:4089-4090) is tested at the sanitizer level only, not against the actual rendered slog output, so it doesn't by itself demonstrate the vulnerability existed in the shipped log format.

Also very minor: the CHANGELOG entry says "proxy.RunContextData continues to forward it upstream unchanged" (CHANGELOG.md:11) — the code that actually forwards the raw value upstream is connectWithRetry/startupParams in proxy/postgres.go, not RunContextData. Cosmetic only, not worth a re-push on its own.

Test coverage is thorough and test-first per AGENTS.md: clean/absent/sanitized+truncated capture, denied-connection capture, forwarding-unaffected guard (the load-bearing one), and SanitizeLogValue unit tests covering empty, plain, embedded CR/LF/NUL, and truncation at both ASCII and multi-byte UTF-8 boundaries. No race conditions, deadlocks, or nil/zero-value issues found in the changed code — startupParams["application_name"] on a possibly-nil/absent-key map safely returns "", and the single-goroutine-per-connection ownership of logEntry in serveAuthenticated means no synchronization is needed.

No inline comments filed — didn't find anything rising to a fix-worthy defect.

@andybons
andybons merged commit 61b2782 into main Jul 15, 2026
2 checks passed
@andybons
andybons deleted the postgres-application-name branch July 15, 2026 23:45
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