Skip to content

Commit 73d47ef

Browse files
TeoSlayerteovl
andauthored
Add supply-chain CI gates and adversarial fuzz evals (#311)
Fill the security-CI gaps that CodeQL (default query suite) and the gitleaks pre-commit hook don't cover, and add adversarial/fuzz EVAL coverage for the security-critical trust boundaries. CI: - security.yml: govulncheck (gating), gitleaks (gating, push+PR), and gosec SARIF upload (non-gating, GitHub Security tab) — the pre-commit hook can't protect direct pushes or unhooked clones. - dependency-review.yml: flags vulnerable/disallowed-license deps on PRs. - architecture.yml: add a broad -race -short job over pkg/cmd/internal (-parallel 4) so any library data race fails CI, not just the daemon lock graph; pin setup-go to go.mod. - Bump go.mod to go 1.25.11 to clear GO-2026-5037 / GO-2026-5039 stdlib vulns that govulncheck flags as called from telemetry/appstore. - .gitleaks.toml: documented allowlist for 11 history false positives (placeholder bearer tokens in .astro docs, an X25519 field comparison in a tunnel test). Tests: - keyexchange frame fuzzers (PILA/PILK): no panic, fail-closed (a random auth frame must never be accepted). - badge/recovery IPC payload fuzzers + badgeverify parser fuzzer: no panic, fail-closed with no registry connection. - pilotctl adversarial test: a badge bound to a different NodeID is rejected by VerifyForNode, and a forged-signature badge for its own node still fails closed. Co-authored-by: Teodor Calin <teodor@vulturelabs.io>
1 parent 91fa07c commit 73d47ef

8 files changed

Lines changed: 463 additions & 2 deletions

File tree

.github/workflows/architecture.yml

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ jobs:
1414

1515
- uses: actions/setup-go@v5
1616
with:
17-
go-version: '1.22'
17+
go-version-file: go.mod
1818

1919
# check-layers extracted to pilot-protocol/check-layers; the
2020
# layer-import + test-import gates that used to live here now
@@ -26,11 +26,25 @@ jobs:
2626
# gateway, policy, webhook) extracted to its own sibling repo,
2727
# and the no_<plugin> build tags they exercised left with them.
2828

29+
- name: Race detector — all library packages (-short)
30+
# Broad `-race` over the whole pkg/cmd/internal tree so a data race in
31+
# any library — keyexchange frame parsing, envelope codecs, routing,
32+
# transport — fails CI, not just the daemon lock graph below. -short
33+
# skips the known-slow stress tests (run un-short in dedicated steps);
34+
# -parallel 4 is the project convention to avoid port/socket exhaustion
35+
# under the race detector's higher resource use. This step is also what
36+
# keeps the rotate-key/sign regression (TestConcurrentRotateKeyAndSign
37+
# in pkg/daemon) and the new keyexchange/IPC fuzz seed corpora under the
38+
# race detector on every PR.
39+
run: go test -race -short -parallel 4 -count=1 -timeout 12m ./pkg/... ./cmd/... ./internal/...
40+
2941
- name: Lock-graph race detection (sec 4.8)
3042
# pkg/registry/ and pkg/secure/ extracted to pilot-protocol/{rendezvous,common}
3143
# in PR #155; lock-graph coverage for those layers now runs from
3244
# their own repos. pkg/daemon stays — that's where the daemon-side
3345
# lock graph (Store.mu, ReplayMu, SalvageMu, tm.mu, …) actually lives.
46+
# Kept distinct from the broad step above because it runs WITHOUT -short
47+
# so the pkg/daemon stress tests are race-checked too.
3448
run: go test -race -timeout 5m ./pkg/daemon/...
3549

3650
- name: Lock-graph stress harness (sec 4.8) — TestConcurrentDialEncryptDecrypt
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
name: dependency-review
2+
3+
# Flags dependency changes that introduce known-vulnerable or disallowed-license
4+
# modules, on the PR diff only. Complements govulncheck (which scans called paths
5+
# in the whole tree) by catching a bad dep at the moment it is added.
6+
7+
on:
8+
pull_request:
9+
branches: [main]
10+
11+
permissions:
12+
contents: read
13+
14+
jobs:
15+
dependency-review:
16+
name: Dependency review
17+
runs-on: ubuntu-latest
18+
steps:
19+
- uses: actions/checkout@v7
20+
- name: Dependency Review
21+
uses: actions/dependency-review-action@v4
22+
with:
23+
fail-on-severity: high

.github/workflows/security.yml

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
name: security
2+
3+
# Supply-chain + SAST gates that complement the existing CodeQL ("Analyze Go")
4+
# and the gitleaks/detect-private-key pre-commit hook. Pre-commit only protects
5+
# contributors who installed the hook; these jobs run on every push and PR so a
6+
# direct push or an unhooked clone can't slip secrets or known-vuln deps past CI.
7+
8+
on:
9+
push:
10+
branches: [main]
11+
pull_request:
12+
branches: [main]
13+
14+
permissions:
15+
contents: read
16+
17+
jobs:
18+
# Known-vulnerability scan over imported packages AND the Go standard library
19+
# the binaries actually call. GATING: a fresh CVE in a called path turns CI red.
20+
govulncheck:
21+
name: govulncheck
22+
runs-on: ubuntu-latest
23+
steps:
24+
- uses: actions/checkout@v7
25+
- uses: actions/setup-go@v5
26+
with:
27+
go-version-file: go.mod
28+
- name: Run govulncheck
29+
env:
30+
GOFLAGS: -mod=mod
31+
run: go run golang.org/x/vuln/cmd/govulncheck@latest ./...
32+
33+
# Secret scan on the full history range of the push / PR. GATING: a committed
34+
# token, key, or credential turns CI red even on a direct push to main, which
35+
# the contributor-side pre-commit hook can't cover.
36+
gitleaks:
37+
name: gitleaks
38+
runs-on: ubuntu-latest
39+
steps:
40+
- uses: actions/checkout@v7
41+
with:
42+
fetch-depth: 0
43+
- name: Run gitleaks
44+
uses: gitleaks/gitleaks-action@v2
45+
env:
46+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
47+
48+
# gosec SAST. NON-GATING: the codebase carries pre-existing findings that are
49+
# by-design for a local CLI (G304/G703 reading user-named files, G204/G702
50+
# shelling out to git/$EDITOR with user input, G115 port/sequence int casts,
51+
# G404 math/rand for non-crypto jitter, G104 best-effort cleanup). Rather than
52+
# a giant cleanup PR or a dishonest blanket-disable, gosec uploads SARIF to the
53+
# GitHub Security tab for triage — the same model CodeQL already uses here.
54+
# New genuinely-dangerous patterns surface in the Security tab without flapping
55+
# PR status. Flip continue-on-error off once the backlog is triaged.
56+
gosec:
57+
name: gosec (SARIF, non-gating)
58+
runs-on: ubuntu-latest
59+
permissions:
60+
contents: read
61+
security-events: write
62+
steps:
63+
- uses: actions/checkout@v7
64+
- uses: actions/setup-go@v5
65+
with:
66+
go-version-file: go.mod
67+
- name: Run gosec
68+
continue-on-error: true
69+
run: |
70+
go run github.com/securego/gosec/v2/cmd/gosec@latest \
71+
-no-fail -fmt sarif -out gosec.sarif ./...
72+
- name: Upload SARIF
73+
uses: github/codeql-action/upload-sarif@v3
74+
with:
75+
sarif_file: gosec.sarif
76+
category: gosec

.gitleaks.toml

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
# gitleaks configuration for the Pilot Protocol repo.
2+
#
3+
# Extends the upstream default ruleset (so every built-in high-signal rule
4+
# still fires) and adds a tightly-scoped allowlist for known false positives.
5+
# Used by BOTH the pre-commit hook and the CI `gitleaks` job so contributor
6+
# and CI scans agree.
7+
#
8+
# Every entry below is a documented false positive — NOT a real credential.
9+
# Keep this list minimal; prefer fixing a real leak over allowlisting it.
10+
11+
[extend]
12+
useDefault = true
13+
14+
[allowlist]
15+
description = "Documented false positives (placeholder docs + test comparisons)"
16+
17+
paths = [
18+
# Documentation/blog pages that show example API calls with PLACEHOLDER
19+
# bearer tokens / "token": "<...>" payloads. These are illustrative snippets
20+
# rendered to the website, not live secrets. Matched rules: curl-auth-header,
21+
# generic-api-key.
22+
'''web/src/pages/.*\.astro$''',
23+
24+
# Tunnel dup-keyexchange test compares X25519 public keys with expressions
25+
# like `pc2.PeerX25519Key == pc1.PeerX25519Key`, which the generic-api-key
26+
# heuristic misreads as an assigned secret. No literal key material is
27+
# present — it is a field-to-field comparison in a unit test. Both the
28+
# current (zz_-prefixed) and the historical filename are listed so a full
29+
# history scan stays clean.
30+
'''pkg/daemon/(zz_)?tunnel_dup_keyexchange_test\.go$''',
31+
]
32+
33+
regexes = [
34+
# Backstop for the same comparison expression, in case the file is renamed.
35+
'''PeerX25519Key\s*==''',
36+
]
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+
package main
4+
5+
import (
6+
"errors"
7+
"testing"
8+
9+
"github.com/pilot-protocol/common/badgeverify"
10+
)
11+
12+
// TestVerifyForNodeRejectsCrossNodeBadge is the pilotctl-layer adversarial
13+
// guard for badge replay across addresses. `pilotctl verify status` decides
14+
// verified=true ONLY when badgeverify.VerifyForNode(badge, sig, nodeID)
15+
// returns nil — see cmdVerifyStatus. The registry transport is untrusted, so a
16+
// malicious registry (or a MITM) can return ANY node's badge in a lookup for
17+
// our node. The binding check inside VerifyForNode is what stops that badge
18+
// from being credited to a different address.
19+
//
20+
// We craft a canonical badge bound to NodeID A and present it for NodeID B.
21+
// VerifyForNode MUST return a non-nil error (fail-closed) — never nil. We do
22+
// not need a pinned issuer signature to prove this: a cross-node badge must be
23+
// rejected whether the rejection comes from the node-mismatch check or the
24+
// signature check, because either way the pilotctl layer must not render it as
25+
// "verified".
26+
func TestVerifyForNodeRejectsCrossNodeBadge(t *testing.T) {
27+
t.Parallel()
28+
29+
const nodeA = uint32(0x0A0A0A)
30+
const nodeB = uint32(0x0B0B0B)
31+
32+
badgeForA, err := badgeverify.Canonical(badgeverify.Badge{
33+
NodeID: nodeA,
34+
Provider: "github",
35+
VerifiedAt: 1700000000,
36+
Exp: 0,
37+
Kid: "bdg-v1",
38+
Subject: "victim",
39+
})
40+
if err != nil {
41+
t.Fatalf("Canonical: %v", err)
42+
}
43+
// An attacker-supplied signature; it cannot be a valid pinned-issuer sig
44+
// here, which is itself part of the point — the layer must fail closed.
45+
const attackerSig = "Zm9yZ2VkLXNpZ25hdHVyZQ=="
46+
47+
// Present node A's badge while authenticated/looked-up as node B.
48+
if _, verr := badgeverify.VerifyForNode(badgeForA, attackerSig, nodeB); verr == nil {
49+
t.Fatal("VerifyForNode accepted a badge bound to a DIFFERENT node — cross-node replay not rejected")
50+
}
51+
52+
// Sanity: presenting it for its own node A must ALSO fail closed here,
53+
// because the forged signature does not verify against the pinned issuer
54+
// key. This proves the gate is genuinely fail-closed, not merely a
55+
// node-id string compare that an attacker could satisfy.
56+
_, selfErr := badgeverify.VerifyForNode(badgeForA, attackerSig, nodeA)
57+
if selfErr == nil {
58+
t.Fatal("VerifyForNode accepted a badge with a non-pinned (forged) signature")
59+
}
60+
// The self-node failure must be a signature/key failure, NOT a node
61+
// mismatch — confirming the binding check and the crypto check are
62+
// independent gates.
63+
if errors.Is(selfErr, badgeverify.ErrNodeMismatch) {
64+
t.Fatalf("same-node badge failed with ErrNodeMismatch, want a signature failure: %v", selfErr)
65+
}
66+
}
67+
68+
// verifyStatusVerdict mirrors the exact decision cmdVerifyStatus makes: a node
69+
// is rendered "verified" iff VerifyForNode returns nil for the looked-up
70+
// nodeID. Kept as a tiny pure helper so the adversarial truth-table is testable
71+
// without standing up a registry + daemon.
72+
func verifyStatusVerdict(badge, sig string, nodeID uint32) bool {
73+
if badge == "" {
74+
return false
75+
}
76+
_, err := badgeverify.VerifyForNode(badge, sig, nodeID)
77+
return err == nil
78+
}
79+
80+
// TestVerifyStatusVerdictCrossNodeNotVerified pins that the status verdict the
81+
// CLI prints is "not verified" for a cross-node badge, i.e. the rejection above
82+
// actually flows through to a user-visible false.
83+
func TestVerifyStatusVerdictCrossNodeNotVerified(t *testing.T) {
84+
t.Parallel()
85+
86+
const nodeA = uint32(0x0A0A0A)
87+
const nodeB = uint32(0x0B0B0B)
88+
badgeForA, err := badgeverify.Canonical(badgeverify.Badge{
89+
NodeID: nodeA, Provider: "github", VerifiedAt: 1700000000, Kid: "bdg-v1",
90+
})
91+
if err != nil {
92+
t.Fatalf("Canonical: %v", err)
93+
}
94+
if verifyStatusVerdict(badgeForA, "Zm9yZ2Vk", nodeB) {
95+
t.Fatal("status verdict reported verified=true for a cross-node badge")
96+
}
97+
if verifyStatusVerdict("", "", nodeB) {
98+
t.Fatal("status verdict reported verified=true with no badge")
99+
}
100+
}

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module github.com/pilot-protocol/pilotprotocol
22

3-
go 1.25.10
3+
go 1.25.11
44

55
require (
66
github.com/coder/websocket v1.8.15
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+
package keyexchange_test
4+
5+
import (
6+
"crypto/ecdh"
7+
"crypto/ed25519"
8+
"crypto/rand"
9+
"encoding/binary"
10+
"net"
11+
"testing"
12+
13+
icrypto "github.com/pilot-protocol/common/crypto"
14+
"github.com/pilot-protocol/pilotprotocol/pkg/daemon/keyexchange"
15+
)
16+
17+
// fuzzManager builds a Manager wired the same way newPeer does, but against a
18+
// testing.TB so it can be constructed once at the *testing.F scope (key
19+
// generation per fuzz iteration would dominate runtime). A peer-verify func is
20+
// installed so HandleAuthFrame can reach the identity-resolution path; it
21+
// returns a fixed pubkey for node 1 and nil otherwise, so fuzzed frames
22+
// exercise both the resolved and unresolved branches.
23+
func fuzzManager(tb testing.TB) *keyexchange.Manager {
24+
tb.Helper()
25+
idn, err := icrypto.GenerateIdentity()
26+
if err != nil {
27+
tb.Fatalf("identity: %v", err)
28+
}
29+
xpriv, err := ecdh.X25519().GenerateKey(rand.Reader)
30+
if err != nil {
31+
tb.Fatalf("x25519: %v", err)
32+
}
33+
m := keyexchange.New(nil)
34+
m.SetIdentity(idn)
35+
m.SetX25519Keys(xpriv, xpriv.PublicKey().Bytes())
36+
m.SetLocalNodeIDFn(func() uint32 { return 7 })
37+
m.SetPeerVerifyFunc(func(nodeID uint32) (ed25519.PublicKey, error) {
38+
if nodeID == 1 {
39+
return idn.PublicKey, nil
40+
}
41+
return nil, nil
42+
})
43+
return m
44+
}
45+
46+
// FuzzHandleAuthFrame throws arbitrary bytes at the PILA (authenticated key
47+
// exchange) wire parser. The contract: never panic, and never install crypto
48+
// from an unauthenticated / malformed frame (HandleAuthFrame returns true only
49+
// after a registry-verified Ed25519 signature check, which fuzzed random bytes
50+
// cannot satisfy). We assert no panic and fail-closed: a random frame that
51+
// somehow returns true would be a forged-auth acceptance — a hard failure.
52+
func FuzzHandleAuthFrame(f *testing.F) {
53+
m := fuzzManager(f)
54+
from := &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 4000}
55+
56+
// Deterministic seeds: empty, exactly-min-length zeros, a structurally
57+
// well-formed-but-unsigned frame, and a boundary-length frame.
58+
f.Add([]byte{})
59+
f.Add(make([]byte, 4+32+32+64))
60+
wellFormed := make([]byte, 4+32+32+64)
61+
binary.BigEndian.PutUint32(wellFormed[0:4], 1) // claims to be node 1
62+
f.Add(wellFormed)
63+
f.Add(make([]byte, 4+32+32+63)) // one byte short of minimum
64+
65+
f.Fuzz(func(t *testing.T, data []byte) {
66+
if m.HandleAuthFrame(data, from, false) {
67+
t.Fatalf("HandleAuthFrame accepted an unauthenticated frame (len=%d)", len(data))
68+
}
69+
// fromRelay path must be equally fail-closed.
70+
if m.HandleAuthFrame(data, from, true) {
71+
t.Fatalf("HandleAuthFrame(relay) accepted an unauthenticated frame (len=%d)", len(data))
72+
}
73+
})
74+
}
75+
76+
// FuzzHandleUnauthFrame throws arbitrary bytes at the PILK (unauthenticated
77+
// key exchange) wire parser. Contract: never panic on malformed input. Unlike
78+
// the auth frame, an unauth frame can legitimately install crypto when the
79+
// node operates without identity-based auth, so we only assert no-panic — the
80+
// fuzzer's job here is to find a decode that crashes, not an acceptance bug.
81+
func FuzzHandleUnauthFrame(f *testing.F) {
82+
m := fuzzManager(f)
83+
from := &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 4000}
84+
85+
f.Add([]byte{})
86+
f.Add([]byte{0x00, 0x01})
87+
f.Add(make([]byte, 4+32))
88+
f.Add(make([]byte, 128))
89+
90+
f.Fuzz(func(t *testing.T, data []byte) {
91+
_ = m.HandleUnauthFrame(data, from, false)
92+
_ = m.HandleUnauthFrame(data, from, true)
93+
})
94+
}

0 commit comments

Comments
 (0)