feat(relay): route git pre-receive policy callback over UDS when configured#1330
Open
tlongwell-block wants to merge 2 commits into
Open
feat(relay): route git pre-receive policy callback over UDS when configured#1330tlongwell-block wants to merge 2 commits into
tlongwell-block wants to merge 2 commits into
Conversation
…igured
The pre-receive hook curls the relay's own /internal/git/policy callback. By
default it targets the TCP loopback 127.0.0.1:{bind_port}. In a mesh deployment
(e.g. Istio) that loopback port can be iptables-captured by the sidecar, so the
relay's call to itself fails with 'network error reaching policy service' and
every push is rejected.
The relay already serves the same router over its UDS listener (main.rs) when
BUZZ_UDS_PATH is set, so the policy endpoint is already reachable over the
socket — only the hook's curl target needs to change.
- transport.rs: when config.uds_path is set, point BUZZ_HOOK_URL at a
placeholder host (curl ignores host:port under --unix-socket) and pass the
socket path via BUZZ_HOOK_SOCKET. Extract policy_hook_url() as a pure,
unit-testable helper. When uds_path is None, behavior is byte-identical to
before (TCP loopback) — local/dev/CI are untouched.
- hook.rs: add --unix-socket "$BUZZ_HOOK_SOCKET" to the policy curl iff
BUZZ_HOOK_SOCKET is non-empty. --unix-socket is stock libcurl.
- Unit tests for both URL branches.
Pairs with a tf-stacks chart change that sets BUZZ_UDS_PATH + a shared emptyDir
mount; that removes the relay's dependency on the mesh loopback entirely.
Refs RESEARCH/SPROUT_GIT_POLICY_DEPLOY_MISMATCH.md.
Co-authored-by: Tyler Longwell <tlongwell@block.xyz>
Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
Max's review caught that routing the policy callback over the UDS isn't enough: the UDS listener served `into_make_service()` (no connect-info), so over the socket there is no `ConnectInfo<SocketAddr>` in request extensions. The `/internal/git/policy` route's `require_localhost` guard reads exactly that and `unwrap_or(false)`s — so the UDS callback would fail closed with HTTP 403, turning the original 'network error' into 'push denied by policy (HTTP 403)'. Fix: - Add a `UdsConnectInfo` marker with `Connected<IncomingStream<UnixListener>>`, and serve the UDS listener with `into_make_service_with_connect_info::<UdsConnectInfo>()`. - Teach `require_localhost` to accept either a loopback TCP `SocketAddr` OR the presence of `ConnectInfo<UdsConnectInfo>`. The UDS is an in-pod filesystem path, so its presence is itself proof the caller is on-host. TCP behavior is unchanged and still fail-closed without a loopback `SocketAddr`. - Unit tests for the guard: no connect-info -> 403, loopback TCP -> 200, non-loopback TCP -> 403, UDS marker -> 200. Reviewed-by: Max Co-authored-by: Tyler Longwell <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
Collaborator
Author
|
Update: review by Max found that the UDS listener served |
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.
Problem
The pre-receive git hook curls the relay's own
/internal/git/policycallback athttp://127.0.0.1:{bind_port}. In a mesh deployment (e.g. Istio on sprout), the sidecar's iptables captures that loopback port, so the relay's call to itself fails withnetwork error reaching policy serviceand every push is rejected. (See companion tf-stacks PR squareup/block-coder-tf-stacks#2244 for the immediate deploy-side unblock.)Why UDS
The relay already serves the same router over its UDS listener (
main.rs, whenBUZZ_UDS_PATHis set) — so/internal/git/policyis already reachable over the socket. Only the hook's curl target needs to change. A unix-socket callback dodges any in-pod L4 interception of the loopback TCP port entirely, so this can't regress when mesh config changes.Change (~15 lines, 2 files + tests)
config.uds_pathis set, pointBUZZ_HOOK_URLat a placeholder host (curl ignores host:port under--unix-socket) and pass the socket path viaBUZZ_HOOK_SOCKET. Extractedpolicy_hook_url()as a pure, unit-testable helper. Whenuds_pathisNone, behavior is byte-identical to before (TCP loopback) — local/dev/CI untouched.--unix-socket "$BUZZ_HOOK_SOCKET"to the policy curl iffBUZZ_HOOK_SOCKETis non-empty (bash array, no word-split risk).--unix-socketis stock libcurl; the relay image ships curl.Validation
cargo build -p buzz-relay✅,cargo clippy -p buzz-relayclean ✅policy_hook_url_uses_tcp_loopback_without_uds,policy_hook_url_uses_placeholder_host_with_uds✅bash -nsyntax-clean; verified the--unix-socketflag is added only when the socket env is set (unset/empty → unchanged TCP path) ✅e2e_git.rsalready exercises push→policy over TCP (no UDS in CI), so the default path stays covered.local_echo_presence...— a config-test bogus-path fixture polluting a parallelConfig::default();owner_archive_rejects_stale...— needs a live migrated Postgres) are pre-existing env/DB issues, not touched by this diff.Deploy follow-up
Pairs with a tf-stacks chart change that sets
BUZZ_UDS_PATH=/run/buzz/relay.sock+ a sharedemptyDirmount at/run/buzzon the relay container. Sequence: this PR merges → image builds → flip chart to UDS → drop the #2244 excludeOutbound workaround.Refs
RESEARCH/SPROUT_GIT_POLICY_DEPLOY_MISMATCH.md.🤖 Drafted by Eva; reviewed/tested with Max.