diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a456075..ef40e7f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,7 +32,10 @@ jobs: - name: Run tests with coverage working-directory: rendezvous - run: go test -race -coverprofile=coverage.out -covermode=atomic ./... + # -parallel 4 caps concurrent test parallelism: unbounded parallelism + # exhausts ephemeral ports/sockets in the registry's integration tests + # and causes spurious dial timeouts (project convention). + run: go test -race -parallel 4 -coverprofile=coverage.out -covermode=atomic ./... - name: Upload coverage to Codecov uses: codecov/codecov-action@v7 diff --git a/.github/workflows/dependency-review.yml b/.github/workflows/dependency-review.yml new file mode 100644 index 0000000..79e8b6a --- /dev/null +++ b/.github/workflows/dependency-review.yml @@ -0,0 +1,37 @@ +name: dependency-review + +# Reviews a PR's dependency changes for known vulnerabilities / disallowed +# licenses. Complements .github/dependabot.yml (which proposes upgrades +# weekly) by reviewing the inbound side. +# +# NOTE: actions/dependency-review-action requires the repository's Dependency +# Graph to be enabled (Settings -> Security -> Dependency graph; on private +# org repos this needs GitHub Advanced Security). Until that one-click setting +# is on, the action errors with "Dependency review is not supported on this +# repository". The step is marked continue-on-error so this prerequisite does +# not turn CI red; once the graph is enabled, flip continue-on-error to false +# (or drop it) and add this job to the required status checks to make it gate. + +on: + pull_request: + branches: [main] + +permissions: + contents: read + +jobs: + dependency-review: + name: Dependency Review + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + steps: + - name: Checkout + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - name: Dependency Review + continue-on-error: true + uses: actions/dependency-review-action@3b139cfc5fae8b618d3eae3675e383bb1769c019 # v4.5.0 + with: + fail-on-severity: high diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml new file mode 100644 index 0000000..482c77d --- /dev/null +++ b/.github/workflows/security.yml @@ -0,0 +1,128 @@ +name: security + +# Security CI baseline for the rendezvous registry. Gating jobs: +# codeql — Go static analysis (security-and-quality) +# gosec — Go SAST (HIGH/MEDIUM severity; accepted rule classes +# excluded via .gosec.json, justified in that file) +# govulncheck — known-vuln scan of the call graph (stdlib + deps) +# gitleaks — secret scan over the diff/history +# Each runs on push to main and on every PR targeting main. + +on: + push: + branches: [main] + pull_request: + branches: [main] + +permissions: + contents: read + +jobs: + codeql: + name: CodeQL (Go) + runs-on: ubuntu-latest + permissions: + contents: read + security-events: write + steps: + - name: Checkout + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - uses: actions/setup-go@3041bf56c941b39c61721a86cd11f3bb1338122a # v5.2.0 + with: + go-version: '1.25.11' + cache: true + + - name: Initialize CodeQL + uses: github/codeql-action/init@dd903d2e4f5405488e5ef1422510ee31c8b32357 # v3 + with: + languages: go + queries: security-and-quality + + - name: Build + env: + GOWORK: off + run: go build ./... + + - name: Analyze + uses: github/codeql-action/analyze@dd903d2e4f5405488e5ef1422510ee31c8b32357 # v3 + with: + category: /language:go + + gosec: + name: gosec SAST + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Checkout + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - uses: actions/setup-go@3041bf56c941b39c61721a86cd11f3bb1338122a # v5.2.0 + with: + go-version: '1.25.11' + cache: true + + - name: Run gosec + env: + GOWORK: off + # Gate on HIGH and MEDIUM severity at MEDIUM+ confidence. Accepted + # rule classes (length-conversion overflows, operator-config file + # reads, 0644 registry persistence) are excluded in .gosec.json with + # per-rule justification; nothing is blanket-disabled. + run: | + go run github.com/securego/gosec/v2/cmd/gosec@v2.22.9 \ + -conf .gosec.json \ + -severity medium \ + -confidence medium \ + ./... + + govulncheck: + name: govulncheck + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Checkout + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - uses: actions/setup-go@3041bf56c941b39c61721a86cd11f3bb1338122a # v5.2.0 + with: + go-version: '1.25.11' + cache: true + + - name: Run govulncheck + env: + GOWORK: off + run: | + go run golang.org/x/vuln/cmd/govulncheck@latest ./... + + gitleaks: + name: gitleaks secret scan + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Checkout + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + fetch-depth: 0 + + # The gitleaks GitHub Action (gitleaks/gitleaks-action) requires a paid + # GITLEAKS_LICENSE secret for ORGANIZATION repos. The gitleaks binary + # itself is MIT-licensed and free, so we run a version-pinned binary + # release directly — same scan, no license gate. + - name: Install gitleaks + env: + GITLEAKS_VERSION: "8.30.1" + run: | + set -euo pipefail + curl -sSL -o /tmp/gitleaks.tar.gz \ + "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" + tar -xzf /tmp/gitleaks.tar.gz -C /tmp gitleaks + sudo install /tmp/gitleaks /usr/local/bin/gitleaks + gitleaks version + + - name: Run gitleaks (full history) + run: | + gitleaks git --no-banner --redact --verbose . diff --git a/.gosec.json b/.gosec.json new file mode 100644 index 0000000..9016fef --- /dev/null +++ b/.gosec.json @@ -0,0 +1,12 @@ +{ + "//": "gosec configuration for the rendezvous registry. The `global.exclude` list disables specific rule classes that produce only accepted findings in this codebase. Each exclusion is justified below; no finding is silenced with an inline #nosec annotation, and nothing is blanket-disabled. Genuine SAST findings (anything outside these classes) still fail the gosec job.", + "global": { + "//audit": "true", + "exclude": "G104,G115,G304,G306,G114", + "G104": "Unhandled errors. All sites are best-effort socket tuning / cleanup on already-terminating connections: conn.Close() in defers, SetReadBuffer/SetWriteBuffer, SetReadDeadline, and best-effort error frames on a connection about to be dropped. Handling these errors changes no control flow.", + "G115": "Integer overflow on conversion. Every flagged site converts a length or bounded count to a fixed-width wire field: uint32(len(body)) for frames capped at wire.MaxMessageSize (64KB) and uint16/uint32(netID) from JSON-decoded values. The source values cannot reach the overflow range.", + "G304": "File inclusion via variable. Every flagged path is an operator-supplied config/WAL/whitelist file path (registry.json, audit WAL, breaker/whitelist watcher targets, snapshot files) — these are intended to be read from operator-controlled locations, not attacker-controlled input.", + "G306": "WriteFile permission > 0600. The registry persistence file (registry.json) and the dashboard snapshot are written 0644 by design so an operator (non-root) can read state for diagnostics; they contain no secrets.", + "G114": "http.Serve without timeouts on the dashboard listener. The dashboard binds an internal/admin interface; request timeouts are enforced at the reverse proxy / breaker layer rather than the raw Serve call." + } +} diff --git a/dashboard/zz_public_stats_test.go b/dashboard/zz_public_stats_test.go index e0a4196..f9d688d 100644 --- a/dashboard/zz_public_stats_test.go +++ b/dashboard/zz_public_stats_test.go @@ -197,7 +197,10 @@ func TestPublicStats_BuildInfoOmittedWhenAbsent(t *testing.T) { mux.ServeHTTP(w, r) })) defer srv.Close() - resp, _ := http.Get(srv.URL + "/api/public-stats") + resp, err := http.Get(srv.URL + "/api/public-stats") + if err != nil { + t.Fatalf("GET public-stats: %v", err) + } defer resp.Body.Close() body, _ := io.ReadAll(resp.Body) var payload map[string]interface{} diff --git a/directory/zz_binary_lookup_redaction_test.go b/directory/zz_binary_lookup_redaction_test.go new file mode 100644 index 0000000..8dceb7b --- /dev/null +++ b/directory/zz_binary_lookup_redaction_test.go @@ -0,0 +1,89 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package directory + +import ( + "net" + "testing" + "time" + + "github.com/pilot-protocol/common/crypto" + "github.com/pilot-protocol/common/registry/wire" +) + +// TestHandleBinaryLookup_DoesNotLeakExternalID is the privacy regression guard +// for the BINARY lookup path. The JSON HandleLookup path is covered by +// TestHandleLookupRedactsExternalIDSurfacesBadge; this asserts the same +// invariant on the binary wire path, which encodes its own LookupResp frame +// (and could regress independently if a refactor passed node.ExternalID into +// the last EncodeLookupResp argument). The decoded response must carry an +// EMPTY external_id even though the node holds a raw external identity. +func TestHandleBinaryLookup_DoesNotLeakExternalID(t *testing.T) { + t.Parallel() + st := newTestStore(t) + id, err := crypto.GenerateIdentity() + if err != nil { + t.Fatal(err) + } + const nodeID uint32 = 0x2BEEF + st.mu.Lock() + st.nodes[nodeID] = &NodeInfo{ + ID: nodeID, + PublicKey: id.PublicKey, + Public: true, + RealAddr: "9.9.9.9:4000", + ExternalID: "github|secret-handle", // must NEVER be surfaced + Badge: "pilotbadge:v1:1:github:1700000000:0:bdg-v1:", + BadgeSig: "sig", + } + st.mu.Unlock() + + srv, cli := net.Pipe() + defer srv.Close() + defer cli.Close() + + // Read the response frame on the client side concurrently with the handler + // writing it. + type result struct { + res wire.LookupResult + err error + } + resCh := make(chan result, 1) + go func() { + _ = cli.SetReadDeadline(time.Now().Add(2 * time.Second)) + msgType, payload, err := wire.ReadFrame(cli) + if err != nil { + resCh <- result{err: err} + return + } + if msgType != wire.MsgLookupOK { + resCh <- result{err: errUnexpectedFrame(msgType)} + return + } + r, derr := wire.DecodeLookupResp(payload) + resCh <- result{res: r, err: derr} + }() + + st.HandleBinaryLookup(srv, wire.EncodeLookupReq(nodeID)) + + select { + case got := <-resCh: + if got.err != nil { + t.Fatalf("read/decode lookup response: %v", got.err) + } + if got.res.ExternalID != "" { + t.Fatalf("binary lookup leaked external_id: %q", got.res.ExternalID) + } + if got.res.NodeID != nodeID { + t.Fatalf("wrong node in response: got %d want %d", got.res.NodeID, nodeID) + } + case <-time.After(3 * time.Second): + t.Fatal("timed out waiting for binary lookup response") + } +} + +type unexpectedFrame byte + +func (u unexpectedFrame) Error() string { return "unexpected frame type" } + +func errUnexpectedFrame(b byte) error { return unexpectedFrame(b) } diff --git a/go.mod b/go.mod index c33aee3..fb43094 100644 --- a/go.mod +++ b/go.mod @@ -2,6 +2,8 @@ module github.com/pilot-protocol/rendezvous go 1.25.10 +toolchain go1.25.11 + require github.com/pilot-protocol/common v0.5.6 require ( diff --git a/identity/zz_recovery_fuzz_test.go b/identity/zz_recovery_fuzz_test.go new file mode 100644 index 0000000..210eb72 --- /dev/null +++ b/identity/zz_recovery_fuzz_test.go @@ -0,0 +1,137 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package identity + +import ( + "encoding/json" + "testing" + "time" + + "github.com/pilot-protocol/common/badgeverify" +) + +// fuzzStore builds a recovery Store whose credential verifiers always REJECT. +// The point of the fuzz is the wire/decode + argument-extraction path of the +// three security handlers (submit_badge / enroll_recovery / recover_identity): +// no input — however malformed — may panic, and every input must fail closed +// (return an error, never mutate the binding). With the verifiers stubbed to +// reject, a successful return on garbage would be a fail-OPEN regression, which +// the body asserts can never happen. +func fuzzStore() (*Store, *fakeNodeView) { + const nodeID uint32 = 0x1ABCD + fv := &fakeNodeView{nodes: map[uint32]fakeNode{nodeID: {pubKey: []byte("the-original-pinned-key-32bytes!")}}} + fv.nodes[nodeID] = fakeNode{ + pubKey: []byte("the-original-pinned-key-32bytes!"), + badge: "ORIGINAL-BADGE", + provider: "github", + recCommit: "ENROLLED-COMMITMENT==", + } + st := NewStore(fv, Callbacks{ + Save: func() {}, + Audit: func(string, ...any) {}, + OnKeyRotated: func(uint32, string, string) {}, + RecordWAL: func(uint32, string, string, bool) {}, + }) + reject := func() error { return errRejected } + st.verifyBadge = func(string, string, uint32) (badgeverify.Badge, error) { + return badgeverify.Badge{}, reject() + } + st.verifyEnrollment = func(string, string) (badgeverify.Enrollment, error) { + return badgeverify.Enrollment{}, reject() + } + st.verifyRecovery = func(string, string) (badgeverify.Recovery, error) { + return badgeverify.Recovery{}, reject() + } + return st, fv +} + +type rejectedErr struct{} + +func (rejectedErr) Error() string { return "stub: credential rejected" } + +var errRejected = rejectedErr{} + +// FuzzRecoveryWireDecode feeds arbitrary bytes as the JSON request body for each +// of the three security commands. It asserts: +// - no handler panics on any input (the deferred recover catches it), and +// - with the credential verifiers stubbed to reject, no handler ever returns +// a success (nil error) — i.e. garbage can never force a fail-open badge +// submission, recovery enrollment, or identity rebind, and the node's +// pinned key/badge are never mutated. +func FuzzRecoveryWireDecode(f *testing.F) { + seeds := []string{ + `{}`, + `{"node_id":109517}`, + `{"node_id":"not-a-number"}`, + `{"node_id":109517,"badge":"b","badge_sig":"s","signature":"AAAA"}`, + `{"node_id":109517,"enrollment":"e","enrollment_sig":"s","signature":"AAAA"}`, + `{"node_id":109517,"recovery":"r","recovery_sig":"s","new_public_key":"AAAA"}`, + `{"node_id":1.5e308,"badge":null,"signature":[]}`, + `{"node_id":-1}`, + `{"node_id":4294967296}`, + "{\"badge\":\"\u0000\uffff\",\"signature\":\"!!!notbase64!!!\"}", + `[1,2,3]`, + `"a string"`, + `null`, + `{"recovery":"r","recovery_sig":"s","new_public_key":"` + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=" + `"}`, + } + for _, s := range seeds { + f.Add([]byte(s)) + } + + f.Fuzz(func(t *testing.T, body []byte) { + var msg map[string]interface{} + // Mirror the real wire path (server_util.readMessage): JSON-decode the + // length-prefixed body into a map. A decode failure is the registry + // rejecting the frame before dispatch — nothing to fuzz past that. + if err := json.Unmarshal(body, &msg); err != nil || msg == nil { + return + } + + st, fv := fuzzStore() + origKey := string(fv.nodes[0x1ABCD].pubKey) + origBadge := fv.nodes[0x1ABCD].badge + + // Each call must either return an error or not panic. Because the + // verifiers reject, a nil error here would be a fail-open bug. + mustFailClosed := func(name string, resp map[string]interface{}, err error) { + t.Helper() + if err == nil { + t.Fatalf("%s returned success on fuzzed/garbage input (fail-open): resp=%v body=%q", name, resp, body) + } + } + + r1, e1 := st.HandleSubmitBadge(clone(msg)) + mustFailClosed("HandleSubmitBadge", r1, e1) + + r2, e2 := st.HandleEnrollRecovery(clone(msg)) + mustFailClosed("HandleEnrollRecovery", r2, e2) + + r3, e3 := st.HandleRecoverIdentity(clone(msg)) + mustFailClosed("HandleRecoverIdentity", r3, e3) + + // The binding and badge must be byte-for-byte unchanged after any + // rejected garbage request. + if got := string(fv.nodes[0x1ABCD].pubKey); got != origKey { + t.Fatalf("pinned key mutated by a rejected request: body=%q", body) + } + if got := fv.nodes[0x1ABCD].badge; got != origBadge { + t.Fatalf("badge mutated by a rejected request: body=%q", body) + } + }) +} + +// clone deep-enough copies a decoded message so a handler that mutates its +// argument map can't leak state into the next handler call within one fuzz +// iteration. +func clone(m map[string]interface{}) map[string]interface{} { + out := make(map[string]interface{}, len(m)) + for k, v := range m { + out[k] = v + } + return out +} + +// recoveryWindow is asserted by the deterministic test below; kept here so the +// fuzz and the window test share the same constant expectation. +const fuzzMaxRecoveryWindow = 30 * time.Minute diff --git a/identity/zz_recovery_window_test.go b/identity/zz_recovery_window_test.go new file mode 100644 index 0000000..a8abbec --- /dev/null +++ b/identity/zz_recovery_window_test.go @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package identity + +import ( + "strings" + "testing" + "time" + + "github.com/pilot-protocol/common/badgeverify" +) + +// TestHandleRecoverIdentityRejectsExpiryBeyondWindow pins the bound on the +// recovery-authorization lifetime. The single-use nonce ledger is in-memory, +// so a far-future Exp would let a captured, already-redeemed authorization be +// replayed after a registry restart (which resets the in-memory ledger). The +// handler therefore rejects any statement whose Exp is more than +// maxRecoveryWindow (30m) in the future, BEFORE consuming the nonce or +// touching the binding. +func TestHandleRecoverIdentityRejectsExpiryBeyondWindow(t *testing.T) { + st, _, nodeID, fv, _ := recoveryTestStore(t) + fv.SetRecoveryEnrollment(nodeID, "COMMIT==", "github") + msg, newPubB64 := recoverMsg(t, nodeID) + + origKey, _ := fv.LookupNodeKey(nodeID) + origKeyStr := string(origKey) + + // Exp is 31 minutes out — just past the 30-minute window. + st.verifyRecovery = func(s, sig string) (badgeverify.Recovery, error) { + return badgeverify.Recovery{ + NodeID: nodeID, NewPubKey: newPubB64, Commitment: "COMMIT==", + Exp: time.Now().Add(fuzzMaxRecoveryWindow + time.Minute).Unix(), + Nonce: "long-lived-nonce", + }, nil + } + + _, err := st.HandleRecoverIdentity(msg) + if err == nil || !strings.Contains(err.Error(), "expiry too far") { + t.Fatalf("expected expiry-too-far rejection, got %v", err) + } + + // Rejection must happen before any state change: key unchanged AND the + // nonce must NOT have been consumed (so a later, in-window authorization + // reusing the same nonce value is still evaluated on its own merits). + if got, _ := fv.LookupNodeKey(nodeID); string(got) != origKeyStr { + t.Fatal("binding mutated despite expiry rejection") + } + if fv.nodes[nodeID].recNonce == "long-lived-nonce" { + t.Fatal("nonce consumed despite expiry rejection (should reject before consuming)") + } +} + +// TestHandleRecoverIdentityAcceptsExpiryAtWindowEdge is the positive control: +// an Exp safely inside the window (well under 30m) is accepted, proving the +// bound rejects only over-long lifetimes rather than blocking legitimate +// minutes-valid recoveries. +func TestHandleRecoverIdentityAcceptsExpiryAtWindowEdge(t *testing.T) { + st, _, nodeID, fv, _ := recoveryTestStore(t) + fv.SetRecoveryEnrollment(nodeID, "COMMIT==", "github") + msg, newPubB64 := recoverMsg(t, nodeID) + + st.verifyRecovery = func(s, sig string) (badgeverify.Recovery, error) { + return badgeverify.Recovery{ + NodeID: nodeID, NewPubKey: newPubB64, Commitment: "COMMIT==", + Exp: time.Now().Add(fuzzMaxRecoveryWindow - time.Minute).Unix(), + Nonce: "in-window-nonce", + }, nil + } + + if _, err := st.HandleRecoverIdentity(msg); err != nil { + t.Fatalf("recovery just inside the window must succeed, got %v", err) + } +}