feat(egress): add HTTP credential source provider - #1188
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b685cf0b0d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b3e76f31d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39d98c297e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a0659bf71
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c598d791f1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60fcdad44e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Add a new credential source type that fetches credential material from an HTTP endpoint. The endpoint returns a JSON response with value, url, headers, and ttl fields — enabling dynamic credential rotation with caching. Uses singleflight to coalesce concurrent fetches and RWMutex for lock-free cache reads. Update all SDKs (Go, Python, JavaScript, Kotlin, C#) to support the new HTTP credential source type alongside the existing inline source. Extract source type constants (SourceTypeInline, SourceTypeHTTP) to eliminate magic strings.
Fix gofmt alignment in HTTPCredentialSource struct fields. Move test imports to file-level and remove unused variable to satisfy ruff.
Sort imports alphabetically and reformat when-branch bodies.
Security fixes from PR review: - Validate URL scheme (http/https only) at write time, reject relative URLs - Disable HTTP redirects to prevent credential header leakage - Track header rotation explicitly so empty headers response clears bootstrap headers instead of leaking them to rotated URLs Add HTTPCredentialSource schema to egress-api.yaml with oneOf discriminator. Regenerate JS and Python API clients from updated spec.
- Add SO_MARK transport to HTTP source fetches so outbound connections bypass the transparent iptables redirect (prevents MITM → active snapshot → singleflight deadlock on first fetch / TTL expiry) - Add mark-based RETURN rule to transparent HTTP iptables chain - Validate URL host presence, HTTP method, and header names/values at write time instead of failing silently at activation - Remove default keyword from OpenAPI spec method field so generated TS client treats it as optional - Regenerate JS and Python API clients
SO_MARK requires CAP_NET_ADMIN which is unavailable in CI runners and development environments. The mark is only needed when iptables transparent redirect is active (container runtime), so silently ignore the error elsewhere.
- Use separate CredentialFetchMark (0x2) for HTTP source transport so it bypasses iptables transparent redirect but NOT nft egress policy - Validate rotated URLs from provider responses before caching - Clear bootstrap headers when provider rotates URL without headers - Reject all control characters in header values, not just CR/LF
- Skip caching rotated headers when the accompanying URL is invalid - Validate rotated header names/values before caching
- Make URL and headers rotation atomic: skip URL update when headers validation fails, preventing bootstrap header leakage to new URL - Add source(InlineCredentialSource) and source(HTTPCredentialSource) overloads to Kotlin Credential.Builder for binary compatibility with existing callers compiled against the previous SDK
60fcdad to
d3e7cf5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3e7cf5310
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codex flagged that providers echoing their own refresh URL without a 'headers' field would trigger headersRotated=true with nextHeaders=nil, so the next fetch went to the same endpoint without bootstrap auth and could start returning 401. Only treat a response URL as a rotation when it actually differs from the current refresh URL; otherwise leave cached headers untouched. An explicit non-nil 'headers' map (including empty) still replaces the header set as before. Adds TestHttpSourceEchoedURLKeepsBootstrapHeaders.
Adds a Credential Sources section covering: - Inline vs HTTP source shapes. - Provider response contract (value/url/headers/ttl) and required fields. - Rotation semantics for URL-only, headers-only, echoed-URL, and empty headers responses, matching the current sidecar behavior. - Write-time validation rules (scheme/host, method, header name/value, redirect rejection). - Egress policy requirement: provider host must be reachable under the sandbox network policy. - Compatibility note on required egress and SDK versions.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19342370e1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Address three P2 issues raised by Codex against 1934237: - validateHTTPSourceURL now range-checks parsed.Port(); values outside 1-65535 (including 0, 65536, 99999) are rejected at create/patch and during response-driven URL rotation instead of only failing later at dial time. - validateHTTPSourceHeaders rejects duplicate header names that differ only in case (e.g. X-Auth vs x-auth). Go map iteration order would otherwise make which value req.Header.Set keeps nondeterministic, producing flaky provider auth. - Credential Vault docs previously told users that omitting 'ttl' forced a fresh fetch on every request. The implementation caches indefinitely when 'ttl' is nil and only refreshes each call for 'ttl: 0'. Reword the table to match code. Regression tests: TestHttpSourceFactoryRejectsOutOfRangePort, TestHttpSourceFactoryRejectsDuplicateHeadersCaseInsensitive.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: feea6b96bb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codex flagged that a zero-TTL HTTP-backed credential referenced by both an injected header and a substitution within one binding, or by more than one binding, could receive different plaintext values for the same outbound request because renderInjectionHeaders and renderSubstitutions each call Resolve independently. Introduce a per-render resolver that memoises the plaintext value for each credential name for the duration of one ActiveSnapshotWithContext call. Every header and every substitution in the resulting snapshot now sees the same value for the same credential, regardless of TTL. A subsequent snapshot still gets a fresh single resolve, preserving the existing zero-TTL semantics between snapshots. Test TestActiveSnapshotResolvesCredentialOncePerRender uses a source whose Resolve returns a monotonically increasing value to prove that one render performs exactly one resolve per credential, that the header and substitution surfaces receive the same value, that a second binding referencing the same credential also sees that value, and that a later snapshot render performs a fresh resolve.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e05b3e076
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…ed URLs Codex flagged two paths on 9e05b3e: - Loopback URLs (127.0.0.0/8, ::1, 'localhost') bypass sandbox egress policy because the nft output chain accepts loopback unconditionally and the credential-source transport uses fwmark 0x2 to also skip the iptables transparent redirect. validateHTTPSourceURL now rejects loopback hosts at write time and (via the shared validator) on every response-driven URL rotation. No configuration knob: production must never see a loopback provider. - A response echoing the current refresh URL in a different but equivalent form (host casing, explicit default port, fragment) was compared with raw string equality and treated as a cross-endpoint rotation, clearing bootstrap headers and 401ing the next fetch. Compare canonicalHTTPSourceURL(response) vs canonicalHTTPSourceURL(current) so scheme/host case, default ports for the scheme, and fragments do not falsely trigger rotation. Tests exercise the resolver against httptest.NewServer without weakening the loopback rejection: they use a non-loopback hostname ('vault.test') in the URL and install a DialContext on the source's http.Transport that redirects every dial to the httptest listener. The production code never sees the test hook. New tests: - TestHttpSourceFactoryRejectsLoopbackHost covers 127.0.0.1, 127.5.5.5, ::1, 'localhost', and mixed-case 'LocalHost:8080'. - TestHttpSourceEquivalentEchoedURLKeepsBootstrapHeaders proves that a provider echoing 'http://Vault.TEST:80/cred' after being configured with 'http://vault.test/cred' keeps its bootstrap X-Auth header on the next fetch. docs/guides/credential-vault.md documents both new rules (loopback rejection with rationale, URL canonicalisation during rotation).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 903941592c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Summary
httpcredential source type that fetches credential material from an HTTP endpoint, with response-driven caching (TTL), dynamic URL/headers rotation, and singleflight-coalesced concurrent fetchesSourceTypeInline/SourceTypeHTTPconstants to eliminate magic stringsHTTPCredentialSourcealongside existingInlineCredentialSourceTest plan
🤖 Generated with Claude Code