feat(git): Role-based push permissions with service-bound auth#441
Merged
Conversation
added 8 commits
April 30, 2026 21:47
Track 1: Core types (sprout-core/src/git_perms.rs) - RefPattern: validated ref patterns with segment-level wildcards - UpdateKind: Create/FastForward/NonFastForward/Delete classification - ProtectionRule: parsed from sprout-protect tags on kind:30617 - EffectiveRules: union semantics (strictest push:role wins) - evaluate_push(): full policy evaluation with built-in defaults - MemberRole: added permission_level() and has_at_least() methods - DoS bounds: 50 rules max, 256 char patterns, 3 wildcards max - 27 unit tests covering all scenarios Track 2: Policy endpoint (sprout-relay/src/api/git/policy.rs) - Internal POST endpoint for pre-receive hook callback - HMAC-SHA256 signature verification (per-push secret) - 30s TTL on callbacks (fail-closed on expiry) - Resolves kind:30617 -> protection rules - Resolves pusher channel role via sprout-channel binding - Returns 200 (allow) or 403 (deny with reasons) Track 3: Hook plumbing (sprout-relay/src/api/git/hook.rs) - Pre-receive hook shell script with fail-closed semantics - Inherits quarantine env vars for git merge-base ancestry checks - HMAC-signs callback payload before POSTing to policy endpoint - Hook installation function for bare repo creation Infrastructure: - Refactored git.rs into git/ module directory - Added git_hook_hmac_secret to relay config - Added hmac, subtle, rand dependencies Security (per Lep review): - HMAC binds callback to specific push operation - Fail-closed: any error -> deny (exit 1) - Quarantine vars inherited for ancestry checks - Bot at permission_level 0 (explicit grants only) - Constant-time HMAC comparison (subtle crate) Signed-off-by: Tyler Longwell <tlongwell@squareup.com>
Integration: - Mount /internal/git/policy route on the main router - receive_pack now passes hook env vars (SPROUT_HOOK_URL, SPROUT_HOOK_SECRET, SPROUT_REPO_ID, SPROUT_PUSHER_PUBKEY) to git subprocess - handle_git_repo_announcement installs pre-receive hook on new repos (replaces old 'disable all hooks' approach) - Refactored run_git_service into run_git_service_with_env for env passthrough - Removed owner-only push check (hook handles role-based authorization now) Any authenticated user can now attempt a push; the pre-receive hook calls back to the policy endpoint which checks channel role + protection rules. Signed-off-by: Tyler Longwell <tlongwell@squareup.com>
The shell script's HMAC sort key was targeting the wrong JSON field (sort -t'"' -k8 hit new_oid instead of ref_name). This caused HMAC verification failures on multi-ref pushes. Fix: rewrite the hook to: 1. Write ref_name + oids to a temp file (one line per ref) 2. Sort the file by ref_name (field 1) — matches Rust's sort 3. Concatenate old_oid + new_oid + ref_name in sorted order 4. Compute HMAC over the canonical payload Also improved: - Proper temp file cleanup via trap - Explicit failure if openssl HMAC computation fails - Clearer phase separation (read → compute → POST) - Comments documenting the quarantine env var inheritance Credit: @clove caught the mismatch during review. Signed-off-by: Tyler Longwell <tlongwell@squareup.com>
When a protection rule only sets flags (e.g., no-force-push) without specifying a push:role, the built-in default minimum role must still be enforced. Previously, the explicit match bypassed defaults entirely, allowing a Guest to FF push to a branch with only no-force-push set. Fix: when push_role is None but has_explicit_match is true, fall back to the built-in default for the minimum push role. Only an explicit push:<role> overrides the default. Added regression test: evaluate_guest_denied_even_with_only_no_force_push_rule Credit: @clove caught this during code review. Signed-off-by: Tyler Longwell <tlongwell@squareup.com>
Finding 3: Fix doc/behavior mismatch in parse_protection_tags — doc now correctly states it returns an error on malformed tags (fail-closed), not 'skipping' them. Finding 4: Reject push:bot and push:guest in protection rules — these are almost certainly user errors (push:bot means 'anyone can push', push:guest is similarly permissive). Added test. Finding 5: Add missing test for no-force-push allowing fast-forward. Confirms that no-force-push only blocks NonFastForward, not FF. 30 unit tests now passing. Signed-off-by: Tyler Longwell <tlongwell@squareup.com>
Hana's architecture review items: 1. Forward-compatibility: unknown rule strings in sprout-protect tags are now silently skipped instead of causing a hard failure. This means a future relay version can add new rules (e.g., 'require-review') without breaking pushes on older relays that don't understand them. Known rules are still enforced correctly. 2. Stale comments updated: - handle_git_repo_announcement doc: 'hooks disabled' → 'pre-receive installed' - Hook install warning: removed reference to non-existent 'fallback check' - GitAuth doc: updated from 'owner-only push' to 'hook-based authorization' 30 unit tests pass. Zero warnings. Signed-off-by: Tyler Longwell <tlongwell@squareup.com>
- Add hook existence check in receive_pack (fail-closed if hook missing) - Add localhost-only middleware on /internal/git/policy endpoint - Add $HMAC_FILE.concat to trap cleanup in hook script - Add set -o pipefail to hook script Addresses Clove findings #1-3 and Lep findings #1-2. Both reviewers flagged the same two critical issues: 1. No fallback when hook missing = fail-open (now denied) 2. Internal endpoint exposed to network (now localhost-only) Signed-off-by: Tyler Longwell <tlongwell@squareup.com>
Security hardening based on crossfire review (Opus 9/10, Gemini 10/10, Codex 8/10):
Policy endpoint:
- Move input validation before HMAC computation (fail fast on garbage)
- Reject ref names containing whitespace or control characters
- Add future-timestamp rejection (max 5s clock drift tolerance)
- Validate repo_owner against kind:30617 event pubkey (prevents spoofing)
- Check archived channel state before evaluating push rules
- Log unknown protection rules as warnings (forward-compat + typo detection)
HMAC / Hook:
- Add `export LC_ALL=C` for deterministic sort and byte-accurate lengths
- Add env var guards (${VAR:?}) for all relay-injected variables
- Sanitize repo_id and ref_name in JSON body construction
- Include is_ancestor in HMAC (prevents FF/NFF downgrade attacks)
- Length-prefix variable-length fields (prevents concatenation confusion)
Auth:
- Service-bound NIP-98 URL verification (?service=git-upload-pack|git-receive-pack)
- Strict service allowlist — fail closed on unrecognized endpoints
- Clone tokens cannot authenticate push endpoints and vice versa
Transport:
- Force core.hooksPath via GIT_CONFIG_COUNT env (prevents repo-local bypass)
- Verify hook is regular file + executable + not symlink (symlink_metadata)
- Fatal hook installation (repo creation fails if hook can't be installed)
- Request body limit (1 MB) on policy endpoint
Core:
- Fix RecursiveWildcard doc ("one or more" not "zero or more")
- push:role can never weaken built-in defaults (takes max of explicit vs default)
- ParsedProtection struct surfaces unknown rules for caller logging
- Comprehensive HMAC tamper tests (11 tests covering every field)
Signed-off-by: Tyler Longwell <tlongwell@squareup.com>
tlongwell-block
force-pushed
the
feature/git-permissions
branch
from
17848c8 to
7fe98cb
Compare
Bots intentionally added to a channel can now push like members. Protection rules (push:admin, no-force-push, require-patch) still apply. Bot is a designation (what it is), not a permission tier (what it can do). The promotion is scoped to the git policy path only — does not affect the core MemberRole hierarchy or other permission checks.
1. Cross-boundary HMAC integration test — bash script output compared against Rust generate_hook_hmac. Verifies the format agreement that the entire security model depends on. (2 tests, multi-ref + single-ref) 2. read -r in pre-receive hook (defense-in-depth against backslash interp) 3. Gate publish_ref_state on ref diff — snapshot refs before receive-pack, compare after, skip kind:30618 publish if nothing changed. Eliminates info leak on denied pushes and avoids wasted work. 4. Validate HMAC secret length on startup — reject explicitly-configured secrets shorter than 32 chars. Auto-generated dev secrets unaffected. 5. Document bot push story in policy.rs module docs — Bot is a designation, not a permission tier. Promoted to Member at the git policy layer. 6. Document require-patch semantics — blocks ALL ref update kinds including create/delete, not just fast-forward pushes. 7. Fix stale HMAC doc comment — was referencing JSON format, now accurately describes the length-prefixed | separated canonical format.
Removed nested format!() in bash_hmac_single_ref test. Now computes REPO_ID_LEN directly in bash like the actual hook does.
Git's credential protocol does NOT pass query strings to helpers. The credential helper only sees the path component, so it can only sign the bare repo URL. The relay now verifies against the bare repo root. Service-scoping at the NIP-98 level is impossible without git protocol changes. Security is still provided by: - ±60s timestamp window (limits replay) - HTTPS in production (prevents token theft) - Pre-receive hook for push authorization (role + protection rules) - Endpoint routing (clone/push are different HTTP paths) Discovered during live E2E testing with ACP agents.
tlongwell-block
added a commit
that referenced
this pull request
May 1, 2026
## Why Sprout's vision (VISION_SOVEREIGN.md) is a self-hosted relay that serves as your entire workspace — code, conversation, agents, automation. A workspace needs a front door: the relay operator decides who can connect. Without relay-level membership, any pubkey can read and write to the relay. NIP-43 is the Nostr standard for relay access metadata. It defines how relays advertise their member list and how users join/leave. Implementing NIP-43 means standard Nostr clients (Amethyst, Coracle) can discover and interact with Sprout's membership system using the protocol they already speak. This builds on the git repo permissions work (PR #441) — that PR gates who can push to repos; this PR gates who can connect to the relay at all. Together they form the access control stack: relay membership → channel membership → repo permissions. ## What ### Core relay membership system - `relay_members` table with role hierarchy (owner/admin/member) - Admin commands: kind 9030 (add), 9031 (remove), 9032 (change role) - Enforcement at all 7 auth entry points (WebSocket, REST, audio, git, media, tokens, API) — non-members are rejected before any operation - Owner bootstrap from RELAY_OWNER_PUBKEY on first startup - REST API for listing/managing members - Config: SPROUT_REQUIRE_RELAY_MEMBERSHIP, RELAY_OWNER_PUBKEY ### NIP-43 compliance layer (Tier 1) - NIP-11 `self` field: relay signing pubkey advertised (conditional on stable key via SPROUT_RELAY_PRIVATE_KEY) - kind 13534: membership list snapshot, relay-signed, published on startup and after every membership change - kind 8000/8001: member added/removed announcements, relay-signed - kind 28936: leave request handler — members can remove themselves - All relay-signed events carry NIP-70 "-" tag (protected event) - Config guard: hard-fail startup if membership enabled without stable key ### Security - Replay protection: ±120s timestamp window on admin commands and leave requests - Owner lockout prevention: owner cannot leave or be removed - Role enforcement: only admins+ can add/remove, only owner can change roles - Channel-scoped and proxy tokens blocked from admin commands - NIP-70 "-" tag validated on leave requests - Relay-signed events bypass client ingest pipeline (no trust escalation) ### Testing - Unit tests for tag extraction, role validation - E2E test script (scripts/e2e-relay-membership.sh) - Red team exercise: 14/15 tests pass (5 happy path, 5 negative, 5 adversarial) ### Deferred (Tier 2) - Invite flow (kinds 28934/28935) — separate product decision - NIP-70 enforcement on ingest pipeline — defense-in-depth, not blocking - Kind-specific subscription gating — connection-level auth already sufficient 23 files changed, ~2000 insertions
This was referenced May 2, 2026
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
1 participant
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.
Summary
Role-based access control for git push operations, enforced by a pre-receive hook that calls back to the relay's internal policy endpoint. Clone/push tokens are service-scoped to prevent cross-operation replay.
What's included
Permission Engine (
sprout-core/git_perms.rs)*(single segment) and**(recursive) wildcardspush:<role>,no-force-push,no-delete,require-patchPolicy Endpoint (
/internal/git/policy)Pre-receive Hook
set -eo pipefail+export LC_ALL=CAuth Hardening
?service=git-upload-packvsgit-receive-pack)core.hooksPathforced viaGIT_CONFIG_COUNT(prevents repo-local bypass)Bot Role Model
Bots intentionally added to a channel can push like members. The promotion is scoped to the git policy path only —
MemberRole::Botremains atpermission_level = 0in the core hierarchy. Protection rules (push:admin,no-force-push,require-patch) still gate bot pushes the same as any member.Rationale: Bot is a designation (what it is), not a permission tier (what it can do). If a channel owner adds a CI agent to the channel, it should be able to push. Branch protection handles the rest.
Security Review
Crossfire reviewed by 4 models:
Consensus: 7.5/10. No critical security vulnerabilities. Path to 9/10 is ~1 hour of hardening (cross-boundary HMAC test,
read -rin hook, gate kind:30618 on ref diff).Tests
sprout-core(pattern matching, rule parsing, evaluation logic)sprout-relay(tamper detection for every field, order independence)Known Limitations (deferred)
publish_ref_statefires on denied pushes (idempotent but wasteful)git_repo_locksDashMap grows unboundedly (bounded by total repos)readmissing-rflag (safe in practice, git rejects backslashes in refs)Future Work
generate_hook_hmac)git-credential-nostrto sign service-bound URLs (prerequisite for deploy)