fix(guard/guardcore): fail closed when the policy signature cannot be trusted

packages/crypto/crypto_test.go

@@ -176,4 +176,16 @@ func TestFingerprintIsStableAndDistinct(t *testing.T) {
 	if Fingerprint(kp1.Public) == Fingerprint(kp2.Public) {
 		t.Fatal("fingerprint should differ between distinct keys")
 	}
-}
+}
+func TestHasKeyPair(t *testing.T) {
+	dir := t.TempDir()
+	if HasKeyPair(dir) {
+		t.Error("HasKeyPair = true on an empty directory, want false")
+	}
+	if _, err := LoadOrCreateKeyPair(dir); err != nil {
+		t.Fatal(err)
+	}
+	if !HasKeyPair(dir) {
+		t.Error("HasKeyPair = false after LoadOrCreateKeyPair created the pair, want true")
+	}
+}

packages/crypto/keys.go

@@ -38,6 +38,20 @@ func GenerateKeyPair() (KeyPair, error) {
 	return KeyPair{Public: pub, Private: priv}, nil
 }
 
+// HasKeyPair reports whether this machine already has a signing key pair in
+// dir. Callers use it to tell a first run without a signing identity (where an
+// unsigned document can only be legacy) from a machine that has signed before
+// (where an unsigned document has had its signature stripped).
+func HasKeyPair(dir string) bool {
+	if _, err := os.Stat(filepath.Join(dir, privateKeyFile)); err != nil {
+		return false
+	}
+	if _, err := os.Stat(filepath.Join(dir, publicKeyFile)); err != nil {
+		return false
+	}
+	return true
+}
+
 // LoadOrCreateKeyPair reads the machine signing key from dir, generating and
 // persisting a new one on first use. The private key is written with 0600
 // permissions; the public key with 0644 so it can be read for verification

packages/guard/guardcore/config.go

@@ -6,6 +6,7 @@ package guardcore
 import (
 	"encoding/json"
 	"errors"
+	"fmt"
 	"os"
 	"path/filepath"
 
@@ -30,14 +31,58 @@ func securityInsidePolicy(doc []byte) (*shared.Security, bool) {
 	return &sec, true
 }
 
-// ErrPolicyTampered is returned when the home policy.json carries a
-// signature that fails verification against this machine's recorded key -
-// a direct rewrite that bypassed SolonGate entirely. Project-local policy
-// files are never signature-checked: LoadLocalPolicyFile already strips
-// their Security/SelfProtect fields below, so they carry no trust to forge.
+// ErrPolicyTampered is returned when the home policy.json cannot be shown to be
+// the file SolonGate wrote: the signature is missing on a machine that has a
+// signing key, does not verify against this machine's key, cannot be decoded,
+// or cannot be checked at all. Every one of those cases used to fall through to
+// "no verification required" and load the file anyway, which let anyone who
+// could rewrite policy.json - or merely strip its signature - have an
+// untrusted policy enforced. Project-local policy files are never
+// signature-checked: LoadLocalPolicyFile already strips their Security/
+// SelfProtect fields below, so they carry no trust to forge.
 var ErrPolicyTampered = errors.New("policy.json failed signature verification: " +
 	"it was modified outside SolonGate; refusing to trust it")
 
+// verifyHomePolicy fails closed: the only outcomes that let the home policy be
+// applied are a signature that verifies against this machine's key, or an
+// unsigned file on a machine that has no signing identity yet - a policy written
+// before signatures existed. A key that exists but cannot be loaded, a signature
+// that cannot be decoded, a signature that does not verify, and a stripped
+// signature on a keyed machine are all treated as tampering.
+func verifyHomePolicy(b []byte) error {
+	dir := filepath.Join(shared.SGDir(), "crypto")
+	if !pcrypto.HasKeyPair(dir) {
+		// Reading a policy must not create keys. LoadOrCreateKeyPair would make
+		// the first run key this install, and then its own unsigned policy would
+		// be rejected on the second run.
+		_, signed, _, verr := pcrypto.VerifyDocument(b, nil)
+		switch {
+		case signed:
+			// Signed by something, and there is no key here to check it against.
+			// That is not a legacy file, so it is not ours.
+			return fmt.Errorf("%w (it carries a signature, but this machine has no signing key to check it with)", ErrPolicyTampered)
+		case verr != nil:
+			return fmt.Errorf("%w (it is not a readable policy document: %v)", ErrPolicyTampered, verr)
+		}
+		return nil
+	}
+
+	kp, err := pcrypto.LoadOrCreateKeyPair(dir)
+	if err != nil {
+		return fmt.Errorf("%w (the machine signing key could not be loaded: %v)", ErrPolicyTampered, err)
+	}
+	_, signed, valid, verr := pcrypto.VerifyDocument(b, kp.Public)
+	switch {
+	case verr != nil:
+		return fmt.Errorf("%w (the signature could not be checked: %v)", ErrPolicyTampered, verr)
+	case signed && !valid:
+		return ErrPolicyTampered
+	case !signed:
+		return fmt.Errorf("%w (it carries no signature, and this machine already has a signing key)", ErrPolicyTampered)
+	}
+	return nil
+}
+
 func LoadLocalPolicyFile(cwd string) (*shared.PolicyCache, error) {
 	if cwd == "" {
 		if wd, err := os.Getwd(); err == nil {
@@ -51,11 +96,8 @@ func LoadLocalPolicyFile(cwd string) (*shared.PolicyCache, error) {
 			continue
 		}
 		if p == own {
-			kp, kerr := pcrypto.LoadOrCreateKeyPair(filepath.Join(shared.SGDir(), "crypto"))
-			if kerr == nil {
-				if _, signed, valid, verr := pcrypto.VerifyDocument(b, kp.Public); verr == nil && signed && !valid {
-					return nil, ErrPolicyTampered
-				}
+			if err := verifyHomePolicy(b); err != nil {
+				return nil, err
 			}
 		}
 		var env *shared.PolicyCache

packages/guard/guardcore/config_test.go

@@ -247,21 +247,25 @@ func TestLoadLocalPolicyFileOwnTakesPrecedenceOverProjectLocal(t *testing.T) {
 	}
 }
 
-func TestLoadLocalPolicyFileSkipsACorruptOwnFileAndFallsBackToProjectLocal(t *testing.T) {
+func TestLoadLocalPolicyFileCorruptOwnFileIsRejectedNotSkipped(t *testing.T) {
 	sgHome(t)
 	cwd := t.TempDir()
 
+	// A home policy that cannot be parsed is a broken trust anchor, not an
+	// absent one. Skipping it used to fall through to the project-local policy,
+	// which carries no Security fields at all - so corrupting the file quietly
+	// downgraded the guard instead of stopping it.
 	writeOwnPolicy(t, []byte(`{not valid json at all`))
 	writeProjectPolicy(t, cwd, marshal(t, map[string]interface{}{
 		"id": "proj-pol", "name": "Project", "mode": "advisory",
 	}))
 
 	env, err := LoadLocalPolicyFile(cwd)
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
+	if !errors.Is(err, ErrPolicyTampered) {
+		t.Fatalf("got err=%v, want ErrPolicyTampered for a corrupt own policy", err)
 	}
-	if env == nil || env.Policy == nil || env.Policy.ID != "proj-pol" {
-		t.Fatalf("expected to fall back to the project-local policy, got %+v", env)
+	if env != nil {
+		t.Errorf("expected no cache for a corrupt own policy, got %+v", env)
 	}
 }
 
@@ -279,4 +283,74 @@ func TestLoadLocalPolicyFileResolvesCwdFromGetwdWhenEmpty(t *testing.T) {
 	if env == nil || env.Policy == nil || env.Policy.ID != "home-pol" {
 		t.Fatalf("expected the own policy to load with an empty cwd, got %+v", env)
 	}
-}
+}
+func TestLoadLocalPolicyFileOwnMalformedSignatureIsRejected(t *testing.T) {
+	sgHome(t)
+	cwd := t.TempDir()
+
+	// A signature field that is present but cannot even be decoded: the loader
+	// used to skip the check on any verification error and trust the file.
+	doc := map[string]interface{}{
+		"policy": map[string]interface{}{"id": "home-pol", "name": "Home", "mode": "advisory"},
+		"_sig":   "%not-base64%",
+	}
+	writeOwnPolicy(t, marshal(t, doc))
+
+	env, err := LoadLocalPolicyFile(cwd)
+	if !errors.Is(err, ErrPolicyTampered) {
+		t.Fatalf("got err=%v, want ErrPolicyTampered for an undecodable signature", err)
+	}
+	if env != nil {
+		t.Errorf("expected no cache for an undecodable signature, got %+v", env)
+	}
+}
+
+func TestLoadLocalPolicyFileOwnUnsignedAfterTheMachineHasASigningKeyIsRejected(t *testing.T) {
+	sgHome(t)
+	cwd := t.TempDir()
+
+	// Once this machine has a signing key, its own policy.json is always written
+	// signed, so a missing signature means the signature was stripped.
+	if _, err := pcrypto.LoadOrCreateKeyPair(filepath.Join(shared.SGDir(), "crypto")); err != nil {
+		t.Fatal(err)
+	}
+	doc := map[string]interface{}{
+		"policy": map[string]interface{}{"id": "home-pol", "name": "Home", "mode": "advisory"},
+	}
+	writeOwnPolicy(t, marshal(t, doc))
+
+	env, err := LoadLocalPolicyFile(cwd)
+	if !errors.Is(err, ErrPolicyTampered) {
+		t.Fatalf("got err=%v, want ErrPolicyTampered for a stripped signature", err)
+	}
+	if env != nil {
+		t.Errorf("expected no cache for a stripped signature, got %+v", env)
+	}
+}
+
+func TestLoadLocalPolicyFileDoesNotCreateASigningKey(t *testing.T) {
+	sgHome(t)
+	cwd := t.TempDir()
+
+	// Pre-signature installs and the parity fixtures write an unsigned
+	// policy.json. Loading it must not create a key pair: if it did, the next run
+	// would see a keyed machine and reject the very same file.
+	doc := map[string]interface{}{
+		"policy":   map[string]interface{}{"id": "home-pol", "name": "Home", "mode": "advisory"},
+		"security": map[string]interface{}{"protectedPaths": []string{"/tmp/keep"}},
+	}
+	writeOwnPolicy(t, marshal(t, doc))
+
+	for i := 0; i < 2; i++ {
+		env, err := LoadLocalPolicyFile(cwd)
+		if err != nil {
+			t.Fatalf("load %d: got err=%v, want the unsigned policy to load", i+1, err)
+		}
+		if env == nil || env.Policy == nil || env.Policy.ID != "home-pol" {
+			t.Fatalf("load %d: got %+v, want the home policy", i+1, env)
+		}
+		if pcrypto.HasKeyPair(filepath.Join(shared.SGDir(), "crypto")) {
+			t.Fatalf("load %d: loading a policy created a signing key", i+1)
+		}
+	}
+}