Compare commits

..

5 commits
v0.2.0 ... dev

Author SHA1 Message Date
Virgil
c9a7a6fb4b fix(trust): enforce scoped repository defaults
Some checks failed
Security Scan / security (push) Failing after 9s
Test / test (push) Failing after 33s
2026-03-30 10:42:05 +00:00
86c68ad1c9 Merge pull request '[agent/codex:gpt-5.3-codex-spark] Read .core/reference/RFC-CORE-008-AGENT-EXPERIENCE.md (the A...' (#14) from main into dev
Some checks failed
Security Scan / security (push) Failing after 15s
Test / test (push) Successful in 10m52s
2026-03-29 15:26:33 +00:00
Virgil
e80ef94552 fix(crypt): align AX error handling and cleanup checks
Some checks failed
Security Scan / security (push) Failing after 10s
Test / test (push) Failing after 9m7s
Co-Authored-By: Virgil <virgil@lethean.io>
2026-03-29 15:25:12 +00:00
f37f5b3a14 Merge pull request 'Fix CodeRabbit findings' (#12) from agent/fix-coderabbit-findings--verify-each-aga into dev
Some checks failed
Security Scan / security (push) Failing after 8s
Test / test (push) Failing after 8m44s
Reviewed-on: #12
2026-03-24 11:33:05 +00:00
Snider
36bf16b06e fix(coderabbit): address review findings
Some checks failed
Security Scan / security (pull_request) Failing after 8s
Test / test (pull_request) Failing after 4m46s
- 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>
2026-03-17 13:32:21 +00:00
14 changed files with 130 additions and 66 deletions

View file

@ -323,7 +323,9 @@ func (a *Authenticator) ValidateSession(token string) (*Session, error) {
} }
if time.Now().After(session.ExpiresAt) { if time.Now().After(session.ExpiresAt) {
_ = a.store.Delete(token) if err := a.store.Delete(token); err != nil {
return nil, coreerr.E(op, "session expired", err)
}
return nil, coreerr.E(op, "session expired", nil) return nil, coreerr.E(op, "session expired", nil)
} }
@ -340,7 +342,9 @@ func (a *Authenticator) RefreshSession(token string) (*Session, error) {
} }
if time.Now().After(session.ExpiresAt) { if time.Now().After(session.ExpiresAt) {
_ = a.store.Delete(token) if err := a.store.Delete(token); err != nil {
return nil, coreerr.E(op, "session expired", err)
}
return nil, coreerr.E(op, "session expired", nil) return nil, coreerr.E(op, "session expired", nil)
} }
@ -389,7 +393,9 @@ func (a *Authenticator) DeleteUser(userID string) error {
} }
// Revoke any active sessions for this user // Revoke any active sessions for this user
_ = a.store.DeleteByUser(userID) if err := a.store.DeleteByUser(userID); err != nil {
return coreerr.E(op, "failed to delete user sessions", err)
}
return nil return nil
} }
@ -419,19 +425,21 @@ func (a *Authenticator) Login(userID, password string) (*Session, error) {
return nil, coreerr.E(op, "failed to read password hash", err) return nil, coreerr.E(op, "failed to read password hash", err)
} }
if strings.HasPrefix(storedHash, "$argon2id$") { if !strings.HasPrefix(storedHash, "$argon2id$") {
valid, err := crypt.VerifyPassword(password, storedHash) return nil, coreerr.E(op, "corrupted password hash", nil)
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)
} }
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")) storedHash, err := a.medium.Read(userPath(userID, ".lthn"))
if err != nil { if err != nil {
return nil, coreerr.E(op, "user not found", err) return nil, coreerr.E(op, "user not found", err)
@ -565,7 +573,9 @@ func (a *Authenticator) RevokeKey(userID, password, reason string) error {
} }
// Invalidate all sessions // Invalidate all sessions
_ = a.store.DeleteByUser(userID) if err := a.store.DeleteByUser(userID); err != nil {
return coreerr.E(op, "failed to delete user sessions", err)
}
return nil return nil
} }
@ -645,19 +655,25 @@ func (a *Authenticator) verifyPassword(userID, password string) error {
// Try Argon2id hash first (.hash file) // Try Argon2id hash first (.hash file)
if a.medium.IsFile(userPath(userID, ".hash")) { if a.medium.IsFile(userPath(userID, ".hash")) {
storedHash, err := a.medium.Read(userPath(userID, ".hash")) storedHash, err := a.medium.Read(userPath(userID, ".hash"))
if err == nil && strings.HasPrefix(storedHash, "$argon2id$") { if err != nil {
valid, verr := crypt.VerifyPassword(password, storedHash) return coreerr.E(op, "failed to read password hash", err)
if verr != nil {
return coreerr.E(op, "failed to verify password", nil)
}
if !valid {
return coreerr.E(op, "invalid password", nil)
}
return nil
} }
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")) storedHash, err := a.medium.Read(userPath(userID, ".lthn"))
if err != nil { if err != nil {
return coreerr.E(op, "user not found", nil) return coreerr.E(op, "user not found", nil)

View file

@ -49,7 +49,11 @@ func runTest(verbose, coverage, short bool, pkg, run string, race, jsonOutput bo
// Create command // Create command
cmd := exec.Command("go", args...) cmd := exec.Command("go", args...)
cmd.Dir, _ = os.Getwd() cwd, err := os.Getwd()
if err != nil {
return coreerr.E("cmd.test", "failed to determine working directory", err)
}
cmd.Dir = cwd
// Set environment to suppress macOS linker warnings // Set environment to suppress macOS linker warnings
cmd.Env = append(os.Environ(), getMacOSDeploymentTarget()) cmd.Env = append(os.Environ(), getMacOSDeploymentTarget())
@ -76,7 +80,7 @@ func runTest(verbose, coverage, short bool, pkg, run string, race, jsonOutput bo
cmd.Stderr = &stderr cmd.Stderr = &stderr
} }
err := cmd.Run() err = cmd.Run()
exitCode := 0 exitCode := 0
if err != nil { if err != nil {
if exitErr, ok := err.(*exec.ExitError); ok { if exitErr, ok := err.(*exec.ExitError); ok {

View file

@ -27,21 +27,23 @@ func Encrypt(plaintext []byte, key []byte) ([]byte, error) {
// Decrypt decrypts data using ChaCha20-Poly1305. // Decrypt decrypts data using ChaCha20-Poly1305.
func Decrypt(ciphertext []byte, key []byte) ([]byte, error) { func Decrypt(ciphertext []byte, key []byte) ([]byte, error) {
const op = "chachapoly.Decrypt"
aead, err := chacha20poly1305.NewX(key) aead, err := chacha20poly1305.NewX(key)
if err != nil { if err != nil {
return nil, err return nil, coreerr.E(op, "failed to create cipher", err)
} }
minLen := aead.NonceSize() + aead.Overhead() minLen := aead.NonceSize() + aead.Overhead()
if len(ciphertext) < minLen { 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():] nonce, ciphertext := ciphertext[:aead.NonceSize()], ciphertext[aead.NonceSize():]
decrypted, err := aead.Open(nil, nonce, ciphertext, nil) decrypted, err := aead.Open(nil, nonce, ciphertext, nil)
if err != nil { if err != nil {
return nil, err return nil, coreerr.E(op, "decryption failed", err)
} }
if len(decrypted) == 0 { if len(decrypted) == 0 {

View file

@ -2,9 +2,10 @@ package chachapoly
import ( import (
"crypto/rand" "crypto/rand"
"errors"
"testing" "testing"
coreerr "dappco.re/go/core/log"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
) )
@ -12,7 +13,7 @@ import (
type mockReader struct{} type mockReader struct{}
func (r *mockReader) Read(p []byte) (n int, err error) { func (r *mockReader) Read(p []byte) (n int, err error) {
return 0, errors.New("read error") return 0, coreerr.E("chachapoly.mockReader.Read", "read error", nil)
} }
func TestEncryptDecrypt(t *testing.T) { func TestEncryptDecrypt(t *testing.T) {

View file

@ -149,7 +149,9 @@ func (s *Service) DecryptPGP(privateKey, message, passphrase string, opts ...any
return "", coreerr.E("openpgp.DecryptPGP", "failed to decrypt private key", err) return "", coreerr.E("openpgp.DecryptPGP", "failed to decrypt private key", err)
} }
for _, subkey := range entity.Subkeys { for _, subkey := range entity.Subkeys {
_ = subkey.PrivateKey.Decrypt([]byte(passphrase)) if err := subkey.PrivateKey.Decrypt([]byte(passphrase)); err != nil {
return "", coreerr.E("openpgp.DecryptPGP", "failed to decrypt subkey", err)
}
} }
} }

View file

@ -34,7 +34,9 @@ func CreateKeyPair(name, email, password string) (*KeyPair, error) {
// Sign all the identities // Sign all the identities
for _, id := range entity.Identities { for _, id := range entity.Identities {
_ = id.SelfSignature.SignUserId(id.UserId.Id, entity.PrimaryKey, entity.PrivateKey, nil) if err := id.SelfSignature.SignUserId(id.UserId.Id, entity.PrimaryKey, entity.PrivateKey, nil); err != nil {
return nil, coreerr.E(op, "failed to sign identity", err)
}
} }
// Encrypt private key with password if provided // Encrypt private key with password if provided
@ -166,7 +168,9 @@ func Decrypt(data []byte, privateKeyArmor, password string) ([]byte, error) {
} }
for _, subkey := range entity.Subkeys { for _, subkey := range entity.Subkeys {
if subkey.PrivateKey != nil && subkey.PrivateKey.Encrypted { if subkey.PrivateKey != nil && subkey.PrivateKey.Encrypted {
_ = subkey.PrivateKey.Decrypt([]byte(password)) if err := subkey.PrivateKey.Decrypt([]byte(password)); err != nil {
return nil, coreerr.E(op, "failed to decrypt subkey", err)
}
} }
} }
} }

View file

@ -6,9 +6,10 @@ import (
"crypto/rand" "crypto/rand"
"crypto/x509" "crypto/x509"
"encoding/pem" "encoding/pem"
"errors"
"testing" "testing"
coreerr "dappco.re/go/core/log"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
) )
@ -16,7 +17,7 @@ import (
type mockReader struct{} type mockReader struct{}
func (r *mockReader) Read(p []byte) (n int, err error) { func (r *mockReader) Read(p []byte) (n int, err error) {
return 0, errors.New("read error") return 0, coreerr.E("rsa.mockReader.Read", "read error", nil)
} }
func TestRSA_Good(t *testing.T) { func TestRSA_Good(t *testing.T) {

View file

@ -161,17 +161,17 @@ Severity is low: an attacker with read access to process memory already has full
access to the process. The Go runtime does not guarantee memory zeroing and access to the process. The Go runtime does not guarantee memory zeroing and
GC-managed runtimes inherently have this limitation. GC-managed runtimes inherently have this limitation.
### Finding F3: Empty ScopedRepos Bypasses Scope Check on Tier 2 (Medium) — Open ### Finding F3: Empty ScopedRepos Bypasses Scope Check on Tier 2 (Medium) — RESOLVED
In `policy.go`, the repo scope check is conditioned on `len(agent.ScopedRepos) > 0`. In `policy.go`, repo-scoped capability access previously skipped checks when
A Tier 2 agent with empty `ScopedRepos` (nil or `[]string{}`) is treated as `len(agent.ScopedRepos) == 0`.
unrestricted rather than as having no access. If an admin registers a Tier 2 A Tier 2 agent with empty `ScopedRepos` (nil or `[]string{}`) was previously treated as
agent without explicitly setting `ScopedRepos`, it gets access to all repositories unrestricted rather than as having no access.
for repo-scoped capabilities (`repo.push`, `pr.create`, `pr.merge`, `secrets.read`).
Potential remediation: treat empty `ScopedRepos` as no access for Tier 2 agents, Resolved by requiring an explicit scope for repo-scoped capabilities:
requiring explicit `["*"]` or `["org/**"]` for unrestricted access. This is a - `[]string{}` / `nil` now denies all repo-scoped access by default.
design decision with backward-compatibility implications. - `[]string{"*"}` grants unrestricted repo access.
- Pattern matching with `host-uk/*` and `host-uk/**` still applies as before.
### Finding F4: `go vet` Clean — Passed ### Finding F4: `go vet` Clean — Passed
@ -224,8 +224,6 @@ callers that need structured logs should wrap or replace the cleanup goroutine.
`crypt/chachapoly` into a single implementation. `crypt/chachapoly` into a single implementation.
- **Hardware key backends**: implement `HardwareKey` for PKCS#11 (via - **Hardware key backends**: implement `HardwareKey` for PKCS#11 (via
`miekg/pkcs11` or `ThalesIgnite/crypto11`) and YubiKey (via `go-piv`). `miekg/pkcs11` or `ThalesIgnite/crypto11`) and YubiKey (via `go-piv`).
- **Resolve Finding F3**: require explicit wildcard for unrestricted Tier 2
access; treat empty `ScopedRepos` as no-access.
- **Structured logging**: replace `fmt.Printf` in `StartCleanup` with an - **Structured logging**: replace `fmt.Printf` in `StartCleanup` with an
`slog.Logger` option on `Authenticator`. `slog.Logger` option on `Authenticator`.
- **Rate limiting enforcement**: the `Agent.RateLimit` field is stored in the - **Rate limiting enforcement**: the `Agent.RateLimit` field is stored in the

View file

@ -151,8 +151,8 @@ func (q *ApprovalQueue) Get(id string) *ApprovalRequest {
return nil return nil
} }
// Return a copy to prevent mutation. // Return a copy to prevent mutation.
copy := *req snapshot := *req
return &copy return &snapshot
} }
// Pending returns all requests with ApprovalPending status. // Pending returns all requests with ApprovalPending status.

View file

@ -34,12 +34,21 @@ func LoadPoliciesFromFile(path string) ([]Policy, error) {
// LoadPolicies reads JSON from a reader and returns parsed policies. // LoadPolicies reads JSON from a reader and returns parsed policies.
func LoadPolicies(r io.Reader) ([]Policy, error) { func LoadPolicies(r io.Reader) ([]Policy, error) {
const op = "trust.LoadPolicies"
var cfg PoliciesConfig var cfg PoliciesConfig
dec := json.NewDecoder(r) dec := json.NewDecoder(r)
dec.DisallowUnknownFields() dec.DisallowUnknownFields()
if err := dec.Decode(&cfg); err != nil { 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) return convertPolicies(cfg)
} }

View file

@ -117,9 +117,9 @@ func (pe *PolicyEngine) Evaluate(agentName string, cap Capability, repo string)
// Check if capability is allowed. // Check if capability is allowed.
for _, allowed := range policy.Allowed { for _, allowed := range policy.Allowed {
if allowed == cap { if allowed == cap {
// For repo-scoped capabilities, verify repo access. // For repo-scoped capabilities, verify repo access for restricted tiers.
if isRepoScoped(cap) && len(agent.ScopedRepos) > 0 { if isRepoScoped(cap) && agent.Tier != TierFull {
if !repoAllowed(agent.ScopedRepos, repo) { if len(agent.ScopedRepos) == 0 || !repoAllowed(agent.ScopedRepos, repo) {
return EvalResult{ return EvalResult{
Decision: Deny, Decision: Deny,
Agent: agentName, Agent: agentName,
@ -247,6 +247,11 @@ func matchScope(pattern, repo string) bool {
return true return true
} }
// Star means unrestricted access for all repos.
if pattern == "*" {
return true
}
// Check for wildcard patterns. // Check for wildcard patterns.
if !strings.Contains(pattern, "*") { if !strings.Contains(pattern, "*") {
return false return false

View file

@ -270,34 +270,49 @@ func TestDefaultRateLimit(t *testing.T) {
// --- Phase 0 Additions --- // --- Phase 0 Additions ---
// TestEvaluate_Good_Tier2EmptyScopedReposAllowsAll verifies that a Tier 2 // TestEvaluate_Bad_Tier2EmptyScopedReposDeniesAll verifies that an empty
// agent with empty ScopedRepos is treated as "unrestricted" for repo-scoped // scoped-repo list blocks repo-scoped capabilities by default.
// capabilities. NOTE: This is a potential security concern documented in func TestEvaluate_Bad_Tier2EmptyScopedReposDeniesAll(t *testing.T) {
// FINDINGS.md — empty ScopedRepos bypasses the repo scope check entirely.
func TestEvaluate_Good_Tier2EmptyScopedReposAllowsAll(t *testing.T) {
r := NewRegistry() r := NewRegistry()
require.NoError(t, r.Register(Agent{ require.NoError(t, r.Register(Agent{
Name: "Hypnos", Name: "Hypnos",
Tier: TierVerified, Tier: TierVerified,
ScopedRepos: []string{}, // empty — currently means "unrestricted" ScopedRepos: []string{},
})) }))
pe := NewPolicyEngine(r) pe := NewPolicyEngine(r)
// Current behaviour: empty ScopedRepos skips scope check (len == 0)
result := pe.Evaluate("Hypnos", CapPushRepo, "host-uk/core") result := pe.Evaluate("Hypnos", CapPushRepo, "host-uk/core")
assert.Equal(t, Allow, result.Decision, assert.Equal(t, Deny, result.Decision,
"empty ScopedRepos currently allows all repos (potential security finding)") "empty ScopedRepos should deny repo-scoped operations by default")
result = pe.Evaluate("Hypnos", CapReadSecrets, "host-uk/core") result = pe.Evaluate("Hypnos", CapReadSecrets, "host-uk/core")
assert.Equal(t, Allow, result.Decision) assert.Equal(t, Deny, result.Decision)
result = pe.Evaluate("Hypnos", CapCreatePR, "host-uk/core") result = pe.Evaluate("Hypnos", CapCreatePR, "host-uk/core")
assert.Equal(t, Allow, result.Decision) assert.Equal(t, Allow, result.Decision)
// Non-repo-scoped capabilities should still work
result = pe.Evaluate("Hypnos", CapCreateIssue, "") result = pe.Evaluate("Hypnos", CapCreateIssue, "")
assert.Equal(t, Allow, result.Decision) assert.Equal(t, Allow, result.Decision)
result = pe.Evaluate("Hypnos", CapCommentIssue, "") }
func TestEvaluate_Good_Tier2WildcardAllowsAll(t *testing.T) {
r := NewRegistry()
require.NoError(t, r.Register(Agent{
Name: "Hydrus",
Tier: TierVerified,
ScopedRepos: []string{"*"},
}))
pe := NewPolicyEngine(r)
result := pe.Evaluate("Hydrus", CapPushRepo, "host-uk/core")
assert.Equal(t, Allow, result.Decision)
result = pe.Evaluate("Hydrus", CapReadSecrets, "host-uk/any")
assert.Equal(t, Allow, result.Decision)
result = pe.Evaluate("Hydrus", CapCreateIssue, "")
assert.Equal(t, Allow, result.Decision)
result = pe.Evaluate("Hydrus", CapCommentIssue, "")
assert.Equal(t, Allow, result.Decision) assert.Equal(t, Allow, result.Decision)
} }

View file

@ -13,6 +13,11 @@ func TestMatchScope_Good_ExactMatch(t *testing.T) {
assert.True(t, matchScope("host-uk/core", "host-uk/core")) assert.True(t, matchScope("host-uk/core", "host-uk/core"))
} }
func TestMatchScope_Good_StarWildcard(t *testing.T) {
assert.True(t, matchScope("*", "host-uk/core"))
assert.True(t, matchScope("*", "core/php/sub"))
}
func TestMatchScope_Good_SingleWildcard(t *testing.T) { func TestMatchScope_Good_SingleWildcard(t *testing.T) {
assert.True(t, matchScope("core/*", "core/php")) assert.True(t, matchScope("core/*", "core/php"))
assert.True(t, matchScope("core/*", "core/go-crypt")) assert.True(t, matchScope("core/*", "core/go-crypt"))

View file

@ -71,7 +71,9 @@ type Agent struct {
Name string Name string
// Tier is the agent's trust level. // Tier is the agent's trust level.
Tier Tier Tier Tier
// ScopedRepos limits repo access for Tier 2 agents. Empty means no repo access. // ScopedRepos limits repo access for Tier 2 agents.
// Empty means no repo access.
// Use ["*"] for unrestricted repo scope.
// Tier 3 agents ignore this field (they have access to all repos). // Tier 3 agents ignore this field (they have access to all repos).
ScopedRepos []string ScopedRepos []string
// RateLimit is the maximum requests per minute. 0 means unlimited. // RateLimit is the maximum requests per minute. 0 means unlimited.