From 36bf16b06ebb68a1291b4aee012dc8fd0d0ed501 Mon Sep 17 00:00:00 2001
From: Snider <snider@host.uk.com>
Date: Tue, 17 Mar 2026 13:32:21 +0000
Subject: [PATCH] fix(coderabbit): address review findings

- auth: prevent legacy .lthn fallback when .hash file exists but is
  unreadable or has unexpected format (security fix in verifyPassword
  and Login)
- chachapoly: wrap raw error returns in Decrypt with coreerr.E()
- trust: reject trailing data in LoadPolicies JSON decoder

Co-Authored-By: Virgil <virgil@lethean.io>
---
 auth/auth.go                   | 48 ++++++++++++++++++++--------------
 crypt/chachapoly/chachapoly.go |  8 +++---
 trust/config.go                | 11 +++++++-
 3 files changed, 43 insertions(+), 24 deletions(-)

diff --git a/auth/auth.go b/auth/auth.go
index 6a6faa8..93a551b 100644
--- a/auth/auth.go
+++ b/auth/auth.go
@@ -420,19 +420,21 @@ func (a *Authenticator) Login(userID, password string) (*Session, error) {
 			return nil, coreerr.E(op, "failed to read password hash", err)
 		}
 
-		if strings.HasPrefix(storedHash, "$argon2id$") {
-			valid, err := crypt.VerifyPassword(password, storedHash)
-			if err != nil {
-				return nil, coreerr.E(op, "failed to verify password", err)
-			}
-			if !valid {
-				return nil, coreerr.E(op, "invalid password", nil)
-			}
-			return a.createSession(userID)
+		if !strings.HasPrefix(storedHash, "$argon2id$") {
+			return nil, coreerr.E(op, "corrupted password hash", nil)
 		}
+
+		valid, err := crypt.VerifyPassword(password, storedHash)
+		if err != nil {
+			return nil, coreerr.E(op, "failed to verify password", err)
+		}
+		if !valid {
+			return nil, coreerr.E(op, "invalid password", nil)
+		}
+		return a.createSession(userID)
 	}
 
-	// Fall back to legacy LTHN hash (.lthn file)
+	// Fall back to legacy LTHN hash (.lthn file) — only when no .hash file exists
 	storedHash, err := a.medium.Read(userPath(userID, ".lthn"))
 	if err != nil {
 		return nil, coreerr.E(op, "user not found", err)
@@ -646,19 +648,25 @@ func (a *Authenticator) verifyPassword(userID, password string) error {
 	// Try Argon2id hash first (.hash file)
 	if a.medium.IsFile(userPath(userID, ".hash")) {
 		storedHash, err := a.medium.Read(userPath(userID, ".hash"))
-		if err == nil && strings.HasPrefix(storedHash, "$argon2id$") {
-			valid, verr := crypt.VerifyPassword(password, storedHash)
-			if verr != nil {
-				return coreerr.E(op, "failed to verify password", nil)
-			}
-			if !valid {
-				return coreerr.E(op, "invalid password", nil)
-			}
-			return nil
+		if err != nil {
+			return coreerr.E(op, "failed to read password hash", err)
 		}
+
+		if !strings.HasPrefix(storedHash, "$argon2id$") {
+			return coreerr.E(op, "corrupted password hash", nil)
+		}
+
+		valid, verr := crypt.VerifyPassword(password, storedHash)
+		if verr != nil {
+			return coreerr.E(op, "failed to verify password", verr)
+		}
+		if !valid {
+			return coreerr.E(op, "invalid password", nil)
+		}
+		return nil
 	}
 
-	// Fall back to legacy LTHN hash (.lthn file)
+	// Fall back to legacy LTHN hash (.lthn file) — only when no .hash file exists
 	storedHash, err := a.medium.Read(userPath(userID, ".lthn"))
 	if err != nil {
 		return coreerr.E(op, "user not found", nil)
diff --git a/crypt/chachapoly/chachapoly.go b/crypt/chachapoly/chachapoly.go
index 7a1f326..227327c 100644
--- a/crypt/chachapoly/chachapoly.go
+++ b/crypt/chachapoly/chachapoly.go
@@ -27,21 +27,23 @@ func Encrypt(plaintext []byte, key []byte) ([]byte, error) {
 
 // Decrypt decrypts data using ChaCha20-Poly1305.
 func Decrypt(ciphertext []byte, key []byte) ([]byte, error) {
+	const op = "chachapoly.Decrypt"
+
 	aead, err := chacha20poly1305.NewX(key)
 	if err != nil {
-		return nil, err
+		return nil, coreerr.E(op, "failed to create cipher", err)
 	}
 
 	minLen := aead.NonceSize() + aead.Overhead()
 	if len(ciphertext) < minLen {
-		return nil, coreerr.E("chachapoly.Decrypt", fmt.Sprintf("ciphertext too short: got %d bytes, need at least %d bytes", len(ciphertext), minLen), nil)
+		return nil, coreerr.E(op, fmt.Sprintf("ciphertext too short: got %d bytes, need at least %d bytes", len(ciphertext), minLen), nil)
 	}
 
 	nonce, ciphertext := ciphertext[:aead.NonceSize()], ciphertext[aead.NonceSize():]
 
 	decrypted, err := aead.Open(nil, nonce, ciphertext, nil)
 	if err != nil {
-		return nil, err
+		return nil, coreerr.E(op, "decryption failed", err)
 	}
 
 	if len(decrypted) == 0 {
diff --git a/trust/config.go b/trust/config.go
index fd198dd..1fcdc10 100644
--- a/trust/config.go
+++ b/trust/config.go
@@ -34,12 +34,21 @@ func LoadPoliciesFromFile(path string) ([]Policy, error) {
 
 // LoadPolicies reads JSON from a reader and returns parsed policies.
 func LoadPolicies(r io.Reader) ([]Policy, error) {
+	const op = "trust.LoadPolicies"
+
 	var cfg PoliciesConfig
 	dec := json.NewDecoder(r)
 	dec.DisallowUnknownFields()
 	if err := dec.Decode(&cfg); err != nil {
-		return nil, coreerr.E("trust.LoadPolicies", "failed to decode JSON", err)
+		return nil, coreerr.E(op, "failed to decode JSON", err)
 	}
+
+	// Reject trailing data after the decoded value
+	var extra json.RawMessage
+	if err := dec.Decode(&extra); err != io.EOF {
+		return nil, coreerr.E(op, "unexpected trailing data in JSON", nil)
+	}
+
 	return convertPolicies(cfg)
 }
 
-- 
2.45.3