mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-09 18:08:31 -10:00
* fix(sandbox): deny reads of zero credential stores * fix(sandbox): address PR review comments - Deny reads of mcp-oauth-tokens.json.secret in the default Zero config candidates: file-backed oauth stores keep their encryption secret in a sibling .secret file, so the default MCP token store needs the same protection as the override paths (CodeRabbit). - Clarify that explicit credential-file overrides are still filtered by on-disk existence like every other candidate (Copilot). - Align the zeroUserConfigDir doc comment with config.UserConfigDir: macOS honors XDG_CONFIG_HOME before falling back to ~/.config (Copilot). Co-Authored-By: Claude Fable 5 <[email protected]> * fix(sandbox): deny the whole Zero config dir, not itemized filenames Address code review on PR #681: - Deny credentialDenyReadPaths' Zero-config candidate as the containing directory instead of an itemized filename list. The token/credential/ config stores each publish through a randomly-named .tmp-<pid>-<nanos> sibling before their atomic rename, and the legacy MCP token store leaves a mcp-oauth-tokens.json.migrated backup after importing it — none of those names were covered by the old itemized list, so a sandboxed command could read them under the read-all posture. - Stop dropping default candidates that don't exist yet at profile-build time. A store created later in a long-lived sandboxed session (e.g. a concurrent ) previously got no deny rule at all; every backend already treats a deny rule over a not-yet-existing path as a harmless no-op that still takes effect once the path appears. - Resolve ZERO_OAUTH_TOKENS_PATH / ZERO_MCP_OAUTH_TOKENS_PATH overrides the same way the token stores resolve them (relative-to-cwd, no ~ expansion) instead of through normalizeProfilePath, which tilde-expands and so could derive a deny path different from where the store actually writes. Adds regression coverage for the directory-wide deny (including the migrated backup and synthetic temp-file siblings), for building the profile before the store directory exists, and for the tilde-override resolution mismatch. Co-Authored-By: Claude Sonnet 5 <[email protected]> * fix(sandbox): address macOS deny-path test and config parity Recompute the expected Zero config deny path after MkdirAll and resolve the temp base with EvalSymlinks so macOS /var -> /private/var does not flake TestPermissionProfileDeniesZeroCredentialFiles. Add a parity test that sandbox.zeroUserConfigDir stays aligned with config.UserConfigDir. Co-authored-by: PierrunoYT <[email protected]> * fix(sandbox): harden credential deny paths * test(cli): normalize credential deny baseline * fix(sandbox): close credential path gaps * test(sandbox): normalize command credential path * fix(sandbox): handle absent credential paths safely * fix(sandbox): protect MCP OAuth siblings * fix(sandbox): protect OAuth lock siblings * docs(sandbox): clarify Linux future-path limits * test(sandbox): probe credential store reads in the Linux smoke test The smoke test wired HOME/XDG_CONFIG_HOME at credential directories but never attempted a read through the sandbox, so it could not catch a real-bwrap regression for the paths this change protects. Write a secret into ~/.aws and $XDG_CONFIG_HOME/zero and add both to the deny-read table. * fix(sandbox): keep the credential deny usable, current, and unguessable Four problems with the credential deny baseline, all in the same mechanism. The whole-directory deny over $XDG_CONFIG_HOME/zero also hid the supported user plugin, specialist, and command roots, whose commands and scripts are executed through this sandbox, so an installed user plugin could no longer start. Narrowing the deny to itemized files was not an option: the stores publish through temporary names and a concurrent login can add a store that did not exist when the profile was built, and only a directory rule covers both. The directory deny therefore stays, and the non-secret subtrees are carved back out through a new DenyReadCarveouts field: seatbelt re-allows them after the deny (SBPL is last-match-wins) plus ancestor metadata so path resolution can reach them, and bubblewrap masks the directory with --perms 111 instead of 000 -- unlistable but traversable -- and re-binds the carveout read-only before the tmpfs is remounted read-only. A relative ZERO_OAUTH_TOKENS_PATH, ZERO_MCP_OAUTH_TOKENS_PATH, or XDG_CONFIG_HOME was resolved against the command directory only, while oauth.ResolveStorePath and mcp.ResolveTokenStorePath call filepath.Abs, i.e. resolve against the Zero process working directory. A command run from a subdirectory therefore had the wrong path denied and the real store stayed readable under the read-all posture. Relative candidates now resolve against both directories, since the process that resolves the value is not necessarily the one that writes the store. The regression test runs the command from a different directory than the process, which the previous tests did not. bubblewrap cannot mount over a path that is absent when the namespace is assembled, so a store created mid-session stayed readable through the live read-only host-root bind for the rest of a long-lived sandbox session. The directories Zero owns -- its config directory and the per-store publication directories -- are now reported as EnsureDenyReadDirs and created (0700) before the namespace is assembled, so their mask always exists. Third-party stores such as ~/.aws are still skipped when absent: Zero must not create them, and that limitation stays documented on the field. The fixed <store>.tmp publication sibling made the plaintext token, and via the same path its encryption key, observable to a same-user sandboxed process with write access to an override parent: it could wait for the known name and rename the opened inode into an allowed path. Both stores now publish through a per-store <store>.publish directory with a randomly-named file inside. The directory name is derived from the store path, so the profile can deny it up front, while the random name inside is what cannot be waited for or renamed away. A test keeps the duplicated suffix in step with oauth.PublicationDirSuffix. Also adds the seatbelt render coverage that was missing: dropping DenyReadIfExists from denyReadRules made every credential store readable again on macOS without failing a single test. * fix(sandbox): close credential deny review gaps Preserve process credential roots alongside command overrides while limiting host precreation and read carveouts to trusted process-derived paths. Canonicalize future credential roots through existing path aliases, reject symlink carveouts, and collapse automatic masks already covered by user or automatic directory denies. Normalize the policy golden and add regression coverage across profile, Seatbelt, and bubblewrap rendering. Amp-Thread-ID: https://ampcode.com/threads/T-019fa019-2f41-7398-8692-75327064c6f0 Co-authored-by: Pierre Bruno <[email protected]> * fix(sandbox): normalize credential carveout paths Amp-Thread-ID: https://ampcode.com/threads/T-019fa019-2f41-7398-8692-75327064c6f0 Co-authored-by: Pierre Bruno <[email protected]> * fix(sandbox): canonicalize backend credential masks Amp-Thread-ID: https://ampcode.com/threads/T-019fa019-2f41-7398-8692-75327064c6f0 Co-authored-by: Pierre Bruno <[email protected]> * test(sandbox): normalize backend mask expectations Amp-Thread-ID: https://ampcode.com/threads/T-019fa019-2f41-7398-8692-75327064c6f0 Co-authored-by: Pierre Bruno <[email protected]> * fix(sandbox): fail closed for replaceable token stores Amp-Thread-ID: https://ampcode.com/threads/T-019fa7a0-1223-701d-9529-48ba5d7cf8c8 Co-authored-by: Pierre Bruno <[email protected]> * fix(sandbox): allow keyring token path overrides Amp-Thread-ID: https://ampcode.com/threads/T-019fbdfa-8469-731e-87c0-b26b7641cf59 Co-authored-by: Pierre Bruno <[email protected]> * fix(sandbox): harden command credential roots Amp-Thread-ID: https://ampcode.com/threads/T-019fbec7-c723-74d9-a323-026040b94436 Co-authored-by: Pierre Bruno <[email protected]> * fix(sandbox): close credential deny review gaps Reapply credential denies after Seatbelt extension carveouts and model the legacy MCP token path as a migration input rather than an atomic writer. Retain Bubblewrap fail-closed markers for OAuth outputs exposed through readable carveouts, and add regressions for both platform policies. Amp-Thread-ID: https://ampcode.com/threads/T-019fbf55-f6b0-71fc-b384-652ba32b9645 Co-authored-by: Pierre Bruno <[email protected]> --------- Co-authored-by: Claude Fable 5 <[email protected]> Co-authored-by: Cursor Agent <[email protected]> Co-authored-by: PierrunoYT <[email protected]> Co-authored-by: Amp <[email protected]>
192 lines
6.4 KiB
Go
192 lines
6.4 KiB
Go
package oauth
|
|
|
|
import (
|
|
"crypto/aes"
|
|
"crypto/cipher"
|
|
"crypto/rand"
|
|
"errors"
|
|
"fmt"
|
|
"io"
|
|
"os"
|
|
"path/filepath"
|
|
"time"
|
|
)
|
|
|
|
const (
|
|
// secretBytes is the AES-256 key length kept in the per-user secret file.
|
|
secretBytes = 32
|
|
|
|
secretRetryAttempts = 500
|
|
secretRetryDelay = 2 * time.Millisecond
|
|
)
|
|
|
|
// aesGCMCrypter encrypts the token file at rest with AES-256-GCM under a
|
|
// per-user random secret persisted (0600) beside the token file. The on-disk
|
|
// blob is nonce || ciphertext; GCM provides confidentiality AND tamper
|
|
// detection, so a corrupted/forged file fails closed on open. This is the
|
|
// opt-in "encrypted-file" storage backend; the default backend writes the
|
|
// 0600 plaintext JSON unchanged.
|
|
type aesGCMCrypter struct {
|
|
secretPath string
|
|
}
|
|
|
|
func newAESGCMCrypter(secretPath string) *aesGCMCrypter {
|
|
return &aesGCMCrypter{secretPath: secretPath}
|
|
}
|
|
|
|
// aead loads (or, when create is set, generates) the secret and returns the GCM
|
|
// AEAD. open passes create=false so a missing secret is a hard error rather than
|
|
// silently minting a new key that could never decrypt the existing file.
|
|
func (c *aesGCMCrypter) aead(create bool) (cipher.AEAD, error) {
|
|
secret, err := loadOrCreateSecret(c.secretPath, create)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
block, err := aes.NewCipher(secret)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("oauth: build cipher: %w", err)
|
|
}
|
|
return cipher.NewGCM(block)
|
|
}
|
|
|
|
// seal encrypts plaintext, prefixing a fresh random nonce. It creates the secret
|
|
// on first use.
|
|
func (c *aesGCMCrypter) seal(plaintext []byte) ([]byte, error) {
|
|
gcm, err := c.aead(true)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
nonce := make([]byte, gcm.NonceSize())
|
|
if _, err := io.ReadFull(rand.Reader, nonce); err != nil {
|
|
return nil, fmt.Errorf("oauth: generate nonce: %w", err)
|
|
}
|
|
return gcm.Seal(nonce, nonce, plaintext, nil), nil
|
|
}
|
|
|
|
// open decrypts a nonce||ciphertext blob, failing closed on a missing secret,
|
|
// a short blob, or a failed authentication tag (tampering / wrong key).
|
|
func (c *aesGCMCrypter) open(blob []byte) ([]byte, error) {
|
|
gcm, err := c.aead(false)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
if len(blob) < gcm.NonceSize() {
|
|
return nil, errors.New("oauth: encrypted token file is too short")
|
|
}
|
|
nonce, ciphertext := blob[:gcm.NonceSize()], blob[gcm.NonceSize():]
|
|
plaintext, err := gcm.Open(nil, nonce, ciphertext, nil)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("oauth: decrypt token file (wrong secret or tampered): %w", err)
|
|
}
|
|
return plaintext, nil
|
|
}
|
|
|
|
// loadOrCreateSecret reads the 32-byte secret at path. When create is set and
|
|
// the file is absent, it generates a random secret and creates the file
|
|
// atomically (0600). A wrong-sized existing secret fails closed (corruption).
|
|
func loadOrCreateSecret(path string, create bool) ([]byte, error) {
|
|
if data, err := readSecretFileRetry(path); err == nil {
|
|
return data, nil
|
|
} else if !errors.Is(err, os.ErrNotExist) {
|
|
return nil, err
|
|
}
|
|
if !create {
|
|
return nil, fmt.Errorf("oauth: token secret %s is missing; cannot decrypt the token file", path)
|
|
}
|
|
if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
|
|
return nil, err
|
|
}
|
|
return createSecretFile(path)
|
|
}
|
|
|
|
func createSecretFile(path string) ([]byte, error) {
|
|
lockPath := path + ".lock"
|
|
var lastErr error
|
|
for attempt := 0; attempt < secretRetryAttempts; attempt++ {
|
|
lock, err := os.OpenFile(lockPath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o600)
|
|
if err == nil {
|
|
_ = lock.Close()
|
|
defer os.Remove(lockPath)
|
|
if data, rerr := readSecretFileRetry(path); rerr == nil {
|
|
return data, nil
|
|
} else if !errors.Is(rerr, os.ErrNotExist) {
|
|
return nil, rerr
|
|
}
|
|
return writeNewSecretFile(path)
|
|
}
|
|
// On Windows a concurrent holder's os.Remove leaves the lock file in a
|
|
// "delete pending" state, so an O_EXCL create races it with
|
|
// ERROR_ACCESS_DENIED (os.ErrPermission) rather than ErrExist. Treat that
|
|
// as contention and retry too — mirroring acquireFileLock in lock.go —
|
|
// otherwise concurrent secret creation spuriously fails on Windows.
|
|
if !errors.Is(err, os.ErrExist) && !errors.Is(err, os.ErrPermission) {
|
|
return nil, fmt.Errorf("oauth: create token secret lock: %w", err)
|
|
}
|
|
// Remember the lock-creation error itself rather than a subsequent
|
|
// "secret file doesn't exist yet" read error -- otherwise a real
|
|
// contention/ACL problem is masked by the expected-while-waiting
|
|
// ErrNotExist once the retries are exhausted.
|
|
lastErr = err
|
|
if data, rerr := readSecretFileRetry(path); rerr == nil {
|
|
return data, nil
|
|
}
|
|
time.Sleep(secretRetryDelay)
|
|
}
|
|
return nil, fmt.Errorf("oauth: timed out waiting for token secret %s: %w", path, lastErr)
|
|
}
|
|
|
|
func writeNewSecretFile(path string) ([]byte, error) {
|
|
secret := make([]byte, secretBytes)
|
|
if _, err := io.ReadFull(rand.Reader, secret); err != nil {
|
|
return nil, fmt.Errorf("oauth: generate token secret: %w", err)
|
|
}
|
|
// The encryption key is published exactly like the token blob: through the
|
|
// protected per-store directory, under a random name a sandboxed process
|
|
// cannot guess, wait for, or rename away.
|
|
tmp, tmpPath, err := createPublicationFile(path)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("oauth: create token secret temp file: %w", err)
|
|
}
|
|
defer os.Remove(tmpPath)
|
|
if _, werr := tmp.Write(secret); werr != nil {
|
|
_ = tmp.Close()
|
|
return nil, fmt.Errorf("oauth: write token secret: %w", werr)
|
|
}
|
|
if cerr := tmp.Close(); cerr != nil {
|
|
return nil, fmt.Errorf("oauth: write token secret: %w", cerr)
|
|
}
|
|
if err := os.Rename(tmpPath, path); err != nil {
|
|
return nil, fmt.Errorf("oauth: publish token secret: %w", err)
|
|
}
|
|
return secret, nil
|
|
}
|
|
|
|
func readSecretFileRetry(path string) ([]byte, error) {
|
|
var lastErr error
|
|
for attempt := 0; attempt < secretRetryAttempts; attempt++ {
|
|
data, err := readSecretFile(path)
|
|
if !isTransientSecretAccessError(err) {
|
|
return data, err
|
|
}
|
|
lastErr = err
|
|
time.Sleep(secretRetryDelay)
|
|
}
|
|
return nil, fmt.Errorf("oauth: timed out waiting for token secret %s: %w", path, lastErr)
|
|
}
|
|
|
|
// readSecretFile reads and validates the secret at path, returning a wrapped
|
|
// os.ErrNotExist when it is absent so callers can branch on creation.
|
|
func readSecretFile(path string) ([]byte, error) {
|
|
data, err := os.ReadFile(path)
|
|
if err != nil {
|
|
if errors.Is(err, os.ErrNotExist) {
|
|
return nil, err
|
|
}
|
|
return nil, fmt.Errorf("oauth: read token secret: %w", err)
|
|
}
|
|
if len(data) != secretBytes {
|
|
return nil, fmt.Errorf("oauth: token secret at %s has unexpected size %d", path, len(data))
|
|
}
|
|
return data, nil
|
|
}
|