Skip to content

Commit 3bf753e

Browse files
TeoSlayerteovl
andauthored
Security hardening batch: updater attestation opt-out, recovery backup, feed repoint, trust-pin TODO (#307)
* updater: add --skip-attestation flag to opt out of SLSA check Threads updater.Config.SkipAttestation (present in pinned updater v0.2.2) through a --skip-attestation flag plus a PILOT_UPDATER_SKIP_ATTESTATION env fallback, mirroring the existing --state-path pattern. Default false keeps provenance verification on; the updater fails closed when gh is absent, so hosts genuinely without gh now have an explicit CLI opt-out. Logs a warning when set. * pilotctl: back up identity.json before recovery overwrite recovery recover replaces the live daemon identity in place via SaveIdentity. Copy any existing identity to identity.json.bak-<unix-ts> first, and refuse the overwrite if the backup cannot be written, so a recovery run can never silently destroy the prior key. Recovery crypto is unchanged. Adds unit tests for the no-file, copy, and unwritable paths. * trustedagents: note pubkey-pinning gap at IsTrusted call sites IsTrusted keys on node_id only, with no pubkey binding. Both web4 call sites are outbound handshake-initiation decisions where the peer's authenticated pubkey is not in scope, so node_id match is all that can be checked locally. Add TODOs documenting that pubkey pinning (IsTrustedWithKey) belongs in the upstream pilot-protocol/trustedagents module at its inbound auto-accept path, where the presented key is available. No behaviour change. * Repoint runtime changelog/motd feeds to pilot-protocol org The pilotctl changelog feed pointed at teoslayer.github.io/pilot-changelog, which now 404s after the TeoSlayer to pilot-protocol org rename, and the daemon MOTD feed used the raw TeoSlayer path. Repoint both to the live pilot-protocol locations (pilot-protocol.github.io and raw.githubusercontent.com/pilot-protocol), plus the matching help text and docs. Historical comments, test fixtures, go.sum, and pilot-skills references (whose repo was not migrated) are left as-is. --------- Co-authored-by: Teodor Calin <teodor@vulturelabs.io>
1 parent 64c0a56 commit 3bf753e

9 files changed

Lines changed: 201 additions & 10 deletions

File tree

cmd/pilotctl/main.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,14 @@ func maybeAutoHandshake(d *driver.Driver, addr protocol.Addr, skip bool) {
550550
}
551551

552552
// Branch 2 — peer is in the embedded trusted-agents allowlist.
553+
//
554+
// TODO(security/H4): IsTrusted keys on node_id only, with no pubkey
555+
// binding. This call is outbound (we initiate toward addr), so the
556+
// peer's authenticated pubkey is not in scope here — node_id match is
557+
// all we can check. Pubkey pinning (IsTrustedWithKey) must be added in
558+
// the upstream github.com/pilot-protocol/trustedagents module and wired
559+
// at the inbound auto-accept path inside its NewService(), where the
560+
// presented key IS available. See that repo, not this call site.
553561
if name, ok := trustedagents.IsTrusted(addr.Node); ok {
554562
if !jsonOutput {
555563
fmt.Fprintf(os.Stderr, "establishing handshake with Trusted Agent %s (%s)...\n", name, addr)
@@ -1351,7 +1359,7 @@ Diagnostic commands:
13511359
pilotctl listen <port> [--count <n>] [--timeout <dur>]
13521360
pilotctl broadcast <network_id> <message>
13531361
pilotctl update [--pin <tag>] run the updater once — check and install new release
1354-
pilotctl updates [--count <n>] [--scope <scope>] read https://teoslayer.github.io/pilot-changelog/feed.xml
1362+
pilotctl updates [--count <n>] [--scope <scope>] read https://pilot-protocol.github.io/pilot-changelog/feed.xml
13551363
13561364
Agent tool discovery:
13571365
pilotctl context

cmd/pilotctl/updates.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ func cmdAutoUpdateStatus() {
9494
// we can stay on the standard library only — no JSON-feed dep needed.
9595
//
9696
// Declared as a var (not const) so tests can point at httptest.Server.
97-
var changelogFeedURL = "https://teoslayer.github.io/pilot-changelog/feed.xml"
97+
var changelogFeedURL = "https://pilot-protocol.github.io/pilot-changelog/feed.xml"
9898

9999
// rssDoc is the minimal RSS 2.0 shape we care about. Only fields needed
100100
// for the human-readable + JSON output are decoded; unknown elements are

cmd/pilotctl/verify.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"encoding/json"
77
"fmt"
88
"os"
9+
"time"
910

1011
"github.com/pilot-protocol/common/badgeverify"
1112
"github.com/pilot-protocol/common/crypto"
@@ -311,6 +312,16 @@ func cmdRecoveryRecover(args []string) {
311312
// The registry rotated the address to the new key; install it locally so
312313
// the daemon can authenticate as the recovered node after a restart.
313314
idPath := flagString(flags, "identity", configDir()+"/identity.json")
315+
// Irreversible-overwrite guard: SaveIdentity replaces the live daemon
316+
// identity in place. If one already exists, copy it aside first so a
317+
// recovery run can never silently destroy the prior key. Refuse rather
318+
// than overwrite blind if the backup can't be written.
319+
if bak, err := backupIdentity(idPath); err != nil {
320+
fatalCode("internal_error",
321+
"recovery recover: refusing to overwrite existing identity %s: backup failed: %v", idPath, err)
322+
} else if bak != "" && !jsonOutput {
323+
fmt.Fprintf(os.Stderr, "backed up existing identity to %s\n", bak)
324+
}
314325
if err := crypto.SaveIdentity(idPath, id); err != nil {
315326
fatalCode("internal_error",
316327
"recovery recover: registry rotated key but installing new identity at %s failed: %v", idPath, err)
@@ -324,3 +335,25 @@ func cmdRecoveryRecover(args []string) {
324335
"registry": resp,
325336
})
326337
}
338+
339+
// backupIdentity copies an existing identity file to
340+
// "<path>.bak-<unix-ts>" before it is overwritten by a recovery run.
341+
// Returns the backup path on success, "" if no file existed at path (so
342+
// there was nothing to back up), or an error if a file exists but the
343+
// backup could not be written — in which case the caller MUST refuse to
344+
// overwrite. Timestamped so repeated recovery attempts never clobber an
345+
// earlier backup. Crypto is untouched: this is a pure file copy.
346+
func backupIdentity(path string) (string, error) {
347+
data, err := os.ReadFile(path)
348+
if err != nil {
349+
if os.IsNotExist(err) {
350+
return "", nil
351+
}
352+
return "", err
353+
}
354+
bakPath := fmt.Sprintf("%s.bak-%d", path, time.Now().Unix())
355+
if err := os.WriteFile(bakPath, data, 0o600); err != nil {
356+
return "", err
357+
}
358+
return bakPath, nil
359+
}
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+
package main
4+
5+
import (
6+
"os"
7+
"path/filepath"
8+
"strings"
9+
"testing"
10+
)
11+
12+
// TestBackupIdentityNoFile: nothing to back up when the path is absent.
13+
// Returns ("", nil) so the caller proceeds to write a fresh identity.
14+
func TestBackupIdentityNoFile(t *testing.T) {
15+
dir := t.TempDir()
16+
path := filepath.Join(dir, "identity.json")
17+
18+
bak, err := backupIdentity(path)
19+
if err != nil {
20+
t.Fatalf("backupIdentity on missing file: unexpected error %v", err)
21+
}
22+
if bak != "" {
23+
t.Fatalf("expected empty backup path for missing file, got %q", bak)
24+
}
25+
}
26+
27+
// TestBackupIdentityCopiesExisting: an existing identity is copied to a
28+
// timestamped .bak file with its contents preserved, and the original is
29+
// left in place for SaveIdentity to overwrite.
30+
func TestBackupIdentityCopiesExisting(t *testing.T) {
31+
dir := t.TempDir()
32+
path := filepath.Join(dir, "identity.json")
33+
const content = `{"node_id":42,"private_key":"old"}`
34+
if err := os.WriteFile(path, []byte(content), 0o600); err != nil {
35+
t.Fatalf("seed identity: %v", err)
36+
}
37+
38+
bak, err := backupIdentity(path)
39+
if err != nil {
40+
t.Fatalf("backupIdentity: unexpected error %v", err)
41+
}
42+
if bak == "" {
43+
t.Fatal("expected a backup path, got empty")
44+
}
45+
if !strings.HasPrefix(bak, path+".bak-") {
46+
t.Fatalf("backup path %q lacks expected prefix %q.bak-", bak, path)
47+
}
48+
49+
got, err := os.ReadFile(bak)
50+
if err != nil {
51+
t.Fatalf("read backup: %v", err)
52+
}
53+
if string(got) != content {
54+
t.Fatalf("backup content mismatch: got %q want %q", got, content)
55+
}
56+
57+
// Original must still exist (we copy, not move) so the overwrite path
58+
// has something to replace.
59+
if _, err := os.Stat(path); err != nil {
60+
t.Fatalf("original identity should remain: %v", err)
61+
}
62+
}
63+
64+
// TestBackupIdentityRefusesUnwritable: when a file exists but the backup
65+
// cannot be written, an error is returned so the caller refuses to
66+
// overwrite the live identity blind.
67+
func TestBackupIdentityRefusesUnwritable(t *testing.T) {
68+
dir := t.TempDir()
69+
path := filepath.Join(dir, "identity.json")
70+
if err := os.WriteFile(path, []byte("x"), 0o600); err != nil {
71+
t.Fatalf("seed identity: %v", err)
72+
}
73+
// Make the directory read-only so the .bak sibling can't be created.
74+
if err := os.Chmod(dir, 0o500); err != nil {
75+
t.Fatalf("chmod dir: %v", err)
76+
}
77+
t.Cleanup(func() { _ = os.Chmod(dir, 0o700) })
78+
79+
if os.Geteuid() == 0 {
80+
t.Skip("running as root bypasses directory permissions")
81+
}
82+
83+
if _, err := backupIdentity(path); err == nil {
84+
t.Fatal("expected error when backup cannot be written, got nil")
85+
}
86+
}

cmd/updater/main.go

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"log/slog"
99
"os"
1010
"os/signal"
11+
"strings"
1112
"syscall"
1213
"time"
1314

@@ -28,6 +29,18 @@ func defaultStatePath() string {
2829
return home + "/.pilot/auto-update.json"
2930
}
3031

32+
// envBool reports whether the named environment variable is set to a truthy
33+
// value ("1", "true", "yes", case-insensitive). Used as the default for
34+
// --skip-attestation so the opt-out can be set without a CLI flag.
35+
func envBool(name string) bool {
36+
switch strings.ToLower(strings.TrimSpace(os.Getenv(name))) {
37+
case "1", "true", "yes", "on":
38+
return true
39+
default:
40+
return false
41+
}
42+
}
43+
3144
func main() {
3245
installDir := flag.String("install-dir", "", "directory containing pilot binaries (required)")
3346
repo := flag.String("repo", "pilot-protocol/pilotprotocol", "GitHub owner/repo for releases")
@@ -37,6 +50,13 @@ func main() {
3750
logFormat := flag.String("log-format", "text", "log format (text, json)")
3851
showVersion := flag.Bool("version", false, "print version and exit")
3952
statePath := flag.String("state-path", defaultStatePath(), "JSON control file {\"enabled\":bool} for automatic updates; auto-update is OFF until enabled (e.g. via `pilotctl update enable`)")
53+
// --skip-attestation opts out of SLSA provenance verification of
54+
// checksums.txt. The updater module fails CLOSED if `gh` is absent (it
55+
// cannot verify attestations), so a host genuinely without `gh` needs an
56+
// explicit way to proceed. Default false: verification stays on in
57+
// production. Mirrors the --state-path pattern with an env fallback.
58+
skipAttestation := flag.Bool("skip-attestation", envBool("PILOT_UPDATER_SKIP_ATTESTATION"),
59+
"skip SLSA attestation verification (default off); use only on hosts without `gh` available")
4060
flag.Parse()
4161

4262
if *showVersion {
@@ -52,12 +72,13 @@ func main() {
5272
setupLogging(*logLevel, *logFormat)
5373

5474
u := updater.New(updater.Config{
55-
CheckInterval: *interval,
56-
Repo: *repo,
57-
InstallDir: *installDir,
58-
Version: version,
59-
PinnedVersion: *pin,
60-
StatePath: *statePath,
75+
CheckInterval: *interval,
76+
Repo: *repo,
77+
InstallDir: *installDir,
78+
Version: version,
79+
PinnedVersion: *pin,
80+
StatePath: *statePath,
81+
SkipAttestation: *skipAttestation,
6182
})
6283

6384
u.Start()
@@ -69,6 +90,9 @@ func main() {
6990
if *pin != "" {
7091
slog.Info("version pinned", "tag", *pin)
7192
}
93+
if *skipAttestation {
94+
slog.Warn("SLSA attestation verification disabled (--skip-attestation); update provenance will NOT be checked")
95+
}
7296

7397
sig := make(chan os.Signal, 1)
7498
signal.Notify(sig, syscall.SIGINT, syscall.SIGTERM)

cmd/updater/zz_envbool_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+
package main
4+
5+
import "testing"
6+
7+
func TestEnvBool(t *testing.T) {
8+
const key = "PILOT_UPDATER_TEST_ENVBOOL"
9+
cases := []struct {
10+
val string
11+
want bool
12+
}{
13+
{"1", true},
14+
{"true", true},
15+
{"TRUE", true},
16+
{"Yes", true},
17+
{" on ", true},
18+
{"0", false},
19+
{"false", false},
20+
{"", false},
21+
{"nope", false},
22+
}
23+
for _, c := range cases {
24+
t.Setenv(key, c.val)
25+
if got := envBool(key); got != c.want {
26+
t.Errorf("envBool(%q) = %v, want %v", c.val, got, c.want)
27+
}
28+
}
29+
30+
// Unset variable is false.
31+
if got := envBool("PILOT_UPDATER_DEFINITELY_UNSET_XYZ"); got {
32+
t.Error("envBool on unset var should be false")
33+
}
34+
}

docs/cli-reference.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ Diagnostic commands:
7777
pilotctl listen <port> [--count <n>] [--timeout <dur>]
7878
pilotctl broadcast <network_id> <message>
7979
pilotctl update [--pin <tag>] run the updater once — check and install new release
80-
pilotctl updates [--count <n>] [--scope <scope>] read https://teoslayer.github.io/pilot-changelog/feed.xml
80+
pilotctl updates [--count <n>] [--scope <scope>] read https://pilot-protocol.github.io/pilot-changelog/feed.xml
8181
8282
Agent tool discovery:
8383
pilotctl context

internal/motd/motd.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ const (
4141
// banner is active and its `title` is the banner text. Publishing or
4242
// clearing a motd entry there propagates to every daemon on its next
4343
// poll (subject to GitHub's raw CDN cache, typically a few minutes).
44-
DefaultFeedURL = "https://raw.githubusercontent.com/TeoSlayer/pilot-changelog/main/feed-motd.json"
44+
DefaultFeedURL = "https://raw.githubusercontent.com/pilot-protocol/pilot-changelog/main/feed-motd.json"
4545

4646
// DefaultInterval is how often the daemon re-fetches the feed when no
4747
// interval is configured.

pkg/daemon/daemon.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3374,6 +3374,12 @@ func (d *Daemon) dialConnectionLocked(ctx context.Context, dstAddr protocol.Addr
33743374
// a nil-deref. The on-wire SYN still goes out below; we just skip the
33753375
// proactive handshake when the plugin isn't loaded.
33763376
if d.handshakes != nil && !d.handshakes.IsTrusted(dstAddr.Node) {
3377+
// TODO(security/H4): IsTrusted keys on node_id only (no pubkey
3378+
// binding). This gate is outbound — we proactively initiate toward
3379+
// dstAddr — so the peer's authenticated pubkey is not in scope and
3380+
// node_id match is all we can check here. Pubkey pinning belongs in
3381+
// the upstream github.com/pilot-protocol/trustedagents module at its
3382+
// inbound auto-accept path, where the presented key is available.
33773383
if _, ok := trustedagents.IsTrusted(dstAddr.Node); ok {
33783384
// Route through HandshakeSendRequest (not the plugin's raw
33793385
// SendRequest) so the per-peer in-flight dedup catches the

0 commit comments

Comments
 (0)