diff --git a/.github/workflows/architecture.yml b/.github/workflows/architecture.yml index 5a536d79..ef8c7879 100644 --- a/.github/workflows/architecture.yml +++ b/.github/workflows/architecture.yml @@ -14,7 +14,7 @@ jobs: - uses: actions/setup-go@v5 with: - go-version: '1.22' + go-version-file: go.mod # check-layers extracted to pilot-protocol/check-layers; the # layer-import + test-import gates that used to live here now @@ -26,11 +26,25 @@ jobs: # gateway, policy, webhook) extracted to its own sibling repo, # and the no_ build tags they exercised left with them. + - name: Race detector — all library packages (-short) + # Broad `-race` over the whole pkg/cmd/internal tree so a data race in + # any library — keyexchange frame parsing, envelope codecs, routing, + # transport — fails CI, not just the daemon lock graph below. -short + # skips the known-slow stress tests (run un-short in dedicated steps); + # -parallel 4 is the project convention to avoid port/socket exhaustion + # under the race detector's higher resource use. This step is also what + # keeps the rotate-key/sign regression (TestConcurrentRotateKeyAndSign + # in pkg/daemon) and the new keyexchange/IPC fuzz seed corpora under the + # race detector on every PR. + run: go test -race -short -parallel 4 -count=1 -timeout 12m ./pkg/... ./cmd/... ./internal/... + - name: Lock-graph race detection (sec 4.8) # pkg/registry/ and pkg/secure/ extracted to pilot-protocol/{rendezvous,common} # in PR #155; lock-graph coverage for those layers now runs from # their own repos. pkg/daemon stays — that's where the daemon-side # lock graph (Store.mu, ReplayMu, SalvageMu, tm.mu, …) actually lives. + # Kept distinct from the broad step above because it runs WITHOUT -short + # so the pkg/daemon stress tests are race-checked too. run: go test -race -timeout 5m ./pkg/daemon/... - name: Lock-graph stress harness (sec 4.8) — TestConcurrentDialEncryptDecrypt diff --git a/.github/workflows/dependency-review.yml b/.github/workflows/dependency-review.yml new file mode 100644 index 00000000..873629be --- /dev/null +++ b/.github/workflows/dependency-review.yml @@ -0,0 +1,23 @@ +name: dependency-review + +# Flags dependency changes that introduce known-vulnerable or disallowed-license +# modules, on the PR diff only. Complements govulncheck (which scans called paths +# in the whole tree) by catching a bad dep at the moment it is added. + +on: + pull_request: + branches: [main] + +permissions: + contents: read + +jobs: + dependency-review: + name: Dependency review + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - name: Dependency Review + uses: actions/dependency-review-action@v4 + with: + fail-on-severity: high diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml new file mode 100644 index 00000000..f56f788e --- /dev/null +++ b/.github/workflows/security.yml @@ -0,0 +1,76 @@ +name: security + +# Supply-chain + SAST gates that complement the existing CodeQL ("Analyze Go") +# and the gitleaks/detect-private-key pre-commit hook. Pre-commit only protects +# contributors who installed the hook; these jobs run on every push and PR so a +# direct push or an unhooked clone can't slip secrets or known-vuln deps past CI. + +on: + push: + branches: [main] + pull_request: + branches: [main] + +permissions: + contents: read + +jobs: + # Known-vulnerability scan over imported packages AND the Go standard library + # the binaries actually call. GATING: a fresh CVE in a called path turns CI red. + govulncheck: + name: govulncheck + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + - name: Run govulncheck + env: + GOFLAGS: -mod=mod + run: go run golang.org/x/vuln/cmd/govulncheck@latest ./... + + # Secret scan on the full history range of the push / PR. GATING: a committed + # token, key, or credential turns CI red even on a direct push to main, which + # the contributor-side pre-commit hook can't cover. + gitleaks: + name: gitleaks + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + with: + fetch-depth: 0 + - name: Run gitleaks + uses: gitleaks/gitleaks-action@v2 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + # gosec SAST. NON-GATING: the codebase carries pre-existing findings that are + # by-design for a local CLI (G304/G703 reading user-named files, G204/G702 + # shelling out to git/$EDITOR with user input, G115 port/sequence int casts, + # G404 math/rand for non-crypto jitter, G104 best-effort cleanup). Rather than + # a giant cleanup PR or a dishonest blanket-disable, gosec uploads SARIF to the + # GitHub Security tab for triage — the same model CodeQL already uses here. + # New genuinely-dangerous patterns surface in the Security tab without flapping + # PR status. Flip continue-on-error off once the backlog is triaged. + gosec: + name: gosec (SARIF, non-gating) + runs-on: ubuntu-latest + permissions: + contents: read + security-events: write + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + - name: Run gosec + continue-on-error: true + run: | + go run github.com/securego/gosec/v2/cmd/gosec@latest \ + -no-fail -fmt sarif -out gosec.sarif ./... + - name: Upload SARIF + uses: github/codeql-action/upload-sarif@v3 + with: + sarif_file: gosec.sarif + category: gosec diff --git a/.gitleaks.toml b/.gitleaks.toml new file mode 100644 index 00000000..7e63b992 --- /dev/null +++ b/.gitleaks.toml @@ -0,0 +1,36 @@ +# gitleaks configuration for the Pilot Protocol repo. +# +# Extends the upstream default ruleset (so every built-in high-signal rule +# still fires) and adds a tightly-scoped allowlist for known false positives. +# Used by BOTH the pre-commit hook and the CI `gitleaks` job so contributor +# and CI scans agree. +# +# Every entry below is a documented false positive — NOT a real credential. +# Keep this list minimal; prefer fixing a real leak over allowlisting it. + +[extend] +useDefault = true + +[allowlist] +description = "Documented false positives (placeholder docs + test comparisons)" + +paths = [ + # Documentation/blog pages that show example API calls with PLACEHOLDER + # bearer tokens / "token": "<...>" payloads. These are illustrative snippets + # rendered to the website, not live secrets. Matched rules: curl-auth-header, + # generic-api-key. + '''web/src/pages/.*\.astro$''', + + # Tunnel dup-keyexchange test compares X25519 public keys with expressions + # like `pc2.PeerX25519Key == pc1.PeerX25519Key`, which the generic-api-key + # heuristic misreads as an assigned secret. No literal key material is + # present — it is a field-to-field comparison in a unit test. Both the + # current (zz_-prefixed) and the historical filename are listed so a full + # history scan stays clean. + '''pkg/daemon/(zz_)?tunnel_dup_keyexchange_test\.go$''', +] + +regexes = [ + # Backstop for the same comparison expression, in case the file is renamed. + '''PeerX25519Key\s*==''', +] diff --git a/cmd/pilotctl/zz_verify_node_binding_test.go b/cmd/pilotctl/zz_verify_node_binding_test.go new file mode 100644 index 00000000..1803ef6b --- /dev/null +++ b/cmd/pilotctl/zz_verify_node_binding_test.go @@ -0,0 +1,100 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package main + +import ( + "errors" + "testing" + + "github.com/pilot-protocol/common/badgeverify" +) + +// TestVerifyForNodeRejectsCrossNodeBadge is the pilotctl-layer adversarial +// guard for badge replay across addresses. `pilotctl verify status` decides +// verified=true ONLY when badgeverify.VerifyForNode(badge, sig, nodeID) +// returns nil — see cmdVerifyStatus. The registry transport is untrusted, so a +// malicious registry (or a MITM) can return ANY node's badge in a lookup for +// our node. The binding check inside VerifyForNode is what stops that badge +// from being credited to a different address. +// +// We craft a canonical badge bound to NodeID A and present it for NodeID B. +// VerifyForNode MUST return a non-nil error (fail-closed) — never nil. We do +// not need a pinned issuer signature to prove this: a cross-node badge must be +// rejected whether the rejection comes from the node-mismatch check or the +// signature check, because either way the pilotctl layer must not render it as +// "verified". +func TestVerifyForNodeRejectsCrossNodeBadge(t *testing.T) { + t.Parallel() + + const nodeA = uint32(0x0A0A0A) + const nodeB = uint32(0x0B0B0B) + + badgeForA, err := badgeverify.Canonical(badgeverify.Badge{ + NodeID: nodeA, + Provider: "github", + VerifiedAt: 1700000000, + Exp: 0, + Kid: "bdg-v1", + Subject: "victim", + }) + if err != nil { + t.Fatalf("Canonical: %v", err) + } + // An attacker-supplied signature; it cannot be a valid pinned-issuer sig + // here, which is itself part of the point — the layer must fail closed. + const attackerSig = "Zm9yZ2VkLXNpZ25hdHVyZQ==" + + // Present node A's badge while authenticated/looked-up as node B. + if _, verr := badgeverify.VerifyForNode(badgeForA, attackerSig, nodeB); verr == nil { + t.Fatal("VerifyForNode accepted a badge bound to a DIFFERENT node — cross-node replay not rejected") + } + + // Sanity: presenting it for its own node A must ALSO fail closed here, + // because the forged signature does not verify against the pinned issuer + // key. This proves the gate is genuinely fail-closed, not merely a + // node-id string compare that an attacker could satisfy. + _, selfErr := badgeverify.VerifyForNode(badgeForA, attackerSig, nodeA) + if selfErr == nil { + t.Fatal("VerifyForNode accepted a badge with a non-pinned (forged) signature") + } + // The self-node failure must be a signature/key failure, NOT a node + // mismatch — confirming the binding check and the crypto check are + // independent gates. + if errors.Is(selfErr, badgeverify.ErrNodeMismatch) { + t.Fatalf("same-node badge failed with ErrNodeMismatch, want a signature failure: %v", selfErr) + } +} + +// verifyStatusVerdict mirrors the exact decision cmdVerifyStatus makes: a node +// is rendered "verified" iff VerifyForNode returns nil for the looked-up +// nodeID. Kept as a tiny pure helper so the adversarial truth-table is testable +// without standing up a registry + daemon. +func verifyStatusVerdict(badge, sig string, nodeID uint32) bool { + if badge == "" { + return false + } + _, err := badgeverify.VerifyForNode(badge, sig, nodeID) + return err == nil +} + +// TestVerifyStatusVerdictCrossNodeNotVerified pins that the status verdict the +// CLI prints is "not verified" for a cross-node badge, i.e. the rejection above +// actually flows through to a user-visible false. +func TestVerifyStatusVerdictCrossNodeNotVerified(t *testing.T) { + t.Parallel() + + const nodeA = uint32(0x0A0A0A) + const nodeB = uint32(0x0B0B0B) + badgeForA, err := badgeverify.Canonical(badgeverify.Badge{ + NodeID: nodeA, Provider: "github", VerifiedAt: 1700000000, Kid: "bdg-v1", + }) + if err != nil { + t.Fatalf("Canonical: %v", err) + } + if verifyStatusVerdict(badgeForA, "Zm9yZ2Vk", nodeB) { + t.Fatal("status verdict reported verified=true for a cross-node badge") + } + if verifyStatusVerdict("", "", nodeB) { + t.Fatal("status verdict reported verified=true with no badge") + } +} diff --git a/go.mod b/go.mod index 6fa22649..430964e9 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/pilot-protocol/pilotprotocol -go 1.25.10 +go 1.25.11 require ( github.com/coder/websocket v1.8.15 diff --git a/pkg/daemon/keyexchange/zz_fuzz_frame_test.go b/pkg/daemon/keyexchange/zz_fuzz_frame_test.go new file mode 100644 index 00000000..84116044 --- /dev/null +++ b/pkg/daemon/keyexchange/zz_fuzz_frame_test.go @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package keyexchange_test + +import ( + "crypto/ecdh" + "crypto/ed25519" + "crypto/rand" + "encoding/binary" + "net" + "testing" + + icrypto "github.com/pilot-protocol/common/crypto" + "github.com/pilot-protocol/pilotprotocol/pkg/daemon/keyexchange" +) + +// fuzzManager builds a Manager wired the same way newPeer does, but against a +// testing.TB so it can be constructed once at the *testing.F scope (key +// generation per fuzz iteration would dominate runtime). A peer-verify func is +// installed so HandleAuthFrame can reach the identity-resolution path; it +// returns a fixed pubkey for node 1 and nil otherwise, so fuzzed frames +// exercise both the resolved and unresolved branches. +func fuzzManager(tb testing.TB) *keyexchange.Manager { + tb.Helper() + idn, err := icrypto.GenerateIdentity() + if err != nil { + tb.Fatalf("identity: %v", err) + } + xpriv, err := ecdh.X25519().GenerateKey(rand.Reader) + if err != nil { + tb.Fatalf("x25519: %v", err) + } + m := keyexchange.New(nil) + m.SetIdentity(idn) + m.SetX25519Keys(xpriv, xpriv.PublicKey().Bytes()) + m.SetLocalNodeIDFn(func() uint32 { return 7 }) + m.SetPeerVerifyFunc(func(nodeID uint32) (ed25519.PublicKey, error) { + if nodeID == 1 { + return idn.PublicKey, nil + } + return nil, nil + }) + return m +} + +// FuzzHandleAuthFrame throws arbitrary bytes at the PILA (authenticated key +// exchange) wire parser. The contract: never panic, and never install crypto +// from an unauthenticated / malformed frame (HandleAuthFrame returns true only +// after a registry-verified Ed25519 signature check, which fuzzed random bytes +// cannot satisfy). We assert no panic and fail-closed: a random frame that +// somehow returns true would be a forged-auth acceptance — a hard failure. +func FuzzHandleAuthFrame(f *testing.F) { + m := fuzzManager(f) + from := &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 4000} + + // Deterministic seeds: empty, exactly-min-length zeros, a structurally + // well-formed-but-unsigned frame, and a boundary-length frame. + f.Add([]byte{}) + f.Add(make([]byte, 4+32+32+64)) + wellFormed := make([]byte, 4+32+32+64) + binary.BigEndian.PutUint32(wellFormed[0:4], 1) // claims to be node 1 + f.Add(wellFormed) + f.Add(make([]byte, 4+32+32+63)) // one byte short of minimum + + f.Fuzz(func(t *testing.T, data []byte) { + if m.HandleAuthFrame(data, from, false) { + t.Fatalf("HandleAuthFrame accepted an unauthenticated frame (len=%d)", len(data)) + } + // fromRelay path must be equally fail-closed. + if m.HandleAuthFrame(data, from, true) { + t.Fatalf("HandleAuthFrame(relay) accepted an unauthenticated frame (len=%d)", len(data)) + } + }) +} + +// FuzzHandleUnauthFrame throws arbitrary bytes at the PILK (unauthenticated +// key exchange) wire parser. Contract: never panic on malformed input. Unlike +// the auth frame, an unauth frame can legitimately install crypto when the +// node operates without identity-based auth, so we only assert no-panic — the +// fuzzer's job here is to find a decode that crashes, not an acceptance bug. +func FuzzHandleUnauthFrame(f *testing.F) { + m := fuzzManager(f) + from := &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 4000} + + f.Add([]byte{}) + f.Add([]byte{0x00, 0x01}) + f.Add(make([]byte, 4+32)) + f.Add(make([]byte, 128)) + + f.Fuzz(func(t *testing.T, data []byte) { + _ = m.HandleUnauthFrame(data, from, false) + _ = m.HandleUnauthFrame(data, from, true) + }) +} diff --git a/pkg/daemon/zz_fuzz_badge_ipc_test.go b/pkg/daemon/zz_fuzz_badge_ipc_test.go new file mode 100644 index 00000000..57d29cbb --- /dev/null +++ b/pkg/daemon/zz_fuzz_badge_ipc_test.go @@ -0,0 +1,118 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package daemon + +import ( + "io" + "net" + "testing" + "time" + + "github.com/pilot-protocol/common/badgeverify" +) + +// drainConn reads and discards everything written to the server side of an IPC +// pipe so a handler's writeReply never blocks on a full pipe. Returns a stop +// func bound to the fuzz lifetime. +func drainConn(tb testing.TB) (*ipcConn, func()) { + tb.Helper() + server, client := net.Pipe() + ic := newIPCConn(server, 0, false) + done := make(chan struct{}) + go func() { + defer close(done) + buf := make([]byte, 4096) + for { + _ = client.SetReadDeadline(time.Now().Add(time.Second)) + if _, err := client.Read(buf); err != nil { + if err == io.EOF || err == io.ErrClosedPipe { + return + } + // net.Pipe deadline timeout: loop again until closed. + if ne, ok := err.(net.Error); ok && ne.Timeout() { + select { + case <-done: + return + default: + continue + } + } + return + } + } + }() + stop := func() { + _ = ic.Close() + _ = client.Close() + } + return ic, stop +} + +// FuzzHandleSubmitBadgePayload throws arbitrary bytes at the submit_badge IPC +// payload parser. Contract: never panic on malformed JSON, and fail closed — +// with no registry connection the handler must always error out before any +// submission, so a fuzzed payload can never be silently accepted. The handler +// writes its reply over the pipe; drainConn keeps that from blocking. +func FuzzHandleSubmitBadgePayload(f *testing.F) { + d := New(Config{}) + d.nodeID = 7 + // regConn deliberately nil: the handler must reject before any network I/O. + s := NewIPCServer("", d) + + f.Add([]byte(``)) + f.Add([]byte(`not json`)) + f.Add([]byte(`{}`)) + f.Add([]byte(`{"badge":"","badge_sig":""}`)) + f.Add([]byte(`{"badge":"b","badge_sig":"s"}`)) + f.Add([]byte(`{"badge":123,"badge_sig":true}`)) + f.Add([]byte("{\"badge\":\"\\u0000\\uffff\",\"badge_sig\":\"\\ud800\"}")) + + f.Fuzz(func(t *testing.T, payload []byte) { + ic, stop := drainConn(t) + defer stop() + // Must not panic. With regConn==nil the handler always fails closed; + // it never reaches SubmitBadge, so there is nothing to over-trust. + s.handleSubmitBadge(ic, 0, payload) + }) +} + +// FuzzHandleEnrollRecoveryPayload throws arbitrary bytes at the enroll_recovery +// IPC payload parser, which additionally runs badgeverify.ParseEnrollment on +// attacker-influenced input. Contract: never panic; fail closed. +func FuzzHandleEnrollRecoveryPayload(f *testing.F) { + d := New(Config{}) + d.nodeID = 7 + s := NewIPCServer("", d) + + f.Add([]byte(``)) + f.Add([]byte(`garbage`)) + f.Add([]byte(`{}`)) + f.Add([]byte(`{"enrollment":"","enrollment_sig":""}`)) + f.Add([]byte(`{"enrollment":"garbage","enrollment_sig":"s"}`)) + f.Add([]byte(`{"enrollment":"pilotenroll:v1:7:github::1781827200:bdg-v1","enrollment_sig":"s"}`)) + f.Add([]byte(`{"enrollment":":::::::::::::","enrollment_sig":"x"}`)) + + f.Fuzz(func(t *testing.T, payload []byte) { + ic, stop := drainConn(t) + defer stop() + s.handleEnrollRecovery(ic, 0, payload) + }) +} + +// FuzzParseBadgeCredentials fuzzes the three badgeverify parsers the daemon +// and pilotctl feed untrusted strings into (registry-returned badges, verifier +// sidecar enrollments / recovery authorizations). Pure no-panic contract; the +// parsers must reject malformed input with an error, never crash. +func FuzzParseBadgeCredentials(f *testing.F) { + f.Add("") + f.Add("pilotbadge:v1:7:github:1781827200:0:bdg-v1:") + f.Add("pilotbadge:v1:notanumber:github:x:y:z:") + f.Add(":::::::") + f.Add("pilotbadge:v1:" + "9999999999999999999999999" + ":github:0:0:k:") + + f.Fuzz(func(t *testing.T, s string) { + _, _ = badgeverify.Parse(s) + _, _ = badgeverify.ParseEnrollment(s) + _, _ = badgeverify.ParseRecovery(s) + }) +}