Review fixes

README.md

@@ -12,8 +12,9 @@ builds, structurally verifies, serves, and client-tests deterministic static
 PyPI, Debian, and Helm repositories. Git-backed workspaces also support local
 `init`, `setup`, `add`, `plan`, and `apply` workflows with immutable blobs,
 reviewable plans, publication ledgers, and per-repository managed release
-switching. Replacement of an existing managed release is currently implemented
-only on Linux. PyPI repositories can additionally stage and publish to
+switching. Replacement of an existing managed release needs an atomic
+directory-entry exchange, so it is implemented on Linux and macOS; creating a
+first release is portable. PyPI repositories can additionally stage and publish to
 S3-compatible object storage with immutable releases, conditional root-index
 switching, checksum observation, conditional restore, and optional scoped
 private reads. Shared S3 blob storage, public GitHub Pages with a companion
@@ -285,6 +286,22 @@ locks, ledgers, deployment receipts, and an optional current plan:
 go run ./cmd/snailmail render --output site
 ```
 
+`plan` and `apply` build and stage under `.snailmail/stage` in the workspace
+rather than in `TMPDIR`, so artifacts hard-link from the local CAS instead of
+being copied and large trees are never held in a `tmpfs`. Set an absolute
+`SNAILMAIL_STAGE_DIR` to stage elsewhere.
+
+The AWS SDK is the largest single component of the binary. Builds that only
+target local directories or GitHub Pages can exclude it:
+
+```sh
+go build -trimpath -ldflags="-s -w" -tags nos3 ./cmd/snailmail
+```
+
+That drops the stripped binary from about 20.6 MB to 12.8 MB. S3 hosts and S3
+blob stores then report that the build has no S3 support; every other format,
+host, and command is unaffected. The default build keeps S3.
+
 `Dockerfile` builds the runtime image; `.github/workflows/snailmail.yml` is a
 pinned template with read-only PR testing and a protected default-branch apply
 job. Configure `AWS_ROLE_ARN`/`AWS_REGION` repository variables for OIDC-backed

adapters/blob/s3/aws.go

@@ -1,3 +1,5 @@
+//go:build !nos3
+
 package s3blob
 
 import (

adapters/blob/s3/aws_disabled.go

@@ -0,0 +1,16 @@
+//go:build nos3
+
+package s3blob
+
+import (
+	"context"
+	"errors"
+
+	"github.com/shellcell/snailmail/blob"
+)
+
+// NewAWS reports that this build excludes S3 support. The protocol logic in
+// this package is SDK-free and still compiled; only the AWS client is absent.
+func NewAWS(_ context.Context, _ blob.Configuration) (*Store, error) {
+	return nil, errors.New("this snailmail build was compiled without S3 support (nos3)")
+}

adapters/blob/s3/aws_test.go

@@ -1,3 +1,5 @@
+//go:build !nos3
+
 package s3blob
 
 import (

adapters/credential/command/command.go

@@ -24,9 +24,11 @@ const HelperEnvironment = "SNAILMAIL_CREDENTIAL_BROKER"
 const brokerCommandTimeout = 30 * time.Second
 
 type Broker struct {
-	mutex    sync.Mutex
-	helper   *os.File
-	identity string
+	mutex     sync.Mutex
+	helper    *os.File
+	directory string
+	execPath  string
+	identity  string
 }
 
 func NewFromEnvironment() (*Broker, error) {
@@ -51,15 +53,28 @@ func NewFromEnvironment() (*Broker, error) {
 		_ = source.Close()
 		return nil, errors.New("private read credential broker must be a native executable")
 	}
-	snapshot, err := os.CreateTemp("", ".snailmail-credential-broker-*")
+	// The snapshot lives in a private directory so that retaining it by path,
+	// which darwin requires, is still unreachable by other users.
+	directory, err := os.MkdirTemp("", ".snailmail-credential-broker-*")
 	if err != nil {
 		_ = source.Close()
 		return nil, errors.New("private read credential broker executable could not be snapshotted")
 	}
+	if err := os.Chmod(directory, 0o700); err != nil {
+		_ = source.Close()
+		_ = os.RemoveAll(directory)
+		return nil, errors.New("private read credential broker directory could not be protected")
+	}
+	snapshot, err := os.CreateTemp(directory, "helper-*")
+	if err != nil {
+		_ = source.Close()
+		_ = os.RemoveAll(directory)
+		return nil, errors.New("private read credential broker executable could not be snapshotted")
+	}
 	snapshotName := snapshot.Name()
 	cleanup := func() {
 		_ = snapshot.Close()
-		_ = os.Remove(snapshotName)
+		_ = os.RemoveAll(directory)
 	}
 	hash := sha256.New()
 	_, copyErr := io.Copy(io.MultiWriter(snapshot, hash), source)
@@ -77,20 +92,21 @@ func NewFromEnvironment() (*Broker, error) {
 		return nil, errors.New("private read credential broker executable could not be prepared")
 	}
 	if err := snapshot.Close(); err != nil {
-		_ = os.Remove(snapshotName)
+		_ = os.RemoveAll(directory)
 		return nil, errors.New("private read credential broker executable could not be prepared")
 	}
 	executable, err := os.Open(snapshotName)
 	if err != nil {
-		_ = os.Remove(snapshotName)
+		_ = os.RemoveAll(directory)
 		return nil, errors.New("private read credential broker executable could not be prepared")
 	}
-	if err := os.Remove(snapshotName); err != nil {
+	execPath, err := retainSnapshot(snapshotName)
+	if err != nil {
 		_ = executable.Close()
-		_ = os.Remove(snapshotName)
+		_ = os.RemoveAll(directory)
 		return nil, errors.New("private read credential broker executable could not be protected")
 	}
-	broker := &Broker{helper: executable, identity: hex.EncodeToString(hash.Sum(nil))}
+	broker := &Broker{helper: executable, directory: directory, execPath: execPath, identity: hex.EncodeToString(hash.Sum(nil))}
 	runtime.SetFinalizer(broker, func(value *Broker) { _ = value.Close() })
 	return broker, nil
 }
@@ -108,6 +124,12 @@ func (broker *Broker) Close() error {
 	}
 	err := broker.helper.Close()
 	broker.helper = nil
+	if broker.directory != "" {
+		if removeErr := os.RemoveAll(broker.directory); err == nil {
+			err = removeErr
+		}
+		broker.directory = ""
+	}
 	return err
 }
 
@@ -130,13 +152,9 @@ func (broker *Broker) Issue(ctx context.Context, scope host.ReadScope) (host.Bas
 	if err != nil {
 		return nil, err
 	}
-	helperPath := "/proc/self/fd/3"
-	if runtime.GOOS == "darwin" {
-		helperPath = "/dev/fd/3"
-	}
 	commandCtx, cancel := context.WithTimeout(ctx, brokerCommandTimeout)
 	defer cancel()
-	command := exec.CommandContext(commandCtx, helperPath)
+	command := exec.CommandContext(commandCtx, broker.execPath)
 	command.ExtraFiles = []*os.File{broker.helper}
 	command.Stdin = bytes.NewReader(request)
 	command.Env = brokerEnvironment()
@@ -165,7 +183,7 @@ func (broker *Broker) Issue(ctx context.Context, scope host.ReadScope) (host.Bas
 	if err != nil || !expiresAt.After(time.Now().UTC()) || expiresAt.After(time.Now().UTC().Add(15*time.Minute)) || response.Username == "" || response.Password == "" {
 		return nil, errors.New("credential broker returned invalid or excessive credentials")
 	}
-	return &credential{username: response.Username, password: response.Password, expiresAt: expiresAt}, nil
+	return &credential{username: []byte(response.Username), password: []byte(response.Password), expiresAt: expiresAt}, nil
 }
 
 func brokerEnvironment() []string {
@@ -212,27 +230,32 @@ func (buffer *limitedBuffer) Write(content []byte) (int, error) {
 	return len(content), nil
 }
 
+// credential holds its secret as bytes so Destroy can actually overwrite it.
+// Go strings are immutable, so assigning "" only drops a reference and leaves
+// the secret readable in the heap until the collector happens to reuse it.
 type credential struct {
 	mutex     sync.Mutex
-	username  string
-	password  string
+	username  []byte
+	password  []byte
 	expiresAt time.Time
 }
 
 func (credential *credential) Basic(_ context.Context) (string, string, error) {
 	credential.mutex.Lock()
 	defer credential.mutex.Unlock()
-	if credential.username == "" || credential.password == "" || !time.Now().UTC().Before(credential.expiresAt) {
+	if len(credential.username) == 0 || len(credential.password) == 0 || !time.Now().UTC().Before(credential.expiresAt) {
 		return "", "", errors.New("private read credential is unavailable or expired")
 	}
-	return credential.username, credential.password, nil
+	return string(credential.username), string(credential.password), nil
 }
 
 func (credential *credential) Destroy() {
 	credential.mutex.Lock()
 	defer credential.mutex.Unlock()
-	credential.username = ""
-	credential.password = ""
+	clear(credential.username)
+	clear(credential.password)
+	credential.username = nil
+	credential.password = nil
 	credential.expiresAt = time.Time{}
 }
 

adapters/credential/command/retain_linux.go

@@ -0,0 +1,16 @@
+//go:build linux
+
+package commandcredential
+
+import "os"
+
+// retainSnapshot unlinks the hashed snapshot and returns the path that resolves
+// to the retained inode. /proc/self/fd resolves to the open file itself, so the
+// executed bytes remain exactly the hashed bytes even though no name refers to
+// them any more.
+func retainSnapshot(name string) (string, error) {
+	if err := os.Remove(name); err != nil {
+		return "", err
+	}
+	return "/proc/self/fd/3", nil
+}

adapters/credential/command/retain_other.go

@@ -0,0 +1,13 @@
+//go:build !linux
+
+package commandcredential
+
+// retainSnapshot keeps the hashed snapshot linked and executes it by path.
+// Outside Linux, /dev/fd reopens by path rather than resolving to the open
+// file, so an unlinked snapshot cannot be executed at all. The snapshot already
+// lives in a private 0700 directory with mode 0500, so no other user can reach
+// or rewrite it, and the configured helper path is never consulted again —
+// which is what keeps the executed bytes equal to the hashed bytes.
+func retainSnapshot(name string) (string, error) {
+	return name, nil
+}

adapters/host/s3/aws.go

@@ -1,3 +1,5 @@
+//go:build !nos3
+
 package s3host
 
 import (

adapters/host/s3/aws_disabled.go

@@ -0,0 +1,16 @@
+//go:build nos3
+
+package s3host
+
+import (
+	"context"
+	"errors"
+
+	"github.com/shellcell/snailmail/host"
+)
+
+// NewAWS reports that this build excludes S3 support. The protocol logic in
+// this package is SDK-free and still compiled; only the AWS client is absent.
+func NewAWS(_ context.Context, _ host.Repository, _ ...host.CredentialBroker) (*Adapter, error) {
+	return nil, errors.New("this snailmail build was compiled without S3 support (nos3)")
+}

adapters/host/s3/aws_test.go

@@ -1,3 +1,5 @@
+//go:build !nos3
+
 package s3host
 
 import (

adapters/host/s3/parallel.go

@@ -0,0 +1,64 @@
+package s3host
+
+import (
+	"context"
+	"runtime"
+	"sync"
+	"sync/atomic"
+)
+
+// defaultObjectConcurrency bounds in-flight object requests. Publication is
+// dominated by round-trip latency rather than local work, so this is
+// deliberately higher than the CPU count, but still low enough to stay well
+// inside per-connection limits and to keep a failure from fanning out widely.
+var defaultObjectConcurrency = min(16, max(4, runtime.GOMAXPROCS(0)*4))
+
+// forEachObject runs work over every index in [0, count) with bounded
+// concurrency and returns the error belonging to the lowest index, so that a
+// failure is reported identically no matter which request lost the race.
+//
+// The first failure stops further indices being dispatched, but requests
+// already in flight are allowed to finish against the caller's context rather
+// than being cancelled. Cancelling them would replace a genuine failure at a
+// lower index with a self-inflicted "context canceled", which is both
+// misleading and non-deterministic.
+func forEachObject(ctx context.Context, count int, work func(ctx context.Context, index int) error) error {
+	if count == 0 {
+		return nil
+	}
+	if count == 1 {
+		return work(ctx, 0)
+	}
+	workers := min(defaultObjectConcurrency, count)
+	errs := make([]error, count)
+	var failed atomic.Bool
+	next := make(chan int)
+	var waitGroup sync.WaitGroup
+	waitGroup.Add(workers)
+	for range workers {
+		go func() {
+			defer waitGroup.Done()
+			for index := range next {
+				if err := work(ctx, index); err != nil {
+					errs[index] = err
+					failed.Store(true)
+				}
+			}
+		}()
+	}
+	for index := range count {
+		if failed.Load() || ctx.Err() != nil {
+			break
+		}
+		next <- index
+	}
+	close(next)
+	waitGroup.Wait()
+	for _, err := range errs {
+		if err != nil {
+			return err
+		}
+	}
+	// No request recorded a failure, so any stop came from the caller.
+	return ctx.Err()
+}