diff --git a/README.md b/README.md index 6dbd1c5..ca85e57 100644 --- a/README.md +++ b/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 diff --git a/adapters/blob/s3/aws.go b/adapters/blob/s3/aws.go index 5b1097a..c836cf9 100644 --- a/adapters/blob/s3/aws.go +++ b/adapters/blob/s3/aws.go @@ -1,3 +1,5 @@ +//go:build !nos3 + package s3blob import ( diff --git a/adapters/blob/s3/aws_disabled.go b/adapters/blob/s3/aws_disabled.go new file mode 100644 index 0000000..80c5abb --- /dev/null +++ b/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)") +} diff --git a/adapters/blob/s3/aws_test.go b/adapters/blob/s3/aws_test.go index 22f160f..492d88a 100644 --- a/adapters/blob/s3/aws_test.go +++ b/adapters/blob/s3/aws_test.go @@ -1,3 +1,5 @@ +//go:build !nos3 + package s3blob import ( diff --git a/adapters/credential/command/command.go b/adapters/credential/command/command.go index 764c6ba..4897182 100644 --- a/adapters/credential/command/command.go +++ b/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{} } diff --git a/adapters/credential/command/retain_linux.go b/adapters/credential/command/retain_linux.go new file mode 100644 index 0000000..350d5d7 --- /dev/null +++ b/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 +} diff --git a/adapters/credential/command/retain_other.go b/adapters/credential/command/retain_other.go new file mode 100644 index 0000000..11c1c3e --- /dev/null +++ b/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 +} diff --git a/adapters/host/s3/aws.go b/adapters/host/s3/aws.go index bc74da1..b595e0f 100644 --- a/adapters/host/s3/aws.go +++ b/adapters/host/s3/aws.go @@ -1,3 +1,5 @@ +//go:build !nos3 + package s3host import ( diff --git a/adapters/host/s3/aws_disabled.go b/adapters/host/s3/aws_disabled.go new file mode 100644 index 0000000..ff4c110 --- /dev/null +++ b/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)") +} diff --git a/adapters/host/s3/aws_test.go b/adapters/host/s3/aws_test.go index 02b0368..b57e986 100644 --- a/adapters/host/s3/aws_test.go +++ b/adapters/host/s3/aws_test.go @@ -1,3 +1,5 @@ +//go:build !nos3 + package s3host import ( diff --git a/adapters/host/s3/parallel.go b/adapters/host/s3/parallel.go new file mode 100644 index 0000000..58e5383 --- /dev/null +++ b/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() +} diff --git a/adapters/host/s3/parallel_test.go b/adapters/host/s3/parallel_test.go new file mode 100644 index 0000000..7bb68a3 --- /dev/null +++ b/adapters/host/s3/parallel_test.go @@ -0,0 +1,113 @@ +package s3host + +import ( + "context" + "errors" + "fmt" + "sync" + "sync/atomic" + "testing" + "time" +) + +func TestForEachObjectRunsEveryIndexOnce(t *testing.T) { + const count = 500 + var mutex sync.Mutex + seen := make(map[int]int, count) + if err := forEachObject(context.Background(), count, func(_ context.Context, index int) error { + mutex.Lock() + defer mutex.Unlock() + seen[index]++ + return nil + }); err != nil { + t.Fatal(err) + } + if len(seen) != count { + t.Fatalf("ran %d indices, want %d", len(seen), count) + } + for index, times := range seen { + if times != 1 { + t.Fatalf("index %d ran %d times", index, times) + } + } +} + +// The pool must have several requests in flight at once. This waits for a +// rendezvous rather than sampling a counter: sampling measures parallelism, +// which a single-CPU machine cannot show even when the pool is working +// correctly, whereas independent progress is what the pool actually promises. +func TestForEachObjectOverlapsRequests(t *testing.T) { + const overlapping = 2 + arrived := make(chan struct{}, overlapping) + release := make(chan struct{}) + go func() { + for range overlapping { + <-arrived + } + close(release) + }() + err := forEachObject(context.Background(), 8, func(_ context.Context, index int) error { + if index >= overlapping { + return nil + } + arrived <- struct{}{} + select { + case <-release: + return nil + case <-time.After(30 * time.Second): + // Only reachable if the pool ran these one after another, since the + // release waits for both to have started. + return fmt.Errorf("request %d ran alone; the pool did not overlap requests", index) + } + }) + if err != nil { + t.Fatal(err) + } +} + +// Whichever request loses the race, the reported failure must be the same one, +// otherwise an operator sees a different error for the same broken state. +func TestForEachObjectReportsLowestFailingIndex(t *testing.T) { + for attempt := range 50 { + err := forEachObject(context.Background(), 200, func(_ context.Context, index int) error { + if index == 7 || index == 90 || index == 150 { + return fmt.Errorf("object %d failed", index) + } + return nil + }) + if err == nil || err.Error() != "object 7 failed" { + t.Fatalf("attempt %d reported %v, want the lowest failing index", attempt, err) + } + } +} + +func TestForEachObjectStopsOnCancelledContext(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + var ran atomic.Int64 + err := forEachObject(ctx, 100, func(_ context.Context, _ int) error { + ran.Add(1) + return nil + }) + if !errors.Is(err, context.Canceled) { + t.Fatalf("error = %v, want context.Canceled", err) + } + if ran.Load() == 100 { + t.Fatal("cancellation did not stop the remaining work") + } +} + +func TestForEachObjectHandlesEmptyAndSingle(t *testing.T) { + if err := forEachObject(context.Background(), 0, func(context.Context, int) error { + t.Fatal("empty work ran") + return nil + }); err != nil { + t.Fatal(err) + } + sentinel := errors.New("single") + if err := forEachObject(context.Background(), 1, func(context.Context, int) error { + return sentinel + }); !errors.Is(err, sentinel) { + t.Fatalf("error = %v, want the sentinel", err) + } +} diff --git a/adapters/host/s3/s3.go b/adapters/host/s3/s3.go index 1a23d59..eca63b6 100644 --- a/adapters/host/s3/s3.go +++ b/adapters/host/s3/s3.go @@ -141,17 +141,21 @@ func (adapter *Adapter) Observe(ctx context.Context, repository host.Repository) return host.PublishedRevision{}, &host.Error{Kind: host.ErrorIndeterminate, Operation: "observe S3 restore state", Err: errors.New("bound retained root is corrupt")} } } - for _, file := range descriptor.Files { + if err := forEachObject(ctx, len(descriptor.Files), func(ctx context.Context, index int) error { + file := descriptor.Files[index] info, err := adapter.client.Head(ctx, releaseKey(repository, revision.TreeSHA256, file.Path)) if errors.Is(err, ErrNotFound) { - return host.PublishedRevision{}, &host.Error{Kind: host.ErrorIndeterminate, Operation: "observe S3 release", Err: fmt.Errorf("release object %q is missing or corrupt", file.Path)} + return &host.Error{Kind: host.ErrorIndeterminate, Operation: "observe S3 release", Err: fmt.Errorf("release object %q is missing or corrupt", file.Path)} } if err != nil { - return host.PublishedRevision{}, infrastructure("inspect S3 release object", err) + return infrastructure("inspect S3 release object", err) } if info.Size != file.Size || info.SHA256 != file.SHA256 || info.Metadata["sha256"] != file.SHA256 { - return host.PublishedRevision{}, &host.Error{Kind: host.ErrorIndeterminate, Operation: "observe S3 release", Err: fmt.Errorf("release object %q is missing or corrupt", file.Path)} + return &host.Error{Kind: host.ErrorIndeterminate, Operation: "observe S3 release", Err: fmt.Errorf("release object %q is missing or corrupt", file.Path)} } + return nil + }); err != nil { + return host.PublishedRevision{}, err } releaseRoot, _, err := adapter.client.Get(ctx, releaseKey(repository, revision.TreeSHA256, pypiRootPath), maximumMetadataSize) if err != nil { @@ -264,35 +268,42 @@ func (adapter *Adapter) Stage(ctx context.Context, repository host.Repository, r resultErr = fmt.Errorf("%w; clean partial S3 stage: %v", resultErr, cleanupErr) } }() + // Every key is recorded before any upload starts so the deferred cleanup + // removes partial stages regardless of which uploads were in flight. for _, file := range descriptor.Files { + uploaded = append(uploaded, stageKey(repository, identifier, file.Path)) + } + if err := forEachObject(ctx, len(descriptor.Files), func(ctx context.Context, index int) error { + file := descriptor.Files[index] name := filepath.Join(request.Directory, filepath.FromSlash(file.Path)) actualSize, actualDigest, err := hashFile(name) if err != nil { - return host.StagedPublication{}, err + return err } if actualSize != file.Size || actualDigest != file.SHA256 { - return host.StagedPublication{}, &host.Error{Kind: host.ErrorStale, Operation: "stage S3 repository", Err: fmt.Errorf("file %q changed after verification", file.Path)} + return &host.Error{Kind: host.ErrorStale, Operation: "stage S3 repository", Err: fmt.Errorf("file %q changed after verification", file.Path)} } input, err := os.Open(name) if err != nil { - return host.StagedPublication{}, err + return err } - key := stageKey(repository, identifier, file.Path) - uploaded = append(uploaded, key) stored, putErr := adapter.client.Put(ctx, PutRequest{ - Key: key, Body: input, Size: file.Size, SHA256: file.SHA256, + Key: stageKey(repository, identifier, file.Path), Body: input, Size: file.Size, SHA256: file.SHA256, ContentType: contentType(file.Path), Metadata: map[string]string{"sha256": file.SHA256}, }) closeErr := input.Close() if putErr != nil { - return host.StagedPublication{}, infrastructure("upload S3 stage object", putErr) + return infrastructure("upload S3 stage object", putErr) } if closeErr != nil { - return host.StagedPublication{}, closeErr + return closeErr } if stored.Size != file.Size || stored.SHA256 != file.SHA256 || stored.Metadata["sha256"] != file.SHA256 { - return host.StagedPublication{}, &host.Error{Kind: host.ErrorInfrastructure, Operation: "verify S3 stage object", Err: fmt.Errorf("object %q metadata does not match", file.Path)} + return &host.Error{Kind: host.ErrorInfrastructure, Operation: "verify S3 stage object", Err: fmt.Errorf("object %q metadata does not match", file.Path)} } + return nil + }); err != nil { + return host.StagedPublication{}, err } manifest, err := app.VerifyRepository(request.Directory) if err != nil || manifest.Format != pypi.FormatID || manifest.TreeSHA256 != request.TreeSHA256 { @@ -699,25 +710,29 @@ func descriptorFromRequest(request host.StageRequest) (publicationDescriptor, st func (adapter *Adapter) materializeRelease(ctx context.Context, repository host.Repository, stageID string, publication publicationDescriptor) (string, error) { descriptor := releaseDescriptorFromPublication(publication) - for _, file := range descriptor.Files { + if err := forEachObject(ctx, len(descriptor.Files), func(ctx context.Context, index int) error { + file := descriptor.Files[index] destination := releaseKey(repository, descriptor.TreeSHA256, file.Path) stored, err := adapter.client.CopyCreate(ctx, stageKey(repository, stageID, file.Path), destination, file.Size, file.SHA256) if errors.Is(err, ErrPrecondition) { stored, err = adapter.client.Head(ctx, destination) if err != nil { - return "", infrastructure("reconcile S3 release object", err) + return infrastructure("reconcile S3 release object", err) } if stored.Size != file.Size || stored.SHA256 != file.SHA256 || stored.Metadata["sha256"] != file.SHA256 { - return "", &host.Error{Kind: host.ErrorIndeterminate, Operation: "materialize S3 release", Err: fmt.Errorf("immutable release object %q conflicts", file.Path)} + return &host.Error{Kind: host.ErrorIndeterminate, Operation: "materialize S3 release", Err: fmt.Errorf("immutable release object %q conflicts", file.Path)} } - continue + return nil } if err != nil { - return "", infrastructure("materialize S3 release object", err) + return infrastructure("materialize S3 release object", err) } if stored.Size != file.Size || stored.SHA256 != file.SHA256 || stored.Metadata["sha256"] != file.SHA256 { - return "", &host.Error{Kind: host.ErrorInfrastructure, Operation: "verify S3 release object", Err: fmt.Errorf("object %q metadata does not match", file.Path)} + return &host.Error{Kind: host.ErrorInfrastructure, Operation: "verify S3 release object", Err: fmt.Errorf("object %q metadata does not match", file.Path)} } + return nil + }); err != nil { + return "", err } content, err := json.Marshal(descriptor) if err != nil { diff --git a/cmd/snailmail/flags.go b/cmd/snailmail/flags.go new file mode 100644 index 0000000..ec8c585 --- /dev/null +++ b/cmd/snailmail/flags.go @@ -0,0 +1,88 @@ +package main + +import ( + "errors" + "flag" + "fmt" + "io" + "time" +) + +// commandFlags removes the boilerplate every subcommand repeated: routing usage +// output to stderr, declaring the shared --workspace flag, and rejecting +// positional arguments the command does not take. +type commandFlags struct { + set *flag.FlagSet + workspace *string +} + +func newCommandFlags(name string, stderr io.Writer) *commandFlags { + set := flag.NewFlagSet(name, flag.ContinueOnError) + set.SetOutput(stderr) + return &commandFlags{set: set} +} + +// withWorkspace declares the shared workspace root flag. +func (flags *commandFlags) withWorkspace() *commandFlags { + flags.workspace = flags.set.String("workspace", ".", "workspace root") + return flags +} + +func (flags *commandFlags) String(name, value, usage string) *string { + return flags.set.String(name, value, usage) +} + +func (flags *commandFlags) Bool(name string, value bool, usage string) *bool { + return flags.set.Bool(name, value, usage) +} + +func (flags *commandFlags) Int(name string, value int, usage string) *int { + return flags.set.Int(name, value, usage) +} + +func (flags *commandFlags) Int64(name string, value int64, usage string) *int64 { + return flags.set.Int64(name, value, usage) +} + +func (flags *commandFlags) Duration(name string, value time.Duration, usage string) *time.Duration { + return flags.set.Duration(name, value, usage) +} + +// Root is the resolved workspace root, or "." for commands without the flag. +func (flags *commandFlags) Root() string { + if flags.workspace == nil { + return "." + } + return *flags.workspace +} + +// Parse, NArg, Arg and Args pass through for the commands that take positional +// arguments and validate the count themselves. +func (flags *commandFlags) Parse(arguments []string) error { return flags.set.Parse(arguments) } +func (flags *commandFlags) NArg() int { return flags.set.NArg() } +func (flags *commandFlags) Arg(index int) string { return flags.set.Arg(index) } +func (flags *commandFlags) Args() []string { return flags.set.Args() } + +// parse reads arguments and rejects any positional argument, which every +// command using it treats as a usage error. +func (flags *commandFlags) parse(arguments []string) error { + if err := flags.set.Parse(arguments); err != nil { + return err + } + if flags.set.NArg() != 0 { + return fmt.Errorf("unexpected argument %q", flags.set.Arg(0)) + } + return nil +} + +// parseWithArguments reads arguments and requires exactly count positional +// arguments, reporting usage when the count does not match. +func (flags *commandFlags) parseWithArguments(arguments []string, count int, usage string) ([]string, error) { + if err := flags.set.Parse(arguments); err != nil { + return nil, err + } + if flags.set.NArg() != count { + return nil, errors.New(usage) + } + return flags.set.Args(), nil +} diff --git a/cmd/snailmail/main.go b/cmd/snailmail/main.go index d32e74b..3a0465c 100644 --- a/cmd/snailmail/main.go +++ b/cmd/snailmail/main.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "errors" - "flag" "fmt" "io" "net" @@ -91,18 +90,13 @@ func run(ctx context.Context, args []string, stdout, stderr io.Writer) error { } func runInit(args []string, stdout, stderr io.Writer) error { - flags := flag.NewFlagSet("init", flag.ContinueOnError) - flags.SetOutput(stderr) - workspace := flags.String("workspace", ".", "workspace root") + flags := newCommandFlags("init", stderr).withWorkspace() name := flags.String("name", "", "workspace name") forgeRepository := flags.String("forge-repo", "", "GitHub state repository (owner/name) for PR gates") - if err := flags.Parse(args); err != nil { + if err := flags.parse(args); err != nil { return err } - if flags.NArg() != 0 { - return fmt.Errorf("unexpected argument %q", flags.Arg(0)) - } - if err := engine.InitWorkspace(engine.InitWorkspaceRequest{Root: *workspace, Name: *name, ForgeRepository: *forgeRepository}); err != nil { + if err := engine.InitWorkspace(engine.InitWorkspaceRequest{Root: flags.Root(), Name: *name, ForgeRepository: *forgeRepository}); err != nil { return err } printBrand(stdout) @@ -115,9 +109,7 @@ func runSetup(args []string, stdout, stderr io.Writer) error { return errors.New("usage: snailmail setup --name NAME --host [host options]") } format := args[0] - flags := flag.NewFlagSet("setup "+format, flag.ContinueOnError) - flags.SetOutput(stderr) - workspace := flags.String("workspace", ".", "workspace root") + flags := newCommandFlags("setup "+format, stderr).withWorkspace() name := flags.String("name", "", "repository name") output := flags.String("output", "", "workspace-relative published directory") hostType := flags.String("host", "local", "host type: local, s3, or github-pages") @@ -143,16 +135,13 @@ func runSetup(args []string, stdout, stderr io.Writer) error { suite := flags.String("suite", "stable", "Debian suite") component := flags.String("component", "main", "Debian component") architectures := flags.String("architectures", "amd64", "comma-separated Debian architectures") - if err := flags.Parse(args[1:]); err != nil { + if err := flags.parse(args[1:]); err != nil { return err } - if flags.NArg() != 0 { - return fmt.Errorf("unexpected argument %q", flags.Arg(0)) - } resolvedApprovalKeys := splitList(*approvalKeys) sort.Strings(resolvedApprovalKeys) if err := engine.SetupRepository(engine.SetupRepositoryRequest{ - Root: *workspace, Name: *name, Format: format, Output: *output, + Root: flags.Root(), Name: *name, Format: format, Output: *output, HostType: *hostType, Visibility: *visibility, Gate: *gatePolicy, ApprovalKeys: resolvedApprovalKeys, SigningKey: *signingKey, AllowUnsigned: *allowUnsigned, Bucket: *bucket, Prefix: *prefix, Region: *region, Endpoint: *endpoint, CanonicalEndpoint: *canonicalEndpoint, UsePathStyle: *usePathStyle, @@ -183,22 +172,17 @@ func runKeys(ctx context.Context, args []string, stdout, stderr io.Writer) error return errors.New("usage: snailmail keys new NAME [--algo openpgp-rsa4096] [--expires-in 17520h]") } name := args[1] - flags := flag.NewFlagSet("keys new", flag.ContinueOnError) - flags.SetOutput(stderr) - workspace := flags.String("workspace", ".", "workspace root") + flags := newCommandFlags("keys new", stderr).withWorkspace() algorithm := flags.String("algo", "openpgp-rsa4096", "signing algorithm") expiresIn := flags.Duration("expires-in", 2*365*24*time.Hour, "key validity duration") - if err := flags.Parse(args[2:]); err != nil { + if err := flags.parse(args[2:]); err != nil { return err } - if flags.NArg() != 0 { - return fmt.Errorf("unexpected argument %q", flags.Arg(0)) - } store, err := wire.NewSignerStore() if err != nil { return err } - result, err := engine.NewKey(ctx, engine.NewKeyRequest{Root: *workspace, Name: name, Algorithm: *algorithm, ExpiresIn: *expiresIn, Keys: store}) + result, err := engine.NewKey(ctx, engine.NewKeyRequest{Root: flags.Root(), Name: name, Algorithm: *algorithm, ExpiresIn: *expiresIn, Keys: store}) if err != nil { return err } @@ -212,20 +196,15 @@ func runKeys(ctx context.Context, args []string, stdout, stderr io.Writer) error return errors.New("usage: snailmail keys publish NAME") } name := args[1] - flags := flag.NewFlagSet("keys publish", flag.ContinueOnError) - flags.SetOutput(stderr) - workspace := flags.String("workspace", ".", "workspace root") - if err := flags.Parse(args[2:]); err != nil { + flags := newCommandFlags("keys publish", stderr).withWorkspace() + if err := flags.parse(args[2:]); err != nil { return err } - if flags.NArg() != 0 { - return fmt.Errorf("unexpected argument %q", flags.Arg(0)) - } store, err := wire.NewSignerStore() if err != nil { return err } - result, err := engine.PublishKey(ctx, engine.PublishKeyRequest{Root: *workspace, Name: name, Keys: store}) + result, err := engine.PublishKey(ctx, engine.PublishKeyRequest{Root: flags.Root(), Name: name, Keys: store}) if err != nil { return err } @@ -238,20 +217,15 @@ func runKeys(ctx context.Context, args []string, stdout, stderr io.Writer) error return errors.New("usage: snailmail keys rotate REPOSITORY --successor KEY [--minimum-refresh 720h] | --advance --yes") } repository := args[1] - flags := flag.NewFlagSet("keys rotate", flag.ContinueOnError) - flags.SetOutput(stderr) - workspace := flags.String("workspace", ".", "workspace root") + flags := newCommandFlags("keys rotate", stderr).withWorkspace() successor := flags.String("successor", "", "successor signing key name") advance := flags.Bool("advance", false, "advance the deployed rotation to its next phase") minimumRefresh := flags.Duration("minimum-refresh", 30*24*time.Hour, "minimum client keyring refresh window") expiresIn := flags.Duration("expires-in", 2*365*24*time.Hour, "new successor key validity") confirmed := flags.Bool("yes", false, "confirm an advance transition") - if err := flags.Parse(args[2:]); err != nil { + if err := flags.parse(args[2:]); err != nil { return err } - if flags.NArg() != 0 { - return fmt.Errorf("unexpected argument %q", flags.Arg(0)) - } if *advance && !*confirmed { return errors.New("keys rotate --advance requires --yes") } @@ -267,7 +241,7 @@ func runKeys(ctx context.Context, args []string, stdout, stderr io.Writer) error } } result, err := engine.RotateKey(ctx, engine.RotateKeyRequest{ - Root: *workspace, Repository: repository, Successor: *successor, Advance: *advance, + Root: flags.Root(), Repository: repository, Successor: *successor, Advance: *advance, MinimumRefresh: *minimumRefresh, ExpiresIn: *expiresIn, Keys: keyGenerator, }) if err != nil { @@ -284,16 +258,11 @@ func runKeys(ctx context.Context, args []string, stdout, stderr io.Writer) error } return nil case "audit": - flags := flag.NewFlagSet("keys audit", flag.ContinueOnError) - flags.SetOutput(stderr) - workspace := flags.String("workspace", ".", "workspace root") - if err := flags.Parse(args[1:]); err != nil { + flags := newCommandFlags("keys audit", stderr).withWorkspace() + if err := flags.parse(args[1:]); err != nil { return err } - if flags.NArg() != 0 { - return fmt.Errorf("unexpected argument %q", flags.Arg(0)) - } - result, err := engine.AuditKeys(engine.PublishKeyRequest{Root: *workspace}, time.Time{}) + result, err := engine.AuditKeys(engine.PublishKeyRequest{Root: flags.Root()}, time.Time{}) if err != nil { return err } @@ -325,9 +294,7 @@ func runKeys(ctx context.Context, args []string, stdout, stderr io.Writer) error } func runAdd(ctx context.Context, args []string, stdout, stderr io.Writer) error { - flags := flag.NewFlagSet("add", flag.ContinueOnError) - flags.SetOutput(stderr) - workspace := flags.String("workspace", ".", "workspace root") + flags := newCommandFlags("add", stderr).withWorkspace() track := flags.String("track", "stable", "placement track") if err := flags.Parse(args); err != nil { return err @@ -336,7 +303,7 @@ func runAdd(ctx context.Context, args []string, stdout, stderr io.Writer) error return errors.New("usage: snailmail add [--track stable] REPOSITORY ARTIFACT...") } result, err := engine.AddArtifacts(engine.AddArtifactsRequest{ - Context: ctx, Root: *workspace, Repository: flags.Arg(0), Artifacts: flags.Args()[1:], Track: *track, + Context: ctx, Root: flags.Root(), Repository: flags.Arg(0), Artifacts: flags.Args()[1:], Track: *track, Blobs: wire.NewBlobResolver(), }) if err != nil { @@ -355,9 +322,7 @@ func runAdd(ctx context.Context, args []string, stdout, stderr io.Writer) error } func runPromote(args []string, stdout, stderr io.Writer) error { - flags := flag.NewFlagSet("promote", flag.ContinueOnError) - flags.SetOutput(stderr) - workspace := flags.String("workspace", ".", "workspace root") + flags := newCommandFlags("promote", stderr).withWorkspace() track := flags.String("track", "stable", "destination placement track") distro := flags.String("distro", "", "Debian placement distro (defaults to repository suite)") if err := flags.Parse(args); err != nil { @@ -367,7 +332,7 @@ func runPromote(args []string, stdout, stderr io.Writer) error { return errors.New("usage: snailmail promote [--track stable] [--distro DISTRO] REPOSITORY PACKAGE VERSION") } result, err := engine.Promote(engine.PlacementMutationRequest{ - Root: *workspace, Repository: flags.Arg(0), Package: flags.Arg(1), Version: flags.Arg(2), Track: *track, Distro: *distro, + Root: flags.Root(), Repository: flags.Arg(0), Package: flags.Arg(1), Version: flags.Arg(2), Track: *track, Distro: *distro, }) if err != nil { return err @@ -382,9 +347,7 @@ func runPromote(args []string, stdout, stderr io.Writer) error { } func runYank(args []string, stdout, stderr io.Writer) error { - flags := flag.NewFlagSet("yank", flag.ContinueOnError) - flags.SetOutput(stderr) - workspace := flags.String("workspace", ".", "workspace root") + flags := newCommandFlags("yank", stderr).withWorkspace() track := flags.String("track", "", "placement track to remove") distro := flags.String("distro", "", "Debian placement distro (defaults to repository suite)") all := flags.Bool("all", false, "remove every placement for the package version") @@ -395,7 +358,7 @@ func runYank(args []string, stdout, stderr io.Writer) error { return errors.New("usage: snailmail yank (--track TRACK [--distro DISTRO] | --all) REPOSITORY PACKAGE VERSION") } result, err := engine.Yank(engine.PlacementMutationRequest{ - Root: *workspace, Repository: flags.Arg(0), Package: flags.Arg(1), Version: flags.Arg(2), Track: *track, Distro: *distro, All: *all, + Root: flags.Root(), Repository: flags.Arg(0), Package: flags.Arg(1), Version: flags.Arg(2), Track: *track, Distro: *distro, All: *all, }) if err != nil { return err @@ -416,9 +379,7 @@ func runPrune(args []string, stdout, stderr io.Writer) error { return errors.New("usage: snailmail prune REPOSITORY --keep N") } repository := args[0] - flags := flag.NewFlagSet("prune", flag.ContinueOnError) - flags.SetOutput(stderr) - workspace := flags.String("workspace", ".", "workspace root") + flags := newCommandFlags("prune", stderr).withWorkspace() keep := flags.Int("keep", 0, "versions to retain per package, track, and distro") if err := flags.Parse(args[1:]); err != nil { return err @@ -426,7 +387,7 @@ func runPrune(args []string, stdout, stderr io.Writer) error { if flags.NArg() != 0 || *keep < 1 { return errors.New("usage: snailmail prune REPOSITORY --keep N") } - result, err := engine.Prune(engine.PruneRequest{Root: *workspace, Repository: repository, Keep: *keep}) + result, err := engine.Prune(engine.PruneRequest{Root: flags.Root(), Repository: repository, Keep: *keep}) if err != nil { return err } @@ -441,9 +402,7 @@ func runPrune(args []string, stdout, stderr io.Writer) error { } func runCheck(ctx context.Context, args []string, stdout, stderr io.Writer) error { - flags := flag.NewFlagSet("check", flag.ContinueOnError) - flags.SetOutput(stderr) - workspace := flags.String("workspace", ".", "workspace root") + flags := newCommandFlags("check", stderr).withWorkspace() origins := flags.Bool("origins", false, "re-fetch recorded adopted origins") maxOrigins := flags.Int("max-origins", 2, "maximum recorded origins to re-fetch (1-4)") originOffset := flags.Int("origin-offset", 0, "skip this many sorted recorded origins") @@ -453,7 +412,7 @@ func runCheck(ctx context.Context, args []string, stdout, stderr io.Writer) erro if flags.NArg() != 0 { return errors.New("usage: snailmail check [--workspace DIR] [--origins --max-origins N --origin-offset N]") } - result, err := engine.CheckWorkspace(ctx, engine.CheckWorkspaceRequest{Root: *workspace, Blobs: wire.NewBlobResolver(), Origins: *origins, Sources: httpsource.New(), MaxOrigins: *maxOrigins, OriginOffset: *originOffset}) + result, err := engine.CheckWorkspace(ctx, engine.CheckWorkspaceRequest{Root: flags.Root(), Blobs: wire.NewBlobResolver(), Origins: *origins, Sources: httpsource.New(), MaxOrigins: *maxOrigins, OriginOffset: *originOffset}) if err != nil { return err } @@ -475,9 +434,7 @@ func runCheck(ctx context.Context, args []string, stdout, stderr io.Writer) erro } func runStatus(ctx context.Context, args []string, stdout, stderr io.Writer) error { - flags := flag.NewFlagSet("status", flag.ContinueOnError) - flags.SetOutput(stderr) - workspace := flags.String("workspace", ".", "workspace root") + flags := newCommandFlags("status", stderr).withWorkspace() jsonOutput := flags.Bool("json", false, "emit machine-readable JSON") if err := flags.Parse(args); err != nil { return err @@ -485,7 +442,7 @@ func runStatus(ctx context.Context, args []string, stdout, stderr io.Writer) err if flags.NArg() != 0 { return errors.New("usage: snailmail status [--workspace DIR] [--json]") } - result, err := engine.StatusWorkspace(ctx, engine.StatusWorkspaceRequest{Root: *workspace}) + result, err := engine.StatusWorkspace(ctx, engine.StatusWorkspaceRequest{Root: flags.Root()}) if err != nil { return err } @@ -509,8 +466,7 @@ func runDoctor(ctx context.Context, args []string, stdout, stderr io.Writer) err } func runDoctorWithFetcher(ctx context.Context, args []string, stdout, stderr io.Writer, fetcher source.Fetcher) error { - flags := flag.NewFlagSet("doctor", flag.ContinueOnError) - flags.SetOutput(stderr) + flags := newCommandFlags("doctor", stderr) format := flags.String("format", "auto", "repository format: auto, pypi, deb, or helm") project := flags.String("project", "", "PyPI project to inspect") suite := flags.String("suite", "", "Debian suite") @@ -561,9 +517,7 @@ func runAdopt(ctx context.Context, args []string, stdout, stderr io.Writer) erro } func runAdoptWithFetcher(ctx context.Context, args []string, stdout, stderr io.Writer, fetcher source.Fetcher) error { - flags := flag.NewFlagSet("adopt", flag.ContinueOnError) - flags.SetOutput(stderr) - workspace := flags.String("workspace", ".", "workspace root") + flags := newCommandFlags("adopt", stderr).withWorkspace() digest := flags.String("sha256", "", "required artifact SHA-256 pin") filename := flags.String("filename", "", "artifact filename override") track := flags.String("track", "", "placement track") @@ -578,7 +532,7 @@ func runAdoptWithFetcher(ctx context.Context, args []string, stdout, stderr io.W return errors.New("usage: snailmail adopt --sha256 HEX --public-origin [options] REPOSITORY URL") } result, err := engine.AdoptArtifact(ctx, engine.AdoptArtifactRequest{ - Root: *workspace, Repository: flags.Arg(0), URL: flags.Arg(1), SHA256: *digest, + Root: flags.Root(), Repository: flags.Arg(0), URL: flags.Arg(1), SHA256: *digest, Filename: *filename, Track: *track, Distro: *distro, DryRun: *dryRun, PublicOrigin: *publicOrigin, Fetcher: fetcher, }) if err != nil { @@ -606,22 +560,17 @@ func runBlobStore(ctx context.Context, args []string, stdout, stderr io.Writer) return errors.New("usage: snailmail blob-store [options]") } storeType := args[0] - flags := flag.NewFlagSet("blob-store "+storeType, flag.ContinueOnError) - flags.SetOutput(stderr) - workspace := flags.String("workspace", ".", "workspace root") + flags := newCommandFlags("blob-store "+storeType, stderr).withWorkspace() bucket := flags.String("bucket", "", "S3 blob bucket") prefix := flags.String("prefix", "", "S3 blob prefix") region := flags.String("region", "", "AWS region") endpoint := flags.String("endpoint", "", "optional S3 API endpoint") usePathStyle := flags.Bool("use-path-style", false, "use path-style S3 requests") - if err := flags.Parse(args[1:]); err != nil { + if err := flags.parse(args[1:]); err != nil { return err } - if flags.NArg() != 0 { - return fmt.Errorf("unexpected argument %q", flags.Arg(0)) - } if err := engine.ConfigureBlobStore(ctx, engine.ConfigureBlobStoreRequest{ - Root: *workspace, Type: storeType, Bucket: *bucket, Prefix: *prefix, Region: *region, + Root: flags.Root(), Type: storeType, Bucket: *bucket, Prefix: *prefix, Region: *region, Endpoint: *endpoint, UsePathStyle: *usePathStyle, Blobs: wire.NewBlobResolver(), }); err != nil { return err @@ -633,19 +582,14 @@ func runBlobStore(ctx context.Context, args []string, stdout, stderr io.Writer) } func runPlan(ctx context.Context, args []string, stdout, stderr io.Writer) error { - flags := flag.NewFlagSet("plan", flag.ContinueOnError) - flags.SetOutput(stderr) - workspace := flags.String("workspace", ".", "workspace root") + flags := newCommandFlags("plan", stderr).withWorkspace() output := flags.String("out", "snailmail.snailmail-plan.json", "plan output file") generatedAtValue := flags.String("generated-at", "", "explicit RFC3339 repository generation time") expires := flags.Duration("expires", 2*time.Hour, "plan lifetime") structuralOnly := flags.Bool("structural-only", false, "review a plan without ecosystem client verification") - if err := flags.Parse(args); err != nil { + if err := flags.parse(args); err != nil { return err } - if flags.NArg() != 0 { - return fmt.Errorf("unexpected argument %q", flags.Arg(0)) - } generatedAt, err := optionalTime(*generatedAtValue) if err != nil { return err @@ -657,7 +601,7 @@ func runPlan(ctx context.Context, args []string, stdout, stderr io.Writer) error return err } result, err := engine.PlanWorkspace(ctx, engine.PlanWorkspaceRequest{ - Root: *workspace, Output: *output, GeneratedAt: generatedAt, ExpiresIn: *expires, + Root: flags.Root(), Output: *output, GeneratedAt: generatedAt, ExpiresIn: *expires, Hosts: hosts, Blobs: wire.NewBlobResolver(), Signers: signers, VerificationMode: verificationMode(*structuralOnly), }) if err != nil { @@ -675,9 +619,7 @@ func runPlan(ctx context.Context, args []string, stdout, stderr io.Writer) error } func runApply(ctx context.Context, args []string, stdout, stderr io.Writer) error { - flags := flag.NewFlagSet("apply", flag.ContinueOnError) - flags.SetOutput(stderr) - workspace := flags.String("workspace", ".", "workspace root") + flags := newCommandFlags("apply", stderr).withWorkspace() plan := flags.String("plan", "snailmail.snailmail-plan.json", "reviewed plan file") structuralOnly := flags.Bool("structural-only", false, "skip ecosystem client verification") python := flags.String("python", "python3", "Python executable for PyPI verification") @@ -686,12 +628,9 @@ func runApply(ctx context.Context, args []string, stdout, stderr io.Writer) erro helmImage := flags.String("helm-image", engine.DefaultHelmVerificationImage, "digest-pinned Helm image") maxWorkspaceMiB := flags.Int64("max-workspace-mib", 4096, "maximum Debian verification workspace in MiB") approvalFile := flags.String("approvals", "", "approval evidence file (defaults beside plan)") - if err := flags.Parse(args); err != nil { + if err := flags.parse(args); err != nil { return err } - if flags.NArg() != 0 { - return fmt.Errorf("unexpected argument %q", flags.Arg(0)) - } hosts := wire.NewHostResolver() defer hosts.Close() resolvedApprovalFile := *approvalFile @@ -699,10 +638,10 @@ func runApply(ctx context.Context, args []string, stdout, stderr io.Writer) erro resolvedApprovalFile = *plan + ".approvals.json" } if !filepath.IsAbs(resolvedApprovalFile) { - resolvedApprovalFile = filepath.Join(*workspace, resolvedApprovalFile) + resolvedApprovalFile = filepath.Join(flags.Root(), resolvedApprovalFile) } result, err := engine.ApplyWorkspace(ctx, engine.ApplyWorkspaceRequest{ - Root: *workspace, Plan: *plan, StructuralOnly: *structuralOnly, Python: *python, Runner: *runner, + Root: flags.Root(), Plan: *plan, StructuralOnly: *structuralOnly, Python: *python, Runner: *runner, DebianImage: *debianImage, HelmImage: *helmImage, MaxWorkspaceBytes: *maxWorkspaceMiB << 20, Hosts: hosts, Blobs: wire.NewBlobResolver(), Gates: gate.NewDefaultEvaluator(resolvedApprovalFile), }) @@ -728,26 +667,21 @@ func runApply(ctx context.Context, args []string, stdout, stderr io.Writer) erro } func runApprove(args []string, stdout, stderr io.Writer) error { - flags := flag.NewFlagSet("approve", flag.ContinueOnError) - flags.SetOutput(stderr) - workspace := flags.String("workspace", ".", "workspace root") + flags := newCommandFlags("approve", stderr).withWorkspace() plan := flags.String("plan", "snailmail.snailmail-plan.json", "reviewed plan file") output := flags.String("out", "", "approval evidence output") repository := flags.String("repository", "", "repository to approve") keyFile := flags.String("key", "", "Ed25519 approval private key file") expires := flags.Duration("expires", 30*time.Minute, "approval lifetime") yes := flags.Bool("yes", false, "confirm approval of the exact plan ID") - if err := flags.Parse(args); err != nil { + if err := flags.parse(args); err != nil { return err } - if flags.NArg() != 0 { - return fmt.Errorf("unexpected argument %q", flags.Arg(0)) - } if !*yes { return errors.New("approval requires --yes after reviewing the exact plan") } result, err := engine.ApprovePlan(engine.ApprovePlanRequest{ - Root: *workspace, Plan: *plan, Output: *output, Repository: *repository, + Root: flags.Root(), Plan: *plan, Output: *output, Repository: *repository, KeyFile: *keyFile, ExpiresIn: *expires, }) if err != nil { @@ -762,8 +696,7 @@ func runApprove(args []string, stdout, stderr io.Writer) error { } func runApprovalKey(args []string, stdout, stderr io.Writer) error { - flags := flag.NewFlagSet("approval-key", flag.ContinueOnError) - flags.SetOutput(stderr) + flags := newCommandFlags("approval-key", stderr) output := flags.String("out", "", "private key output file") if err := flags.Parse(args); err != nil { return err @@ -783,18 +716,13 @@ func runApprovalKey(args []string, stdout, stderr io.Writer) error { } func runRender(args []string, stdout, stderr io.Writer) error { - flags := flag.NewFlagSet("render", flag.ContinueOnError) - flags.SetOutput(stderr) - workspace := flags.String("workspace", ".", "workspace root") + flags := newCommandFlags("render", stderr).withWorkspace() output := flags.String("output", "site", "status site output directory") plan := flags.String("plan", "snailmail.snailmail-plan.json", "optional plan used for pending gates") - if err := flags.Parse(args); err != nil { + if err := flags.parse(args); err != nil { return err } - if flags.NArg() != 0 { - return fmt.Errorf("unexpected argument %q", flags.Arg(0)) - } - result, err := engine.RenderStatus(engine.RenderStatusRequest{Root: *workspace, Output: *output, Plan: *plan}) + result, err := engine.RenderStatus(engine.RenderStatusRequest{Root: flags.Root(), Output: *output, Plan: *plan}) if err != nil { return err } @@ -821,17 +749,13 @@ func runBuild(ctx context.Context, args []string, stdout, stderr io.Writer) erro } func runBuildPyPI(ctx context.Context, args []string, stdout, stderr io.Writer) error { - flags := flag.NewFlagSet("build pypi", flag.ContinueOnError) - flags.SetOutput(stderr) + flags := newCommandFlags("build pypi", stderr) input := flags.String("input", "", "directory containing wheels and source distributions") output := flags.String("output", "", "directory to write the static repository") generatedAtValue := flags.String("generated-at", defaultGeneratedAt, "explicit RFC3339 generation time") - if err := flags.Parse(args); err != nil { + if err := flags.parse(args); err != nil { return err } - if flags.NArg() != 0 { - return fmt.Errorf("unexpected argument %q", flags.Arg(0)) - } generatedAt, err := time.Parse(time.RFC3339, *generatedAtValue) if err != nil { return fmt.Errorf("parse --generated-at: %w", err) @@ -852,20 +776,16 @@ func runBuildPyPI(ctx context.Context, args []string, stdout, stderr io.Writer) } func runBuildDeb(ctx context.Context, args []string, stdout, stderr io.Writer) error { - flags := flag.NewFlagSet("build deb", flag.ContinueOnError) - flags.SetOutput(stderr) + flags := newCommandFlags("build deb", stderr) input := flags.String("input", "", "directory containing Debian packages") output := flags.String("output", "", "directory to write the static repository") suite := flags.String("suite", "stable", "Debian suite/codename") component := flags.String("component", "main", "Debian component") architecturesValue := flags.String("architectures", "amd64", "comma-separated target architectures") generatedAtValue := flags.String("generated-at", defaultGeneratedAt, "explicit RFC3339 generation time") - if err := flags.Parse(args); err != nil { + if err := flags.parse(args); err != nil { return err } - if flags.NArg() != 0 { - return fmt.Errorf("unexpected argument %q", flags.Arg(0)) - } generatedAt, err := time.Parse(time.RFC3339, *generatedAtValue) if err != nil { return fmt.Errorf("parse --generated-at: %w", err) @@ -889,17 +809,13 @@ func runBuildDeb(ctx context.Context, args []string, stdout, stderr io.Writer) e } func runBuildHelm(ctx context.Context, args []string, stdout, stderr io.Writer) error { - flags := flag.NewFlagSet("build helm", flag.ContinueOnError) - flags.SetOutput(stderr) + flags := newCommandFlags("build helm", stderr) input := flags.String("input", "", "directory containing packaged Helm charts") output := flags.String("output", "", "directory to write the static repository") generatedAtValue := flags.String("generated-at", defaultGeneratedAt, "explicit RFC3339 generation time") - if err := flags.Parse(args); err != nil { + if err := flags.parse(args); err != nil { return err } - if flags.NArg() != 0 { - return fmt.Errorf("unexpected argument %q", flags.Arg(0)) - } generatedAt, err := time.Parse(time.RFC3339, *generatedAtValue) if err != nil { return fmt.Errorf("parse --generated-at: %w", err) @@ -932,17 +848,13 @@ func runVerify(ctx context.Context, args []string, stdout, stderr io.Writer) err } func runVerifyPyPI(ctx context.Context, args []string, stdout, stderr io.Writer) error { - flags := flag.NewFlagSet("verify pypi", flag.ContinueOnError) - flags.SetOutput(stderr) + flags := newCommandFlags("verify pypi", stderr) repository := flags.String("repo", "", "generated repository directory") python := flags.String("python", "python3", "Python executable whose pip client verifies installs") structuralOnly := flags.Bool("structural-only", false, "verify files and indexes without invoking pip") - if err := flags.Parse(args); err != nil { + if err := flags.parse(args); err != nil { return err } - if flags.NArg() != 0 { - return fmt.Errorf("unexpected argument %q", flags.Arg(0)) - } result, err := engine.VerifyPyPI(ctx, engine.VerifyPyPIRequest{ Repository: *repository, Python: *python, @@ -963,19 +875,15 @@ func runVerifyPyPI(ctx context.Context, args []string, stdout, stderr io.Writer) } func runVerifyDeb(ctx context.Context, args []string, stdout, stderr io.Writer) error { - flags := flag.NewFlagSet("verify deb", flag.ContinueOnError) - flags.SetOutput(stderr) + flags := newCommandFlags("verify deb", stderr) repository := flags.String("repo", "", "generated repository directory") runner := flags.String("runner", "podman", "OCI runner executable") image := flags.String("image", engine.DefaultDebianVerificationImage, "digest-pinned Debian client image") maxWorkspaceMiB := flags.Int64("max-workspace-mib", 4096, "maximum temporary verification workspace in MiB") structuralOnly := flags.Bool("structural-only", false, "verify files and indexes without invoking apt") - if err := flags.Parse(args); err != nil { + if err := flags.parse(args); err != nil { return err } - if flags.NArg() != 0 { - return fmt.Errorf("unexpected argument %q", flags.Arg(0)) - } result, err := engine.VerifyDeb(ctx, engine.VerifyDebRequest{ Repository: *repository, Runner: *runner, @@ -998,18 +906,14 @@ func runVerifyDeb(ctx context.Context, args []string, stdout, stderr io.Writer) } func runVerifyHelm(ctx context.Context, args []string, stdout, stderr io.Writer) error { - flags := flag.NewFlagSet("verify helm", flag.ContinueOnError) - flags.SetOutput(stderr) + flags := newCommandFlags("verify helm", stderr) repository := flags.String("repo", "", "generated repository directory") runner := flags.String("runner", "podman", "OCI runner executable") image := flags.String("image", engine.DefaultHelmVerificationImage, "digest-pinned Helm client image") structuralOnly := flags.Bool("structural-only", false, "verify files and index without invoking Helm") - if err := flags.Parse(args); err != nil { + if err := flags.parse(args); err != nil { return err } - if flags.NArg() != 0 { - return fmt.Errorf("unexpected argument %q", flags.Arg(0)) - } result, err := engine.VerifyHelm(ctx, engine.VerifyHelmRequest{Repository: *repository, Runner: *runner, Image: *image, StructuralOnly: *structuralOnly}) if err != nil { return err @@ -1026,16 +930,12 @@ func runVerifyHelm(ctx context.Context, args []string, stdout, stderr io.Writer) } func runServe(ctx context.Context, args []string, stdout, stderr io.Writer) error { - flags := flag.NewFlagSet("serve", flag.ContinueOnError) - flags.SetOutput(stderr) + flags := newCommandFlags("serve", stderr) repository := flags.String("repo", "", "generated repository directory") listenAddress := flags.String("listen", "127.0.0.1:8080", "HTTP listen address") - if err := flags.Parse(args); err != nil { + if err := flags.parse(args); err != nil { return err } - if flags.NArg() != 0 { - return fmt.Errorf("unexpected argument %q", flags.Arg(0)) - } absolute, err := filepath.Abs(*repository) if err != nil { return fmt.Errorf("resolve repository: %w", err) diff --git a/engine/engine.go b/engine/engine.go index 6f2010f..1e42171 100644 --- a/engine/engine.go +++ b/engine/engine.go @@ -79,6 +79,9 @@ type VerifyResult struct { TreeSHA256 string FileCount int InstalledCases int + // Manifest is the verified file list, returned so a caller that has just + // verified a tree does not have to re-verify it to enumerate its contents. + Manifest buildgraph.RepositoryManifest } type RepositoryInfo struct { @@ -273,6 +276,7 @@ func VerifyPyPI(ctx context.Context, request VerifyPyPIRequest) (VerifyResult, e TreeSHA256: manifest.TreeSHA256, FileCount: len(manifest.Files), InstalledCases: installed, + Manifest: manifest, }, nil } @@ -304,6 +308,7 @@ func VerifyDeb(ctx context.Context, request VerifyDebRequest) (VerifyResult, err TreeSHA256: manifest.TreeSHA256, FileCount: len(manifest.Files), InstalledCases: installed, + Manifest: manifest, }, nil } @@ -331,6 +336,7 @@ func VerifyHelm(ctx context.Context, request VerifyHelmRequest) (VerifyResult, e TreeSHA256: manifest.TreeSHA256, FileCount: len(manifest.Files), InstalledCases: verified, + Manifest: manifest, }, nil } diff --git a/engine/gitattributes_test.go b/engine/gitattributes_test.go new file mode 100644 index 0000000..a80e0fc --- /dev/null +++ b/engine/gitattributes_test.go @@ -0,0 +1,66 @@ +package engine + +import ( + "bytes" + "context" + "os" + "os/exec" + "path/filepath" + "testing" +) + +// A repository that converts line endings stores a different blob than the +// bytes on disk. Comparing raw worktree bytes to the committed blob reported +// every authoritative file as changed, which made planning impossible in any +// such workspace; hashing by path applies the same clean filter to both sides. +func TestPlanAcceptsWorkspaceWithLineEndingConversion(t *testing.T) { + root := t.TempDir() + if output, err := exec.Command("git", "-C", root, "init").CombinedOutput(); err != nil { + t.Fatalf("git init: %v: %s", err, output) + } + if output, err := exec.Command("git", "-C", root, "config", "core.autocrlf", "true").CombinedOutput(); err != nil { + t.Fatalf("git config: %v: %s", err, output) + } + if err := os.WriteFile(filepath.Join(root, ".gitattributes"), []byte("*.toml text eol=crlf\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := InitWorkspace(InitWorkspaceRequest{Root: root, Name: "test-workspace"}); err != nil { + t.Fatal(err) + } + if err := SetupRepository(SetupRepositoryRequest{ + Root: root, Name: "pypi", Format: "pypi", Output: filepath.ToSlash(filepath.Join("public", "pypi")), + }); err != nil { + t.Fatal(err) + } + if output, err := exec.Command("git", "-C", root, "add", "--", + ".gitattributes", ".gitignore", "snailmail.toml", "repos").CombinedOutput(); err != nil { + t.Fatalf("git add: %v: %s", err, output) + } + commit := exec.Command("git", "-C", root, + "-c", "user.name=snailmail-test", "-c", "user.email=test@example.invalid", + "commit", "-m", "initialize converted workspace") + if output, err := commit.CombinedOutput(); err != nil { + t.Fatalf("git commit: %v: %s", err, output) + } + // Re-materialise the worktree so Git applies eol=crlf: the committed blob + // keeps LF while the file on disk gains CRLF, which is exactly the state + // that made raw byte comparison report a spurious difference. + for _, name := range []string{"snailmail.toml", filepath.Join("repos", "pypi.lock.toml")} { + if err := os.Remove(filepath.Join(root, name)); err != nil { + t.Fatal(err) + } + } + if output, err := exec.Command("git", "-C", root, "checkout", "--", ".").CombinedOutput(); err != nil { + t.Fatalf("git checkout: %v: %s", err, output) + } + manifest, err := os.ReadFile(filepath.Join(root, "snailmail.toml")) + if err != nil { + t.Fatal(err) + } + if !bytes.Contains(manifest, []byte("\r\n")) { + t.Fatal("worktree manifest was not converted to CRLF, so this case proves nothing") + } + if _, err := PlanWorkspace(context.Background(), PlanWorkspaceRequest{Root: root}); err != nil { + t.Fatalf("planning a line-ending-converting workspace failed: %v", err) + } +} diff --git a/engine/staging_test.go b/engine/staging_test.go new file mode 100644 index 0000000..372e839 --- /dev/null +++ b/engine/staging_test.go @@ -0,0 +1,64 @@ +package engine + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" +) + +// Build and stage trees must not land in TMPDIR: it is commonly a tmpfs, so a +// large repository would be held in RAM, and it is a different filesystem from +// the local CAS, so linking artifacts into a build input degrades to copying +// every byte. +func TestPlanStagesInsideWorkspaceRatherThanTempDir(t *testing.T) { + root := t.TempDir() + isolatedTemp := t.TempDir() + t.Setenv("TMPDIR", isolatedTemp) + + initializeRepository(t, root, "pypi") + artifact := workspaceArtifact(t, root, "pypi", "1.2.3") + if _, err := AddArtifacts(AddArtifactsRequest{Root: root, Repository: "pypi", Artifacts: []string{artifact}}); err != nil { + t.Fatal(err) + } + commitWorkspace(t, root, "initialize workspace") + if _, err := PlanWorkspace(context.Background(), PlanWorkspaceRequest{Root: root}); err != nil { + t.Fatal(err) + } + + entries, err := os.ReadDir(isolatedTemp) + if err != nil { + t.Fatal(err) + } + for _, entry := range entries { + if strings.Contains(entry.Name(), "snailmail") { + t.Fatalf("planning left %q in TMPDIR", entry.Name()) + } + } + if _, err := os.Lstat(filepath.Join(root, ".snailmail", "stage")); err != nil { + t.Fatalf("workspace staging directory was not created: %v", err) + } +} + +func TestStagingRootRejectsRelativeOverride(t *testing.T) { + t.Setenv(StageDirectoryEnvironment, "relative/stage") + if _, err := stagingRoot(t.TempDir()); err == nil { + t.Fatal("expected a relative staging override to be rejected") + } +} + +func TestStagingRootHonorsAbsoluteOverride(t *testing.T) { + override := filepath.Join(t.TempDir(), "elsewhere") + t.Setenv(StageDirectoryEnvironment, override) + directory, err := stagingRoot(t.TempDir()) + if err != nil { + t.Fatal(err) + } + if directory != override { + t.Fatalf("staging root = %q, want %q", directory, override) + } + if _, err := os.Lstat(override); err != nil { + t.Fatalf("override directory was not created: %v", err) + } +} diff --git a/engine/workspace.go b/engine/workspace.go index 0f3d6c6..d449c16 100644 --- a/engine/workspace.go +++ b/engine/workspace.go @@ -617,6 +617,41 @@ func PlanWorkspace(ctx context.Context, request PlanWorkspaceRequest) (PlanWorks return PlanWorkspaceResult{PlanID: plan.PlanID, Output: output, Changes: changes, Acquisitions: plannedAcquisitions}, nil } +// applyRepository is one repository's state as apply carries it from plan +// revalidation through staging to publication. +type applyRepository struct { + planned state.PlanRepository + repository state.Repository + lock state.RepositoryLock + host host.Host + hostRepository host.Repository + observed host.PublishedRevision + hostStage host.StagedPublication + stageRoot string + stage string + stagedManifest buildgraph.RepositoryManifest + current bool + deployment state.DeploymentRecord + signingState deploymentSigningState +} + +// restoreFailedPublication compensates a failed verification by asking the host +// to put back the revision this change displaced. Restoring is itself a +// publication effect, so it is gated exactly like the change that failed. +func restoreFailedPublication(ctx context.Context, item applyRepository, reference host.RestoreRef, expected host.ExpectedRevision, authorize func(applyRepository) error) error { + if err := authorize(item); err != nil { + return fmt.Errorf("restore gate failed: %w", err) + } + // The restore must still run when the caller's context is already done, + // otherwise an interrupt would leave the failed tree published. + restoreCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 2*time.Minute) + defer cancel() + if _, err := item.host.Restore(restoreCtx, item.hostRepository, reference, expected); err != nil { + return fmt.Errorf("restore failed: %w", err) + } + return nil +} + func ApplyWorkspace(ctx context.Context, request ApplyWorkspaceRequest) (ApplyWorkspaceResult, error) { root, err := workspaceRoot(request.Root) if err != nil { @@ -695,205 +730,29 @@ func ApplyWorkspace(ctx context.Context, request ApplyWorkspaceRequest) (ApplyWo if err != nil { return ApplyWorkspaceResult{}, err } - type applyRepository struct { - planned state.PlanRepository - repository state.Repository - lock state.RepositoryLock - host host.Host - hostRepository host.Repository - observed host.PublishedRevision - hostStage host.StagedPublication - stageRoot string - stage string - current bool - deployment state.DeploymentRecord - signingState deploymentSigningState - } var prepared []applyRepository hosts := request.Hosts if hosts == nil { hosts = localHostResolver{} } seenRepositories := make(map[string]bool) + preparation := &applyPreparation{ + ctx: ctx, root: root, request: request, plan: plan, manifest: manifest, hosts: hosts, + blobStore: blobStore, now: now, expiresAt: expiresAt, generatedAt: generatedAt, + signatureTime: signatureTime, ledgerCommitted: ledgerCommitted, + } for _, planned := range plan.Payload.Repositories { if seenRepositories[planned.Name] { return ApplyWorkspaceResult{}, fmt.Errorf("plan contains duplicate repository %q", planned.Name) } seenRepositories[planned.Name] = true - repository, exists := manifest.Repositories[planned.Name] - if !exists || repository.Format != planned.Format || repository.Gate != planned.Gate || !reflect.DeepEqual(repository.ApprovalKeys, planned.ApprovalKeys) || len(planned.Signing) > 1 || (len(planned.Signing) == 0) != (len(repository.SigningKeys) == 0) { - return ApplyWorkspaceResult{}, fmt.Errorf("stale plan: repository %q configuration changed", planned.Name) - } - activeSigningKey, _, _, _, signingStateErr := repositorySigningState(repository) - if signingStateErr != nil { - return ApplyWorkspaceResult{}, signingStateErr - } - for index, signing := range planned.Signing { - if index != 0 || signing.KeyName != activeSigningKey { - return ApplyWorkspaceResult{}, fmt.Errorf("stale plan: repository %q signing key changed", planned.Name) - } - key, exists := manifest.Keys[signing.KeyName] - if !exists { - return ApplyWorkspaceResult{}, fmt.Errorf("stale plan: repository %q signing key is missing", planned.Name) - } - keyExpiresAt, err := time.Parse(time.RFC3339, key.ExpiresAt) - if err != nil || expiresAt.After(keyExpiresAt) { - return ApplyWorkspaceResult{}, fmt.Errorf("repository %q plan expires after its signing key", planned.Name) - } - if err := validateSigningRecipeMetadata(signing, repository.Suite); err != nil { - return ApplyWorkspaceResult{}, fmt.Errorf("plan repository %q: %w", planned.Name, err) - } - } - hostIdentity, err := repositoryHostIdentity(repository) - if err != nil || hostIdentity != planned.HostIdentitySHA256 || repository.Host != planned.Host || repository.Visibility != planned.Visibility { - return ApplyWorkspaceResult{}, fmt.Errorf("stale plan: repository %q host changed", planned.Name) - } - hostRepository := toHostRepository(root, manifest.Workspace.ID, hostIdentity, planned.Name, repository) - if hostRepository.CanonicalEndpoint != planned.CanonicalEndpoint { - return ApplyWorkspaceResult{}, fmt.Errorf("stale plan: repository %q canonical endpoint changed", planned.Name) - } - installDocDigest, err := repositoryInstallDocDigest(root, planned.Name, repository) - if err != nil || installDocDigest != planned.InstallDocSHA256 { - return ApplyWorkspaceResult{}, fmt.Errorf("stale plan: repository %q install document changed", planned.Name) - } - selectedHost, err := hosts.Resolve(ctx, hostRepository) - if err != nil { - return ApplyWorkspaceResult{}, err - } - capabilities, err := selectedHost.Capabilities(ctx, hostRepository) - if err != nil { - return ApplyWorkspaceResult{}, err - } - if capabilities.FaithfulPreview != planned.FaithfulPreview || capabilities.ConditionalCommit != planned.ConditionalCommit || capabilities.ConditionalRestore != planned.ConditionalRestore || - capabilities.PrivateRead != planned.PrivateRead || capabilities.CredentialBrokerIdentity != planned.CredentialBrokerIdentity { - return ApplyWorkspaceResult{}, fmt.Errorf("stale plan: repository %q host capabilities changed", planned.Name) - } - lockPath, err := state.WorkspacePath(root, repository.Lock) - if err != nil { - return ApplyWorkspaceResult{}, err - } - lockDigest, err := state.HashFile(lockPath) - if err != nil || lockDigest != planned.LockSHA256 { - return ApplyWorkspaceResult{}, fmt.Errorf("stale plan: repository %q lock changed", planned.Name) - } - lock, err := state.LoadLock(root, repository) - if err != nil { - return ApplyWorkspaceResult{}, err - } - if err := state.ValidateLock(lock, planned.Name, repository.Format); err != nil { - return ApplyWorkspaceResult{}, err - } - ledger, err := state.LoadLedgerHistory(root, planned.Name) - if err != nil { - return ApplyWorkspaceResult{}, err - } - if err := state.ValidatePublicationHistory(planned.Name, ledger); err != nil { - return ApplyWorkspaceResult{}, err - } - if err := state.ValidatePublishedBindings(lock, ledger); err != nil { - return ApplyWorkspaceResult{}, err - } - missingBindings := missingPublicationBindings(lock, repository, ledger) - expectedBindings := []state.PlanPublicationBinding(nil) - if planned.PublicationRecords { - expectedBindings = publicationBindingsForVersions(visiblePackageVersions(lock, repository)) - } - expectedAcquisitions := planAcquisitionsForVersions(visiblePackageVersions(lock, repository)) - if !reflect.DeepEqual(planned.Acquisitions, expectedAcquisitions) { - return ApplyWorkspaceResult{}, fmt.Errorf("plan repository %q has inconsistent adopted acquisitions", planned.Name) - } - if planned.PublicationRecords != (len(planned.PublicationBindings) != 0) || - !reflect.DeepEqual(planned.PublicationBindings, expectedBindings) || - (!ledgerCommitted && planned.PublicationRecords != (len(missingBindings) != 0)) || - (ledgerCommitted && !planned.PublicationRecords && len(missingBindings) != 0) { - return ApplyWorkspaceResult{}, fmt.Errorf("plan repository %q has inconsistent publication-record effects", planned.Name) - } - deployment, err := state.LoadDeployment(root, planned.Name) - if err != nil { - return ApplyWorkspaceResult{}, err - } - desiredSigningState, err := repositoryDeploymentSigningState(repository, manifest.Keys) - if err != nil { - return ApplyWorkspaceResult{}, err - } - observed, err := selectedHost.Observe(ctx, hostRepository) - if err != nil { - return ApplyWorkspaceResult{}, err - } - plannedObserved := publishedFromPlanObservation(planned) - trustNotBefore := time.Time{} - if repository.SigningRotation != nil || deployment.SigningRotationPhase != "" { - trustNotBefore, err = state.AuthoritativeDeploymentTrustSince(root, planned.Name, deployment) - if err != nil { - return ApplyWorkspaceResult{}, err - } - } - if err := validateRepositorySigningTransition(repository, manifest.Keys, deployment, plannedObserved, now, trustNotBefore); err != nil { - return ApplyWorkspaceResult{}, fmt.Errorf("repository %q: %w", planned.Name, err) - } - matchesObserved := revisionMatchesPlanObservation(observed, planned) - managedRemote := repository.Host.Type == "s3" || repository.Host.Type == "github-pages" - matchesApplied := planned.Action != "noop" && observed.TreeSHA256 == planned.DesiredTreeSHA256 && - (!managedRemote || (observed.PlanID == plan.PlanID && observed.ChangeID == planned.ChangeID && observed.ManifestSHA256 == planned.DesiredManifestSHA256)) - deploymentApplied := deployment.PlanID == plan.PlanID && deployment.ChangeID == planned.ChangeID && deployment.TreeSHA256 == planned.DesiredTreeSHA256 && deployment.ManifestSHA256 == planned.DesiredManifestSHA256 && deployment.NativeRevision == observed.NativeRevision && deploymentSigningMatches(deployment, desiredSigningState) - deploymentCurrent := deploymentMatchesDesired(deployment, observed, planned.DesiredTreeSHA256, planned.DesiredManifestSHA256, desiredSigningState) - if !reflect.DeepEqual(deployment, planned.ObservedDeployment) && !deploymentApplied { - return ApplyWorkspaceResult{}, fmt.Errorf("stale plan: repository %q deployment receipt changed", planned.Name) - } - if !matchesObserved && !matchesApplied { - if observed.TreeSHA256 == planned.ObservedTreeSHA256 { - return ApplyWorkspaceResult{}, fmt.Errorf("stale plan: repository %q native revision changed", planned.Name) - } - if observed.TreeSHA256 == planned.DesiredTreeSHA256 { - return ApplyWorkspaceResult{}, fmt.Errorf("stale plan: repository %q desired tree was published by another change", planned.Name) - } - return ApplyWorkspaceResult{}, fmt.Errorf("stale plan: repository %q target changed", planned.Name) - } - expectedAction := "noop" - if planned.ObservedTreeSHA256 != planned.DesiredTreeSHA256 || (managedRemote && planned.ObservedManifestSHA256 != planned.DesiredManifestSHA256) || !publicationBindingsComplete(lock, repository, ledger) || !deploymentMatchesDesired(planned.ObservedDeployment, plannedObserved, planned.DesiredTreeSHA256, planned.DesiredManifestSHA256, desiredSigningState) { - expectedAction = "update" - if planned.ObservedRevision == "" { - expectedAction = "create" - } - } - if planned.Action != expectedAction || planned.ChangeID != planned.Name+":"+planned.DesiredTreeSHA256[:12] { - return ApplyWorkspaceResult{}, fmt.Errorf("plan repository %q has inconsistent action metadata", planned.Name) - } - receiptRecovery := matchesApplied && reflect.DeepEqual(deployment, planned.ObservedDeployment) - current := observed.TreeSHA256 == planned.DesiredTreeSHA256 && (!managedRemote || observed.ManifestSHA256 == planned.DesiredManifestSHA256) && (deploymentApplied || (planned.Action == "noop" && deploymentCurrent) || receiptRecovery) - item := applyRepository{ - planned: planned, repository: repository, lock: lock, host: selectedHost, - hostRepository: hostRepository, observed: observed, current: current, deployment: deployment, signingState: desiredSigningState, - } - if item.current && (request.StructuralOnly || planned.Action == "noop") && !(planned.Action != "noop" && len(planned.Signing) != 0) { - prepared = append(prepared, item) - continue - } - stage, err := os.MkdirTemp("", ".snailmail-apply-*") + item, err := preparation.prepareRepository(planned) if err != nil { - return ApplyWorkspaceResult{}, err - } - stageOutput := filepath.Join(stage, "repository") - staged, buildErr := buildLockedRepository(ctx, root, planned.Name, repository, lock, generatedAt, signatureTime, stageOutput, blobStore, manifest.Keys, planned.Signing, nil) - if buildErr == nil && (staged.TreeSHA256 != planned.DesiredTreeSHA256 || staged.ManifestSHA256 != planned.DesiredManifestSHA256) { - buildErr = errors.New("stale plan: rebuilt tree digest changed") - } - if buildErr == nil && !signingContentsEqual(staged.Signing, planned.Signing) { - buildErr = errors.New("stale plan: signing recipe changed") - } - if buildErr == nil { - structuralRequest := request - structuralRequest.StructuralOnly = true - buildErr = verifyStaged(ctx, repository.Format, stageOutput, structuralRequest) - } - if buildErr != nil { - _ = os.RemoveAll(stage) for _, previous := range prepared { _ = os.RemoveAll(previous.stageRoot) } - return ApplyWorkspaceResult{}, buildErr + return ApplyWorkspaceResult{}, err } - item.stageRoot, item.stage = stage, stageOutput prepared = append(prepared, item) } defer func() { @@ -943,7 +802,7 @@ func ApplyWorkspace(ctx context.Context, request ApplyWorkspaceRequest) (ApplyWo if item.current { continue } - files, err := stagedHostFiles(item.stage) + files, err := stagedHostFiles(item.stage, item.stagedManifest) if err != nil { return ApplyWorkspaceResult{}, err } @@ -962,7 +821,7 @@ func ApplyWorkspace(ctx context.Context, request ApplyWorkspaceRequest) (ApplyWo continue } if item.hostRepository.Type == "local" { - if err := verifyStaged(ctx, item.repository.Format, item.stage, request); err != nil { + if _, err := verifyStaged(ctx, item.repository.Format, item.stage, request); err != nil { return ApplyWorkspaceResult{}, err } continue @@ -1077,100 +936,22 @@ func ApplyWorkspace(ctx context.Context, request ApplyWorkspaceRequest) (ApplyWo } result := ApplyWorkspaceResult{PlanID: plan.PlanID} deployments := make([]state.DeploymentRecord, 0, len(prepared)) + execution := &applyExecution{ + ctx: ctx, root: root, request: request, plan: plan, + applyGitRevision: applyGitRevision, authorize: authorize, result: result, + } for _, item := range prepared { + var err error if item.current { - result.Current++ - if !request.StructuralOnly && item.planned.Action != "noop" { - access, accessErr := item.host.ReadAccess(ctx, item.hostRepository, item.observed) - if accessErr != nil { - return result, accessErr - } - if err := verifyCanonicalClient(ctx, root, item.repository, item.stage, access, request); err != nil { - if item.observed.RestoreID != "" { - if gateErr := authorize(item); gateErr != nil { - return result, gateErr - } - restoreCtx, cancelRestore := context.WithTimeout(context.WithoutCancel(ctx), 2*time.Minute) - _, restoreErr := item.host.Restore(restoreCtx, item.hostRepository, host.RestoreRef{ - ID: item.observed.RestoreID, PlanID: item.observed.PlanID, - ChangeID: item.observed.ChangeID, FailedTree: item.observed.TreeSHA256, - DescriptorSHA256: item.observed.RestoreSHA256, RootSHA256: item.observed.RestoreRootSHA256, - }, expectedRevisionFromPublished(item.observed)) - cancelRestore() - if restoreErr != nil { - return result, fmt.Errorf("canonical retry probe failed: %v; restore failed: %w", err, restoreErr) - } - result.Current-- - } - return result, err - } - } - if item.planned.Action != "noop" { - deployments = append(deployments, deploymentRecordFor(item.planned, item.deployment, item.observed, item.signingState, item.observed.NativeRevision, plan.PlanID, plan.Payload.CreatedAt, request.currentTime())) - } - continue - } - if err := state.AssertGitRevision(root, applyGitRevision); err != nil { - return result, err - } - if err := authorize(item); err != nil { - return result, err + err = execution.verifyCurrent(item) + } else { + err = execution.publish(item) } - committed, err := item.host.Commit(ctx, item.hostRepository, item.hostStage, expectedRevisionFromPlan(item.planned)) if err != nil { - return result, err - } - if committed.Access.Credential != nil { - defer committed.Access.Credential.Destroy() - } - result.Applied++ - canonical, observeErr := item.host.Observe(ctx, item.hostRepository) - if observeErr != nil || canonical.TreeSHA256 != item.planned.DesiredTreeSHA256 || canonical.NativeRevision != committed.Revision.NativeRevision || - (item.hostRepository.Type == "s3" && canonical != committed.Revision) { - probeErr := observeErr - if probeErr == nil { - probeErr = errors.New("canonical host observation does not match committed tree") - } - if committed.RestoreRef != nil { - if gateErr := authorize(item); gateErr != nil { - return result, fmt.Errorf("canonical probe failed: %v; restore gate failed: %w", probeErr, gateErr) - } - restoreCtx, cancelRestore := context.WithTimeout(context.WithoutCancel(ctx), 2*time.Minute) - _, restoreErr := item.host.Restore(restoreCtx, item.hostRepository, *committed.RestoreRef, expectedRevisionFromPublished(committed.Revision)) - cancelRestore() - if restoreErr != nil { - return result, fmt.Errorf("canonical probe failed: %v; restore failed: %w", probeErr, restoreErr) - } - result.Applied-- - } - return result, fmt.Errorf("canonical probe failed: %w", probeErr) + return execution.result, err } - if !request.StructuralOnly { - access := committed.Access - if access.Endpoint == "" { - access.Endpoint = committed.CanonicalEndpoint - } - if probeErr := verifyCanonicalClient(ctx, root, item.repository, item.stage, access, request); probeErr != nil { - if committed.RestoreRef != nil { - if gateErr := authorize(item); gateErr != nil { - return result, fmt.Errorf("canonical client probe failed: %v; restore gate failed: %w", probeErr, gateErr) - } - restoreCtx, cancelRestore := context.WithTimeout(context.WithoutCancel(ctx), 2*time.Minute) - _, restoreErr := item.host.Restore(restoreCtx, item.hostRepository, *committed.RestoreRef, expectedRevisionFromPublished(committed.Revision)) - cancelRestore() - if restoreErr != nil { - return result, fmt.Errorf("canonical client probe failed: %v; restore failed: %w", probeErr, restoreErr) - } - result.Applied-- - } - return result, fmt.Errorf("canonical client probe failed: %w", probeErr) - } - } - if err := state.AssertGitRevision(root, applyGitRevision); err != nil { - return result, err - } - deployments = append(deployments, deploymentRecordFor(item.planned, item.deployment, item.observed, item.signingState, committed.Revision.NativeRevision, plan.PlanID, plan.Payload.CreatedAt, request.currentTime())) } + result, deployments = execution.result, execution.deployments if unlockGit != nil { unlockGit() unlockGit = nil @@ -1435,7 +1216,8 @@ func verifyCanonicalClient(ctx context.Context, root string, repository state.Re if err != nil { return err } - return verifyStaged(ctx, repository.Format, output, request) + _, err = verifyStaged(ctx, repository.Format, output, request) + return err } if repository.Format != "pypi" { return fmt.Errorf("canonical client verification is not implemented for format %q", repository.Format) @@ -1445,7 +1227,11 @@ func verifyCanonicalClient(ctx context.Context, root string, repository state.Re } func buildLockedRepository(ctx context.Context, root, name string, repository state.Repository, lock state.RepositoryLock, generatedAt, signatureTime time.Time, output string, blobStore blob.Store, keys map[string]state.SigningKey, plannedSigning []state.PlanSigning, signers signer.Resolver) (BuildResult, error) { - input, err := os.MkdirTemp("", ".snailmail-locked-input-*") + staging, err := stagingRoot(root) + if err != nil { + return BuildResult{}, err + } + input, err := os.MkdirTemp(staging, ".snailmail-locked-input-*") if err != nil { return BuildResult{}, err } @@ -1473,7 +1259,7 @@ func buildLockedRepository(ctx context.Context, root, name string, repository st } } if output == "" { - temporary, err := os.MkdirTemp("", ".snailmail-plan-build-*") + temporary, err := os.MkdirTemp(staging, ".snailmail-plan-build-*") if err != nil { return BuildResult{}, err } @@ -1554,25 +1340,6 @@ func materializeLockedArtifact(ctx context.Context, output string, generatedAt t return BuildResult{Format: manifest.Format, Output: output, TreeSHA256: manifest.TreeSHA256, ManifestSHA256: manifestSHA256}, nil } -func activePackageVersions(lock state.RepositoryLock) []state.PackageVersion { - active := make(map[string]bool) - for _, placement := range lock.Placement { - active[placement.Package+"\x00"+placement.Version] = true - } - var result []state.PackageVersion - for _, packageVersion := range lock.PackageVersion { - if active[packageVersion.Package+"\x00"+packageVersion.Version] { - result = append(result, packageVersion) - } - } - return result -} - -func activeLock(lock state.RepositoryLock) state.RepositoryLock { - lock.PackageVersion = activePackageVersions(lock) - return lock -} - func visiblePackageVersions(lock state.RepositoryLock, repository state.Repository) []state.PackageVersion { active := make(map[string]bool) for _, placement := range lock.Placement { @@ -1668,19 +1435,19 @@ func planAcquisitionsForVersions(versions []state.PackageVersion) []state.PlanAc return acquisitions } -func verifyStaged(ctx context.Context, format, repository string, request ApplyWorkspaceRequest) error { +func verifyStaged(ctx context.Context, format, repository string, request ApplyWorkspaceRequest) (buildgraph.RepositoryManifest, error) { switch format { case "pypi": - _, err := VerifyPyPI(ctx, VerifyPyPIRequest{Repository: repository, Python: request.Python, StructuralOnly: request.StructuralOnly}) - return err + result, err := VerifyPyPI(ctx, VerifyPyPIRequest{Repository: repository, Python: request.Python, StructuralOnly: request.StructuralOnly}) + return result.Manifest, err case "deb": - _, err := VerifyDeb(ctx, VerifyDebRequest{Repository: repository, Runner: request.Runner, Image: request.DebianImage, MaxWorkspaceBytes: request.MaxWorkspaceBytes, StructuralOnly: request.StructuralOnly}) - return err + result, err := VerifyDeb(ctx, VerifyDebRequest{Repository: repository, Runner: request.Runner, Image: request.DebianImage, MaxWorkspaceBytes: request.MaxWorkspaceBytes, StructuralOnly: request.StructuralOnly}) + return result.Manifest, err case "helm": - _, err := VerifyHelm(ctx, VerifyHelmRequest{Repository: repository, Runner: request.Runner, Image: request.HelmImage, StructuralOnly: request.StructuralOnly}) - return err + result, err := VerifyHelm(ctx, VerifyHelmRequest{Repository: repository, Runner: request.Runner, Image: request.HelmImage, StructuralOnly: request.StructuralOnly}) + return result.Manifest, err default: - return fmt.Errorf("unsupported repository format %q", format) + return buildgraph.RepositoryManifest{}, fmt.Errorf("unsupported repository format %q", format) } } @@ -1709,6 +1476,30 @@ func linkOrCopy(source, target string) error { return closeInputErr } +// StageDirectoryEnvironment overrides where build and stage trees are created. +const StageDirectoryEnvironment = "SNAILMAIL_STAGE_DIR" + +// stagingRoot returns the directory that holds temporary build and stage trees. +// +// These default to the workspace rather than TMPDIR for two reasons. TMPDIR is +// commonly a tmpfs, so a multi-gigabyte repository tree would be held in RAM, +// and it is a different filesystem from the local CAS, so linking artifacts +// into a build input falls back to copying every byte. Staging beside the CAS +// keeps the links and the bytes on disk. +func stagingRoot(root string) (string, error) { + directory := filepath.Join(root, ".snailmail", "stage") + if override := strings.TrimSpace(os.Getenv(StageDirectoryEnvironment)); override != "" { + if !filepath.IsAbs(override) { + return "", fmt.Errorf("%s must be an absolute path", StageDirectoryEnvironment) + } + directory = override + } + if err := os.MkdirAll(directory, 0o700); err != nil { + return "", fmt.Errorf("create staging directory: %w", err) + } + return directory, nil +} + func workspaceRoot(root string) (string, error) { if root == "" { root = "." @@ -1791,11 +1582,10 @@ func repositoryInstallDocDigest(root, name string, repository state.Repository) return state.HashFile(filename) } -func stagedHostFiles(directory string) ([]host.File, error) { - manifest, err := app.VerifyRepository(directory) - if err != nil { - return nil, err - } +// stagedHostFiles enumerates a stage that this apply has already verified in +// full. The host verifies the directory again when it stages it, so the file +// list is a convenience here rather than the integrity boundary. +func stagedHostFiles(directory string, manifest buildgraph.RepositoryManifest) ([]host.File, error) { files := make([]host.File, 0, len(manifest.Files)+1) for _, file := range manifest.Files { files = append(files, host.File{Path: file.Path, Size: file.Size, SHA256: file.SHA256}) @@ -2087,3 +1877,320 @@ func PlanSummary(plan state.Plan) string { } return strings.Join(lines, "\n") } + +// applyExecution carries the mutable outcome of a publication pass so that each +// repository can be handled by its own function rather than by another few +// hundred lines inside ApplyWorkspace. +type applyExecution struct { + ctx context.Context + root string + request ApplyWorkspaceRequest + plan state.Plan + applyGitRevision string + authorize func(applyRepository) error + + result ApplyWorkspaceResult + deployments []state.DeploymentRecord +} + +// verifyCurrent handles a repository the host already serves at the desired +// tree. Nothing is published, but a client probe still has to pass before the +// deployment receipt can claim the tree is being served. +func (execution *applyExecution) verifyCurrent(item applyRepository) error { + ctx, request := execution.ctx, execution.request + execution.result.Current++ + if !request.StructuralOnly && item.planned.Action != "noop" { + access, accessErr := item.host.ReadAccess(ctx, item.hostRepository, item.observed) + if accessErr != nil { + return accessErr + } + if err := verifyCanonicalClient(ctx, execution.root, item.repository, item.stage, access, request); err != nil { + if item.observed.RestoreID != "" { + reference := host.RestoreRef{ + ID: item.observed.RestoreID, PlanID: item.observed.PlanID, + ChangeID: item.observed.ChangeID, FailedTree: item.observed.TreeSHA256, + DescriptorSHA256: item.observed.RestoreSHA256, RootSHA256: item.observed.RestoreRootSHA256, + } + if restoreErr := restoreFailedPublication(ctx, item, reference, expectedRevisionFromPublished(item.observed), execution.authorize); restoreErr != nil { + return fmt.Errorf("canonical retry probe failed: %v; %w", err, restoreErr) + } + execution.result.Current-- + } + return err + } + } + if item.planned.Action != "noop" { + execution.recordDeployment(item, item.observed.NativeRevision) + } + return nil +} + +// publish switches one repository to its staged tree, proves the host serves +// exactly that tree, and restores the displaced revision if it does not. +func (execution *applyExecution) publish(item applyRepository) error { + ctx, request := execution.ctx, execution.request + // The publication locks are held across the whole pass, so Git cannot move + // the branch under it; confirming HEAD is enough and keeps the cost of a + // publication independent of how many repositories it covers. + if err := state.AssertGitHeadRevision(execution.root, execution.applyGitRevision); err != nil { + return err + } + if err := execution.authorize(item); err != nil { + return err + } + committed, err := item.host.Commit(ctx, item.hostRepository, item.hostStage, expectedRevisionFromPlan(item.planned)) + if err != nil { + return err + } + // Scoped to this repository, so a multi-repository apply does not keep every + // short-lived credential alive until the whole apply finishes. + if committed.Access.Credential != nil { + defer committed.Access.Credential.Destroy() + } + execution.result.Applied++ + canonical, observeErr := item.host.Observe(ctx, item.hostRepository) + if observeErr != nil || canonical.TreeSHA256 != item.planned.DesiredTreeSHA256 || canonical.NativeRevision != committed.Revision.NativeRevision || + (item.hostRepository.Type == "s3" && canonical != committed.Revision) { + probeErr := observeErr + if probeErr == nil { + probeErr = errors.New("canonical host observation does not match committed tree") + } + return execution.restore(item, committed, probeErr, "canonical probe failed") + } + if !request.StructuralOnly { + access := committed.Access + if access.Endpoint == "" { + access.Endpoint = committed.CanonicalEndpoint + } + if probeErr := verifyCanonicalClient(ctx, execution.root, item.repository, item.stage, access, request); probeErr != nil { + return execution.restore(item, committed, probeErr, "canonical client probe failed") + } + } + if err := state.AssertGitHeadRevision(execution.root, execution.applyGitRevision); err != nil { + return err + } + execution.recordDeployment(item, committed.Revision.NativeRevision) + return nil +} + +// restore compensates a failed probe and always reports the original failure, +// with the restore outcome attached. +func (execution *applyExecution) restore(item applyRepository, committed host.CommitResult, probeErr error, label string) error { + if committed.RestoreRef == nil { + return fmt.Errorf("%s: %w", label, probeErr) + } + if restoreErr := restoreFailedPublication(execution.ctx, item, *committed.RestoreRef, expectedRevisionFromPublished(committed.Revision), execution.authorize); restoreErr != nil { + return fmt.Errorf("%s: %v; %w", label, probeErr, restoreErr) + } + execution.result.Applied-- + return fmt.Errorf("%s: %w", label, probeErr) +} + +func (execution *applyExecution) recordDeployment(item applyRepository, nativeRevision string) { + execution.deployments = append(execution.deployments, deploymentRecordFor( + item.planned, item.deployment, item.observed, item.signingState, nativeRevision, + execution.plan.PlanID, execution.plan.Payload.CreatedAt, execution.request.currentTime(), + )) +} + +// applyPreparation revalidates the reviewed plan against the current workspace +// and builds each repository's stage. Nothing here has a publication effect: +// every failure leaves the hosts untouched. +type applyPreparation struct { + ctx context.Context + root string + request ApplyWorkspaceRequest + plan state.Plan + manifest state.Manifest + hosts host.Resolver + blobStore blob.Store + now time.Time + expiresAt time.Time + generatedAt time.Time + signatureTime time.Time + ledgerCommitted bool +} + +// prepareRepository checks one planned repository against the workspace it was +// planned from and, unless the host already serves the desired tree, rebuilds +// and verifies its stage. The caller owns cleanup of stages already built. +func (preparation *applyPreparation) prepareRepository(planned state.PlanRepository) (applyRepository, error) { + repository, exists := preparation.manifest.Repositories[planned.Name] + if !exists || repository.Format != planned.Format || repository.Gate != planned.Gate || !reflect.DeepEqual(repository.ApprovalKeys, planned.ApprovalKeys) || len(planned.Signing) > 1 || (len(planned.Signing) == 0) != (len(repository.SigningKeys) == 0) { + return applyRepository{}, fmt.Errorf("stale plan: repository %q configuration changed", planned.Name) + } + activeSigningKey, _, _, _, signingStateErr := repositorySigningState(repository) + if signingStateErr != nil { + return applyRepository{}, signingStateErr + } + for index, signing := range planned.Signing { + if index != 0 || signing.KeyName != activeSigningKey { + return applyRepository{}, fmt.Errorf("stale plan: repository %q signing key changed", planned.Name) + } + key, exists := preparation.manifest.Keys[signing.KeyName] + if !exists { + return applyRepository{}, fmt.Errorf("stale plan: repository %q signing key is missing", planned.Name) + } + keyExpiresAt, err := time.Parse(time.RFC3339, key.ExpiresAt) + if err != nil || preparation.expiresAt.After(keyExpiresAt) { + return applyRepository{}, fmt.Errorf("repository %q plan expires after its signing key", planned.Name) + } + if err := validateSigningRecipeMetadata(signing, repository.Suite); err != nil { + return applyRepository{}, fmt.Errorf("plan repository %q: %w", planned.Name, err) + } + } + hostIdentity, err := repositoryHostIdentity(repository) + if err != nil || hostIdentity != planned.HostIdentitySHA256 || repository.Host != planned.Host || repository.Visibility != planned.Visibility { + return applyRepository{}, fmt.Errorf("stale plan: repository %q host changed", planned.Name) + } + hostRepository := toHostRepository(preparation.root, preparation.manifest.Workspace.ID, hostIdentity, planned.Name, repository) + if hostRepository.CanonicalEndpoint != planned.CanonicalEndpoint { + return applyRepository{}, fmt.Errorf("stale plan: repository %q canonical endpoint changed", planned.Name) + } + installDocDigest, err := repositoryInstallDocDigest(preparation.root, planned.Name, repository) + if err != nil || installDocDigest != planned.InstallDocSHA256 { + return applyRepository{}, fmt.Errorf("stale plan: repository %q install document changed", planned.Name) + } + selectedHost, err := preparation.hosts.Resolve(preparation.ctx, hostRepository) + if err != nil { + return applyRepository{}, err + } + capabilities, err := selectedHost.Capabilities(preparation.ctx, hostRepository) + if err != nil { + return applyRepository{}, err + } + if capabilities.FaithfulPreview != planned.FaithfulPreview || capabilities.ConditionalCommit != planned.ConditionalCommit || capabilities.ConditionalRestore != planned.ConditionalRestore || + capabilities.PrivateRead != planned.PrivateRead || capabilities.CredentialBrokerIdentity != planned.CredentialBrokerIdentity { + return applyRepository{}, fmt.Errorf("stale plan: repository %q host capabilities changed", planned.Name) + } + lockPath, err := state.WorkspacePath(preparation.root, repository.Lock) + if err != nil { + return applyRepository{}, err + } + lockDigest, err := state.HashFile(lockPath) + if err != nil || lockDigest != planned.LockSHA256 { + return applyRepository{}, fmt.Errorf("stale plan: repository %q lock changed", planned.Name) + } + lock, err := state.LoadLock(preparation.root, repository) + if err != nil { + return applyRepository{}, err + } + if err := state.ValidateLock(lock, planned.Name, repository.Format); err != nil { + return applyRepository{}, err + } + ledger, err := state.LoadLedgerHistory(preparation.root, planned.Name) + if err != nil { + return applyRepository{}, err + } + if err := state.ValidatePublicationHistory(planned.Name, ledger); err != nil { + return applyRepository{}, err + } + if err := state.ValidatePublishedBindings(lock, ledger); err != nil { + return applyRepository{}, err + } + missingBindings := missingPublicationBindings(lock, repository, ledger) + expectedBindings := []state.PlanPublicationBinding(nil) + if planned.PublicationRecords { + expectedBindings = publicationBindingsForVersions(visiblePackageVersions(lock, repository)) + } + expectedAcquisitions := planAcquisitionsForVersions(visiblePackageVersions(lock, repository)) + if !reflect.DeepEqual(planned.Acquisitions, expectedAcquisitions) { + return applyRepository{}, fmt.Errorf("plan repository %q has inconsistent adopted acquisitions", planned.Name) + } + if planned.PublicationRecords != (len(planned.PublicationBindings) != 0) || + !reflect.DeepEqual(planned.PublicationBindings, expectedBindings) || + (!preparation.ledgerCommitted && planned.PublicationRecords != (len(missingBindings) != 0)) || + (preparation.ledgerCommitted && !planned.PublicationRecords && len(missingBindings) != 0) { + return applyRepository{}, fmt.Errorf("plan repository %q has inconsistent publication-record effects", planned.Name) + } + deployment, err := state.LoadDeployment(preparation.root, planned.Name) + if err != nil { + return applyRepository{}, err + } + desiredSigningState, err := repositoryDeploymentSigningState(repository, preparation.manifest.Keys) + if err != nil { + return applyRepository{}, err + } + observed, err := selectedHost.Observe(preparation.ctx, hostRepository) + if err != nil { + return applyRepository{}, err + } + plannedObserved := publishedFromPlanObservation(planned) + trustNotBefore := time.Time{} + if repository.SigningRotation != nil || deployment.SigningRotationPhase != "" { + trustNotBefore, err = state.AuthoritativeDeploymentTrustSince(preparation.root, planned.Name, deployment) + if err != nil { + return applyRepository{}, err + } + } + if err := validateRepositorySigningTransition(repository, preparation.manifest.Keys, deployment, plannedObserved, preparation.now, trustNotBefore); err != nil { + return applyRepository{}, fmt.Errorf("repository %q: %w", planned.Name, err) + } + matchesObserved := revisionMatchesPlanObservation(observed, planned) + managedRemote := repository.Host.Type == "s3" || repository.Host.Type == "github-pages" + matchesApplied := planned.Action != "noop" && observed.TreeSHA256 == planned.DesiredTreeSHA256 && + (!managedRemote || (observed.PlanID == preparation.plan.PlanID && observed.ChangeID == planned.ChangeID && observed.ManifestSHA256 == planned.DesiredManifestSHA256)) + deploymentApplied := deployment.PlanID == preparation.plan.PlanID && deployment.ChangeID == planned.ChangeID && deployment.TreeSHA256 == planned.DesiredTreeSHA256 && deployment.ManifestSHA256 == planned.DesiredManifestSHA256 && deployment.NativeRevision == observed.NativeRevision && deploymentSigningMatches(deployment, desiredSigningState) + deploymentCurrent := deploymentMatchesDesired(deployment, observed, planned.DesiredTreeSHA256, planned.DesiredManifestSHA256, desiredSigningState) + if !reflect.DeepEqual(deployment, planned.ObservedDeployment) && !deploymentApplied { + return applyRepository{}, fmt.Errorf("stale plan: repository %q deployment receipt changed", planned.Name) + } + if !matchesObserved && !matchesApplied { + if observed.TreeSHA256 == planned.ObservedTreeSHA256 { + return applyRepository{}, fmt.Errorf("stale plan: repository %q native revision changed", planned.Name) + } + if observed.TreeSHA256 == planned.DesiredTreeSHA256 { + return applyRepository{}, fmt.Errorf("stale plan: repository %q desired tree was published by another change", planned.Name) + } + return applyRepository{}, fmt.Errorf("stale plan: repository %q target changed", planned.Name) + } + expectedAction := "noop" + if planned.ObservedTreeSHA256 != planned.DesiredTreeSHA256 || (managedRemote && planned.ObservedManifestSHA256 != planned.DesiredManifestSHA256) || !publicationBindingsComplete(lock, repository, ledger) || !deploymentMatchesDesired(planned.ObservedDeployment, plannedObserved, planned.DesiredTreeSHA256, planned.DesiredManifestSHA256, desiredSigningState) { + expectedAction = "update" + if planned.ObservedRevision == "" { + expectedAction = "create" + } + } + if planned.Action != expectedAction || planned.ChangeID != planned.Name+":"+planned.DesiredTreeSHA256[:12] { + return applyRepository{}, fmt.Errorf("plan repository %q has inconsistent action metadata", planned.Name) + } + receiptRecovery := matchesApplied && reflect.DeepEqual(deployment, planned.ObservedDeployment) + current := observed.TreeSHA256 == planned.DesiredTreeSHA256 && (!managedRemote || observed.ManifestSHA256 == planned.DesiredManifestSHA256) && (deploymentApplied || (planned.Action == "noop" && deploymentCurrent) || receiptRecovery) + item := applyRepository{ + planned: planned, repository: repository, lock: lock, host: selectedHost, + hostRepository: hostRepository, observed: observed, current: current, deployment: deployment, signingState: desiredSigningState, + } + // Nothing to build: the host already serves this tree and no signing effect + // has to be replayed, so there is no stage for this repository. + if item.current && (preparation.request.StructuralOnly || planned.Action == "noop") && !(planned.Action != "noop" && len(planned.Signing) != 0) { + return item, nil + } + staging, err := stagingRoot(preparation.root) + if err != nil { + return applyRepository{}, err + } + stage, err := os.MkdirTemp(staging, ".snailmail-apply-*") + if err != nil { + return applyRepository{}, err + } + stageOutput := filepath.Join(stage, "repository") + staged, buildErr := buildLockedRepository(preparation.ctx, preparation.root, planned.Name, repository, lock, preparation.generatedAt, preparation.signatureTime, stageOutput, preparation.blobStore, preparation.manifest.Keys, planned.Signing, nil) + if buildErr == nil && (staged.TreeSHA256 != planned.DesiredTreeSHA256 || staged.ManifestSHA256 != planned.DesiredManifestSHA256) { + buildErr = errors.New("stale plan: rebuilt tree digest changed") + } + if buildErr == nil && !signingContentsEqual(staged.Signing, planned.Signing) { + buildErr = errors.New("stale plan: signing recipe changed") + } + var stagedManifest buildgraph.RepositoryManifest + if buildErr == nil { + structuralRequest := preparation.request + structuralRequest.StructuralOnly = true + stagedManifest, buildErr = verifyStaged(preparation.ctx, repository.Format, stageOutput, structuralRequest) + } + if buildErr != nil { + _ = os.RemoveAll(stage) + return applyRepository{}, buildErr + } + item.stageRoot, item.stage, item.stagedManifest = stage, stageOutput, stagedManifest + return item, nil +} diff --git a/engine/workspace_test.go b/engine/workspace_test.go index 06c622b..d116f6d 100644 --- a/engine/workspace_test.go +++ b/engine/workspace_test.go @@ -1981,3 +1981,25 @@ func (remote *recordingHost) Restore(context.Context, host.Repository, host.Rest func (remote *recordingHost) Abort(context.Context, host.Repository, host.StagedPublication) error { return nil } + +// Publication-record tests need the lock restricted to placed versions +// regardless of a repository's rendered view; production code always filters +// through a repository, so these live with their only callers. +func activeLock(lock state.RepositoryLock) state.RepositoryLock { + lock.PackageVersion = activePackageVersions(lock) + return lock +} + +func activePackageVersions(lock state.RepositoryLock) []state.PackageVersion { + active := make(map[string]bool) + for _, placement := range lock.Placement { + active[placement.Package+"\x00"+placement.Version] = true + } + var result []state.PackageVersion + for _, packageVersion := range lock.PackageVersion { + if active[packageVersion.Package+"\x00"+packageVersion.Version] { + result = append(result, packageVersion) + } + } + return result +} diff --git a/formats/deb/inspect.go b/formats/deb/inspect.go index 1e24aad..fadd47b 100644 --- a/formats/deb/inspect.go +++ b/formats/deb/inspect.go @@ -3,6 +3,7 @@ package deb import ( "archive/tar" "bufio" + "bytes" "compress/gzip" "errors" "fmt" @@ -238,7 +239,7 @@ func compressedTarReader(name string, raw io.Reader, maxMemory uint64) (io.Reade compressed, err := (xz.ReaderConfig{DictCap: int(maxMemory)}).NewReader(raw) return compressed, nil, err case strings.HasSuffix(name, ".zst"): - compressed, err := zstd.NewReader(raw, zstd.WithDecoderMaxMemory(maxMemory)) + compressed, err := zstd.NewReader(raw, zstd.WithDecoderMaxMemory(maxMemory), zstd.WithDecoderConcurrency(1)) if err != nil { return nil, nil, err } @@ -303,7 +304,7 @@ func readControlArchive(name string, raw io.Reader) (map[string]string, error) { } stream = compressed case strings.HasSuffix(name, ".zst"): - compressed, err := zstd.NewReader(raw, zstd.WithDecoderMaxMemory(maxControlArchive)) + compressed, err := zstd.NewReader(raw, zstd.WithDecoderMaxMemory(maxControlArchive), zstd.WithDecoderConcurrency(1)) if err != nil { return nil, fmt.Errorf("open zstd control archive: %w", err) } @@ -358,7 +359,7 @@ func parseControl(content []byte) (map[string]string, error) { } fields := make(map[string]string) seenFields := make(map[string]bool) - scanner := bufio.NewScanner(strings.NewReader(string(content))) + scanner := bufio.NewScanner(bytes.NewReader(content)) scanner.Buffer(make([]byte, 4096), maxControlSize) current := "" for scanner.Scan() { diff --git a/formats/deb/render.go b/formats/deb/render.go index ad9e845..04f3644 100644 --- a/formats/deb/render.go +++ b/formats/deb/render.go @@ -118,14 +118,17 @@ func Build(blobs []domain.Blob, options BuildOptions) (domain.RepositoryArtifact var releaseFiles []releaseFile for _, architecture := range architectures { - var packages strings.Builder + // Render straight into the buffer whose bytes become the file. Building a + // string per stanza, copying it into an index builder and then copying + // the whole index again to bytes held a large Packages index three times. + var packages bytes.Buffer for _, entry := range indexed { if entry.blob.Facts.Architecture != "all" && entry.blob.Facts.Architecture != architecture { continue } - packages.WriteString(renderPackage(entry)) + writePackage(&packages, entry) } - packagesBytes := []byte(packages.String()) + packagesBytes := packages.Bytes() base := path.Join("dists", options.Suite, options.Component, "binary-"+architecture) packagesPath := path.Join(base, "Packages") files = append(files, domain.File{Path: packagesPath, Content: packagesBytes}) @@ -210,7 +213,7 @@ func packagePoolPath(component string, blob domain.Blob) string { return path.Join("pool", component, prefix, blob.Facts.Name, blob.Filename) } -func renderPackage(entry indexedBlob) string { +func writePackage(output *bytes.Buffer, entry indexedBlob) { fields := make(map[string]string, len(entry.blob.Facts.Fields)+5) for name, value := range entry.blob.Facts.Fields { if name != "Status" && name != "Filename" && name != "Size" && name != "MD5sum" && name != "SHA1" && name != "SHA256" { @@ -234,12 +237,10 @@ func renderPackage(entry indexedBlob) string { } return names[i] < names[j] }) - var stanza strings.Builder for _, name := range names { - writeField(&stanza, name, fields[name]) + writeField(output, name, fields[name]) } - stanza.WriteByte('\n') - return stanza.String() + output.WriteByte('\n') } func fieldRank(name string) int { @@ -252,7 +253,7 @@ func fieldRank(name string) int { return len(order) } -func writeField(output *strings.Builder, name, value string) { +func writeField(output *bytes.Buffer, name, value string) { lines := strings.Split(value, "\n") output.WriteString(name) output.WriteString(": ") diff --git a/formats/deb/render_bench_test.go b/formats/deb/render_bench_test.go new file mode 100644 index 0000000..ce67669 --- /dev/null +++ b/formats/deb/render_bench_test.go @@ -0,0 +1,36 @@ +package deb + +import ( + "fmt" + "testing" + "time" + + "github.com/shellcell/snailmail/internal/domain" +) + +// BenchmarkBuildPackagesIndex guards the memory cost of rendering a large +// Packages index, which is the largest allocation in a Debian build. +func BenchmarkBuildPackagesIndex(b *testing.B) { + blobs := make([]domain.Blob, 0, 2000) + for i := 0; i < 2000; i++ { + name := fmt.Sprintf("pkg%04d", i) + blobs = append(blobs, domain.Blob{ + Filename: name + "_1.0.0_amd64.deb", Size: 1024, + MD5: "0123456789abcdef0123456789abcdef", + SHA1: "0123456789abcdef0123456789abcdef01234567", + SHA256: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + Facts: domain.PackageFacts{Name: name, Version: "1.0.0", Architecture: "amd64", Fields: map[string]string{ + "Package": name, "Version": "1.0.0", "Architecture": "amd64", + "Description": "a package\nwith a long multi-line description\n.\nand more text to make the stanza realistic", + "Maintainer": "Someone ", "Depends": "libc6, libssl3, zlib1g", + }}, + }) + } + options := BuildOptions{Suite: "stable", Component: "main", Architectures: []string{"amd64"}, GeneratedAt: time.Unix(0, 0).UTC()} + b.ReportAllocs() + for b.Loop() { + if _, err := Build(blobs, options); err != nil { + b.Fatal(err) + } + } +} diff --git a/go.mod b/go.mod index 372420a..4c893a4 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/Masterminds/semver/v3 v3.5.0 github.com/klauspost/compress v1.19.1 github.com/ulikunitz/xz v0.5.16 - golang.org/x/net v0.43.0 + golang.org/x/net v0.57.0 golang.org/x/sys v0.47.0 gopkg.in/yaml.v3 v3.0.1 ) @@ -16,7 +16,7 @@ require ( github.com/aws/aws-sdk-go-v2 v1.43.0 github.com/aws/aws-sdk-go-v2/config v1.32.31 github.com/aws/aws-sdk-go-v2/service/s3 v1.106.0 - github.com/aws/smithy-go v1.27.3 + github.com/aws/smithy-go v1.27.5 github.com/pelletier/go-toml/v2 v2.4.3 ) @@ -35,6 +35,6 @@ require ( github.com/aws/aws-sdk-go-v2/service/sso v1.33.0 // indirect github.com/aws/aws-sdk-go-v2/service/ssooidc v1.38.0 // indirect github.com/aws/aws-sdk-go-v2/service/sts v1.45.0 // indirect - github.com/cloudflare/circl v1.6.2 // indirect - golang.org/x/crypto v0.41.0 // indirect + github.com/cloudflare/circl v1.6.4 // indirect + golang.org/x/crypto v0.54.0 // indirect ) diff --git a/go.sum b/go.sum index 4af1e2d..d21e05e 100644 --- a/go.sum +++ b/go.sum @@ -36,20 +36,20 @@ github.com/aws/aws-sdk-go-v2/service/ssooidc v1.38.0 h1:tC323YV77QdafeBr6LUhLDTs github.com/aws/aws-sdk-go-v2/service/ssooidc v1.38.0/go.mod h1:SfLK1sgviHmbI+MozR9iDwDjL4cdCVZtahsjoR+z7wg= github.com/aws/aws-sdk-go-v2/service/sts v1.45.0 h1:Pd6PNlp4t8PTXxqzstICl52Wsy78vpjFZ7PRUj44mJc= github.com/aws/aws-sdk-go-v2/service/sts v1.45.0/go.mod h1:rmQ0TnHzuLPmabgjPcsywhsSOmaBDgzR4zvDxSPsGdg= -github.com/aws/smithy-go v1.27.3 h1:F3Zb497UhhskkfpJmfkXswyo+t0sh9OTBnIHjogWbVY= -github.com/aws/smithy-go v1.27.3/go.mod h1:YE2RhdIuDbA5E5bTdciG9KrW3+TiEONeUWCqxX9i1Fc= -github.com/cloudflare/circl v1.6.2 h1:hL7VBpHHKzrV5WTfHCaBsgx/HGbBYlgrwvNXEVDYYsQ= -github.com/cloudflare/circl v1.6.2/go.mod h1:2eXP6Qfat4O/Yhh8BznvKnJ+uzEoTQ6jVKJRn81BiS4= +github.com/aws/smithy-go v1.27.5 h1:d1ro7KpYOYwP6m73YFa+Kc/A130VsAdX68SpsJwARMM= +github.com/aws/smithy-go v1.27.5/go.mod h1:YE2RhdIuDbA5E5bTdciG9KrW3+TiEONeUWCqxX9i1Fc= +github.com/cloudflare/circl v1.6.4 h1:pOXuDTCEYyzydgUpQ0CQz3LsinKjiSk6nNP5Lt5K64U= +github.com/cloudflare/circl v1.6.4/go.mod h1:YxarevkLlbaHuWsxG6vmYNWBEsSp4pnp7j+4VljMavY= github.com/klauspost/compress v1.19.1 h1:VsB4HPswih7mmZ8WleSFQ75c/Ui1M4trX5oAsJnhSlk= github.com/klauspost/compress v1.19.1/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl0PGDogxkrSQ= github.com/pelletier/go-toml/v2 v2.4.3 h1:GTRvJQutkOSftxIFD5xw9aepkYNuPWmVJpffdDPYVpY= github.com/pelletier/go-toml/v2 v2.4.3/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= github.com/ulikunitz/xz v0.5.16 h1:ld6NyySjx5lowVKwJvMRLnW5nxKX/xnpSiFYZ/Lxur0= github.com/ulikunitz/xz v0.5.16/go.mod h1:H9Rt/W6/Qj27PGauhQc6nfCDy7vHpzsOThBSaYDoEhw= -golang.org/x/crypto v0.41.0 h1:WKYxWedPGCTVVl5+WHSSrOBT0O8lx32+zxmHxijgXp4= -golang.org/x/crypto v0.41.0/go.mod h1:pO5AFd7FA68rFak7rOAGVuygIISepHftHnr8dr6+sUc= -golang.org/x/net v0.43.0 h1:lat02VYK2j4aLzMzecihNvTlJNQUq316m2Mr9rnM6YE= -golang.org/x/net v0.43.0/go.mod h1:vhO1fvI4dGsIjh73sWfUVjj3N7CA9WkKJNQm2svM6Jg= +golang.org/x/crypto v0.54.0 h1:YLIA59K4fiNzHzjnZt2tUJQjQtUWfWbeHBqKtk3eScw= +golang.org/x/crypto v0.54.0/go.mod h1:KWL8ny2AZdGR2cWmzeHrp2azQPGogOv+HeQaVEXC2dk= +golang.org/x/net v0.57.0 h1:K5+3DljvIuDG9/Jv9rvyMywYNFCQ9RSUY6OOTTkT+tE= +golang.org/x/net v0.57.0/go.mod h1:KpXc8iv+r3XplLAG/f7Jsf9RPszJzdR0f58q9vGOuEU= golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= diff --git a/internal/app/commit.go b/internal/app/commit.go new file mode 100644 index 0000000..5c5def7 --- /dev/null +++ b/internal/app/commit.go @@ -0,0 +1,25 @@ +package app + +import ( + "fmt" + "os" +) + +// exchangeReleaseLink atomically swaps output with the temporary symlink, +// leaving the displaced entry at temporary. Reading back what was displaced is +// what makes the switch a compare-and-swap rather than a lost update: a +// concurrent writer's release survives at temporary and is reported instead of +// being silently overwritten. +func exchangeReleaseLink(temporary, output, expectedLink string) (bool, error) { + if err := exchangePaths(temporary, output); err != nil { + return false, err + } + actualLink, readErr := os.Readlink(temporary) + if readErr == nil && actualLink == expectedLink { + return true, nil + } + if readErr != nil { + return true, fmt.Errorf("entry changed concurrently and is preserved at %q: %w", temporary, readErr) + } + return true, fmt.Errorf("entry changed concurrently from %q to %q and is preserved at %q", expectedLink, actualLink, temporary) +} diff --git a/internal/app/commit_darwin.go b/internal/app/commit_darwin.go new file mode 100644 index 0000000..f8c8f81 --- /dev/null +++ b/internal/app/commit_darwin.go @@ -0,0 +1,11 @@ +//go:build darwin + +package app + +import "golang.org/x/sys/unix" + +// exchangePaths swaps two directory entries in one atomic step. RENAME_SWAP is +// the darwin equivalent of Linux's RENAME_EXCHANGE. +func exchangePaths(from, to string) error { + return unix.RenamexNp(from, to, unix.RENAME_SWAP) +} diff --git a/internal/app/commit_linux_test.go b/internal/app/commit_exchange_test.go similarity index 80% rename from internal/app/commit_linux_test.go rename to internal/app/commit_exchange_test.go index 655cd6b..0351551 100644 --- a/internal/app/commit_linux_test.go +++ b/internal/app/commit_exchange_test.go @@ -1,4 +1,4 @@ -//go:build linux +//go:build linux || darwin package app @@ -8,7 +8,7 @@ import ( "testing" ) -func TestCommitReleaseLinkPreservesConcurrentReplacement(t *testing.T) { +func TestExchangeReleaseLinkPreservesConcurrentReplacement(t *testing.T) { directory := t.TempDir() output := filepath.Join(directory, "repository") temporary := filepath.Join(directory, "new-link") @@ -18,7 +18,7 @@ func TestCommitReleaseLinkPreservesConcurrentReplacement(t *testing.T) { if err := os.Symlink("new-release", temporary); err != nil { t.Fatal(err) } - committed, err := commitReleaseLink(temporary, output, "old-release") + committed, err := exchangeReleaseLink(temporary, output, "old-release") if err == nil || !committed { t.Fatal("expected concurrent replacement to fail") } @@ -31,7 +31,7 @@ func TestCommitReleaseLinkPreservesConcurrentReplacement(t *testing.T) { } } -func TestCommitReleaseLinkExchangesExpectedSymlink(t *testing.T) { +func TestExchangeReleaseLinkExchangesExpectedSymlink(t *testing.T) { directory := t.TempDir() output := filepath.Join(directory, "repository") temporary := filepath.Join(directory, "new-link") @@ -41,7 +41,7 @@ func TestCommitReleaseLinkExchangesExpectedSymlink(t *testing.T) { if err := os.Symlink("new-release", temporary); err != nil { t.Fatal(err) } - committed, err := commitReleaseLink(temporary, output, "old-release") + committed, err := exchangeReleaseLink(temporary, output, "old-release") if err != nil { t.Fatal(err) } diff --git a/internal/app/commit_linux.go b/internal/app/commit_linux.go index a7d0291..5a0207d 100644 --- a/internal/app/commit_linux.go +++ b/internal/app/commit_linux.go @@ -2,29 +2,9 @@ package app -import ( - "fmt" - "os" +import "golang.org/x/sys/unix" - "golang.org/x/sys/unix" -) - -func commitReleaseLink(temporary, output, expectedLink string) (bool, error) { - if expectedLink == "" { - if err := unix.Renameat2(unix.AT_FDCWD, temporary, unix.AT_FDCWD, output, unix.RENAME_NOREPLACE); err != nil { - return false, err - } - return true, nil - } - if err := unix.Renameat2(unix.AT_FDCWD, temporary, unix.AT_FDCWD, output, unix.RENAME_EXCHANGE); err != nil { - return false, err - } - actualLink, readErr := os.Readlink(temporary) - if readErr == nil && actualLink == expectedLink { - return true, nil - } - if readErr != nil { - return true, fmt.Errorf("entry changed concurrently and is preserved at %q: %w", temporary, readErr) - } - return true, fmt.Errorf("entry changed concurrently from %q to %q and is preserved at %q", expectedLink, actualLink, temporary) +// exchangePaths swaps two directory entries in one atomic step. +func exchangePaths(from, to string) error { + return unix.Renameat2(unix.AT_FDCWD, from, unix.AT_FDCWD, to, unix.RENAME_EXCHANGE) } diff --git a/internal/app/commit_other.go b/internal/app/commit_other.go index 4cd24b3..81feae1 100644 --- a/internal/app/commit_other.go +++ b/internal/app/commit_other.go @@ -1,21 +1,14 @@ -//go:build !linux +//go:build !linux && !darwin package app -import ( - "fmt" - "os" -) +import "errors" -func commitReleaseLink(temporary, output, expectedLink string) (bool, error) { - if expectedLink != "" { - return false, fmt.Errorf("atomic managed-release replacement is not implemented on this platform") - } - if err := os.Link(temporary, output); err != nil { - return false, err - } - if err := os.Remove(temporary); err != nil { - return true, err - } - return true, nil +// exchangePaths needs an atomic directory-entry exchange. Linux provides it +// through renameat2(RENAME_EXCHANGE) and darwin through +// renamex_np(RENAME_SWAP); platforms without either cannot replace an existing +// managed release without risking a lost update. Creating a first managed +// release stays portable. +func exchangePaths(_, _ string) error { + return errors.New("atomic managed-release replacement is not implemented on this platform") } diff --git a/internal/app/environment.go b/internal/app/environment.go new file mode 100644 index 0000000..5da2f84 --- /dev/null +++ b/internal/app/environment.go @@ -0,0 +1,39 @@ +package app + +import ( + "os" + "sort" + "strings" +) + +// runnerEnvironmentAllowlist is what a container runner legitimately needs to +// find its socket, configuration and credentials. Everything else — the signing +// passphrase and cloud credentials in particular — has no business reaching a +// process whose only job is to run a verification image. +var runnerEnvironmentAllowlist = map[string]bool{ + "PATH": true, "HOME": true, "TMPDIR": true, "USER": true, "LOGNAME": true, + "LANG": true, "LC_ALL": true, "TERM": true, + "XDG_RUNTIME_DIR": true, "XDG_CONFIG_HOME": true, "XDG_DATA_HOME": true, + "SSL_CERT_FILE": true, "SSL_CERT_DIR": true, + "DOCKER_HOST": true, "DOCKER_CONFIG": true, "DOCKER_CERT_PATH": true, "DOCKER_TLS_VERIFY": true, + "CONTAINER_HOST": true, "CONTAINER_CONNECTION": true, "CONTAINERS_CONF": true, + "CONTAINERS_STORAGE_CONF": true, "CONTAINERS_REGISTRIES_CONF": true, + "REGISTRY_AUTH_FILE": true, "BUILDAH_ISOLATION": true, + "HTTP_PROXY": true, "HTTPS_PROXY": true, "NO_PROXY": true, + "http_proxy": true, "https_proxy": true, "no_proxy": true, +} + +// runnerEnvironment returns the allowlisted subset of the process environment, +// in sorted order so a child environment is a deterministic function of its +// inputs. +func runnerEnvironment() []string { + environment := make([]string, 0, len(runnerEnvironmentAllowlist)) + for _, entry := range os.Environ() { + name, _, found := strings.Cut(entry, "=") + if found && runnerEnvironmentAllowlist[name] { + environment = append(environment, entry) + } + } + sort.Strings(environment) + return environment +} diff --git a/internal/app/materialize.go b/internal/app/materialize.go index d7e3aa0..0ab511d 100644 --- a/internal/app/materialize.go +++ b/internal/app/materialize.go @@ -310,13 +310,10 @@ func publishRelease(output, staging, expectedLink, expectedTree string) (bool, e if err := os.Symlink(newLink, filepath.Join(control, "current")); err != nil { return false, fmt.Errorf("create current release link: %w", err) } - temporaryName, err := reserveSymlink(parent, "."+filepath.Base(output)+".snailmail-link-*", filepath.Join(filepath.Base(control), "current")) - if err != nil { - return false, err - } - committed, err := commitReleaseLink(temporaryName, output, "") - if err != nil { - return committed, fmt.Errorf("commit initial release link: %w", err) + // os.Symlink is itself the atomic create-if-absent commit: it fails with + // EEXIST rather than replacing an entry another writer published. + if err := os.Symlink(filepath.Join(filepath.Base(control), "current"), output); err != nil { + return false, fmt.Errorf("commit initial release link: %w", err) } if err := syncDirectory(control); err != nil { return true, err @@ -338,7 +335,7 @@ func publishRelease(output, staging, expectedLink, expectedTree string) (bool, e if err != nil { return false, err } - committed, err := commitReleaseLink(temporaryName, filepath.Join(control, "current"), expectedLink) + committed, err := exchangeReleaseLink(temporaryName, filepath.Join(control, "current"), expectedLink) if err != nil { return committed, fmt.Errorf("commit current release: %w", err) } diff --git a/internal/app/redact_test.go b/internal/app/redact_test.go new file mode 100644 index 0000000..cab81d8 --- /dev/null +++ b/internal/app/redact_test.go @@ -0,0 +1,25 @@ +package app + +import ( + "strings" + "testing" +) + +func TestRedactCredentialLeavesPublicOutputIntact(t *testing.T) { + // base64(":") is "Og==" and can appear in output that has nothing to do with + // a credential; a public repository must not have it rewritten. + output := "Downloading wheel Og== from simple/index.html" + if redacted := redactCredential(output, "", ""); redacted != output { + t.Fatalf("public output was rewritten: %q", redacted) + } +} + +func TestRedactCredentialHidesEveryDerivedForm(t *testing.T) { + output := "user=reader pass=top secret escaped=top+secret basic=cmVhZGVyOnRvcCBzZWNyZXQ=" + redacted := redactCredential(output, "reader", "top secret") + for _, secret := range []string{"reader", "top secret", "top+secret", "cmVhZGVyOnRvcCBzZWNyZXQ="} { + if strings.Contains(redacted, secret) { + t.Fatalf("credential form %q survived redaction in %q", secret, redacted) + } + } +} diff --git a/internal/app/verify.go b/internal/app/verify.go index 97a668c..c6a4c72 100644 --- a/internal/app/verify.go +++ b/internal/app/verify.go @@ -32,6 +32,7 @@ import ( "github.com/shellcell/snailmail/host" "github.com/shellcell/snailmail/internal/buildgraph" "github.com/shellcell/snailmail/internal/domain" + "github.com/shellcell/snailmail/internal/factscache" openpgpsigner "github.com/shellcell/snailmail/signer/openpgp" ) @@ -72,7 +73,7 @@ func verifyRepository(root string) (buildgraph.RepositoryManifest, []domain.Blob if len(manifestBytes) > maxManifestSize { return buildgraph.RepositoryManifest{}, nil, errors.New("repository manifest exceeds 8 MiB") } - decoder := json.NewDecoder(strings.NewReader(string(manifestBytes))) + decoder := json.NewDecoder(bytes.NewReader(manifestBytes)) decoder.DisallowUnknownFields() var manifest buildgraph.RepositoryManifest if err := decoder.Decode(&manifest); err != nil { @@ -374,38 +375,41 @@ func verifyHostedPyPIBytes(ctx context.Context, access host.ClientAccess, userna return errors.New("repository verification does not follow redirects") }, } - verifyURL := func(address string, expectedSize int64, expectedSHA256 string) ([]byte, error) { + // Routes cover every file in the tree, including distributions up to + // pypi.MaxArtifactSize. Only the digest and length matter here, so the body + // is hashed as it streams rather than buffered whole. + verifyURL := func(address string, expectedSize int64, expectedSHA256 string) error { request, err := http.NewRequestWithContext(ctx, http.MethodGet, address, nil) if err != nil { - return nil, err + return err } if private { request.SetBasicAuth(username, password) } response, err := client.Do(request) if err != nil { - return nil, err + return err } - content, readErr := io.ReadAll(io.LimitReader(response.Body, expectedSize+1)) + hash := sha256.New() + size, readErr := io.Copy(hash, io.LimitReader(response.Body, expectedSize+1)) closeErr := response.Body.Close() if readErr != nil { - return nil, readErr + return readErr } if closeErr != nil { - return nil, closeErr + return closeErr } - digest := sha256.Sum256(content) - if response.StatusCode != http.StatusOK || int64(len(content)) != expectedSize || hex.EncodeToString(digest[:]) != expectedSHA256 { - return nil, errors.New("host-served repository bytes do not match the reviewed tree") + if response.StatusCode != http.StatusOK || size != expectedSize || hex.EncodeToString(hash.Sum(nil)) != expectedSHA256 { + return errors.New("host-served repository bytes do not match the reviewed tree") } - return content, nil + return nil } for _, route := range access.Routes { parsed, err := url.Parse(route.URL) if err != nil || parsed.Scheme != endpoint.Scheme || parsed.Host != endpoint.Host || parsed.User != nil || parsed.RawQuery != "" || parsed.Fragment != "" || route.Size < 0 { return errors.New("host supplied an invalid client verification route") } - if _, err := verifyURL(route.URL, route.Size, route.SHA256); err != nil { + if err := verifyURL(route.URL, route.Size, route.SHA256); err != nil { return fmt.Errorf("verify host-served route %q: %w", parsed.EscapedPath(), err) } } @@ -451,6 +455,11 @@ func isLoopbackHost(value string) bool { } func redactCredential(value, username, password string) string { + // With no credential every derived form is a constant — base64(":") in + // particular — so redacting them would rewrite unrelated client output. + if username == "" || password == "" { + return value + } secrets := []string{ username, password, url.QueryEscape(username), url.QueryEscape(password), base64.StdEncoding.EncodeToString([]byte(username + ":" + password)), @@ -567,7 +576,7 @@ fi `, } command := exec.CommandContext(caseCtx, runner, arguments...) - command.Env = os.Environ() + command.Env = runnerEnvironment() output, err := command.CombinedOutput() if err != nil { return fmt.Errorf("Debian client verification for %s=%s/%s: %w\n%s", verification.Package, verification.Version, verification.Architecture, err, strings.TrimSpace(string(output))) @@ -729,9 +738,40 @@ else fi HANDLER chmod 0700 /tmp/http-handler -nc -lk -s 127.0.0.1 -p 8879 -e /tmp/http-handler & -server=$! -trap 'kill "$server"' EXIT +# busybox httpd is a real static file server and is present in any +# busybox-based image. The nc handler stays as the fallback: -e and -k vary +# between nc implementations, which is why it is not the first choice. +server="" +if command -v busybox >/dev/null 2>&1 && busybox httpd --help >/dev/null 2>&1; then + busybox httpd -f -p 127.0.0.1:8879 -h /repo & + server=$! + sleep 1 + if ! kill -0 "$server" 2>/dev/null; then + server="" + fi +fi +if test -z "$server"; then + nc -lk -s 127.0.0.1 -p 8879 -e /tmp/http-handler & + server=$! +fi +trap 'kill "$server" 2>/dev/null' EXIT +# Fail with the image contract rather than an opaque Helm error when whichever +# server was chosen never comes up. +if command -v wget >/dev/null 2>&1; then + ready="" + for attempt in 1 2 3 4 5 6 7 8 9 10; do + if wget -q -O /dev/null http://127.0.0.1:8879/index.yaml 2>/dev/null; then + ready=yes + break + fi + sleep 1 + done + if test -z "$ready"; then + echo "verification image provides no usable static HTTP server;" >&2 + echo "it needs busybox httpd, or an nc supporting -l -k -s -p -e" >&2 + exit 1 + fi +fi helm repo add staged http://127.0.0.1:8879 helm repo update staged if test -n "$SNAILMAIL_CHART"; then @@ -744,7 +784,7 @@ fi `, } command := exec.CommandContext(caseCtx, runner, arguments...) - command.Env = os.Environ() + command.Env = runnerEnvironment() output, err := command.CombinedOutput() if err != nil { return fmt.Errorf("Helm client verification for %s@%s: %w\n%s", verification.Project, verification.Version, err, strings.TrimSpace(string(output))) @@ -865,18 +905,23 @@ func verifyPyPIStructure(root string, manifest buildgraph.RepositoryManifest) ([ if len(parts) != 3 || parts[0] != "packages" || parts[1] != file.SHA256 || !pypi.IsDistributionFilename(parts[2]) { return nil, fmt.Errorf("unexpected PyPI repository path %q", file.Path) } - name := filepath.Join(root, filepath.FromSlash(file.Path)) - packageFile, err := os.Open(name) - if err != nil { - return nil, fmt.Errorf("open PyPI package %q: %w", file.Path, err) - } - facts, inspectErr := pypi.Inspect(parts[2], packageFile, file.Size) - closeErr := packageFile.Close() - if inspectErr != nil { - return nil, inspectErr - } - if closeErr != nil { - return nil, fmt.Errorf("close PyPI package %q: %w", file.Path, closeErr) + facts, cached := factscache.Lookup(pypi.FormatID, file.SHA256) + if !cached { + name := filepath.Join(root, filepath.FromSlash(file.Path)) + packageFile, err := os.Open(name) + if err != nil { + return nil, fmt.Errorf("open PyPI package %q: %w", file.Path, err) + } + var inspectErr error + facts, inspectErr = pypi.Inspect(parts[2], packageFile, file.Size) + closeErr := packageFile.Close() + if inspectErr != nil { + return nil, inspectErr + } + if closeErr != nil { + return nil, fmt.Errorf("close PyPI package %q: %w", file.Path, closeErr) + } + factscache.Store(pypi.FormatID, file.SHA256, facts) } blobs = append(blobs, domain.Blob{Filename: parts[2], Size: file.Size, SHA256: file.SHA256, Facts: facts}) } @@ -909,17 +954,22 @@ func verifyDebStructure(root string, manifest buildgraph.RepositoryManifest) ([] return nil, fmt.Errorf("unexpected Debian repository path %q", file.Path) } name := filepath.Join(root, filepath.FromSlash(file.Path)) - packageFile, err := os.Open(name) - if err != nil { - return nil, fmt.Errorf("open Debian package %q: %w", file.Path, err) - } - facts, inspectErr := deb.Inspect(path.Base(file.Path), packageFile, file.Size) - closeErr := packageFile.Close() - if inspectErr != nil { - return nil, inspectErr - } - if closeErr != nil { - return nil, fmt.Errorf("close Debian package %q: %w", file.Path, closeErr) + facts, cached := factscache.Lookup(deb.FormatID, file.SHA256) + if !cached { + packageFile, err := os.Open(name) + if err != nil { + return nil, fmt.Errorf("open Debian package %q: %w", file.Path, err) + } + var inspectErr error + facts, inspectErr = deb.Inspect(path.Base(file.Path), packageFile, file.Size) + closeErr := packageFile.Close() + if inspectErr != nil { + return nil, inspectErr + } + if closeErr != nil { + return nil, fmt.Errorf("close Debian package %q: %w", file.Path, closeErr) + } + factscache.Store(deb.FormatID, file.SHA256, facts) } md5Value, sha1Value, err := legacyChecksums(name) if err != nil { @@ -1011,18 +1061,23 @@ func verifyHelmStructure(root string, manifest buildgraph.RepositoryManifest) ([ if len(parts) != 3 || parts[0] != "charts" || parts[1] != file.SHA256 || !helm.IsChartFilename(parts[2]) { return nil, fmt.Errorf("unexpected Helm repository path %q", file.Path) } - name := filepath.Join(root, filepath.FromSlash(file.Path)) - chartFile, err := os.Open(name) - if err != nil { - return nil, fmt.Errorf("open Helm chart %q: %w", file.Path, err) - } - facts, inspectErr := helm.Inspect(parts[2], chartFile, file.Size) - closeErr := chartFile.Close() - if inspectErr != nil { - return nil, inspectErr - } - if closeErr != nil { - return nil, fmt.Errorf("close Helm chart %q: %w", file.Path, closeErr) + facts, cached := factscache.Lookup(helm.FormatID, file.SHA256) + if !cached { + name := filepath.Join(root, filepath.FromSlash(file.Path)) + chartFile, err := os.Open(name) + if err != nil { + return nil, fmt.Errorf("open Helm chart %q: %w", file.Path, err) + } + var inspectErr error + facts, inspectErr = helm.Inspect(parts[2], chartFile, file.Size) + closeErr := chartFile.Close() + if inspectErr != nil { + return nil, inspectErr + } + if closeErr != nil { + return nil, fmt.Errorf("close Helm chart %q: %w", file.Path, closeErr) + } + factscache.Store(helm.FormatID, file.SHA256, facts) } blobs = append(blobs, domain.Blob{Filename: parts[2], Size: file.Size, SHA256: file.SHA256, Facts: facts}) } @@ -1104,7 +1159,7 @@ func snapshotRepository(ctx context.Context, source string, manifest buildgraph. if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil { return "", err } - if err := copyFile(name, target, file.Size); err != nil { + if err := linkOrCopyFile(name, target, file.Size); err != nil { return "", err } } @@ -1112,6 +1167,25 @@ func snapshotRepository(ctx context.Context, source string, manifest buildgraph. return snapshot, nil } +// linkOrCopyFile prefers a hard link, so snapshotting a repository that can +// reach the 4 GiB verification limit does not duplicate every byte. The link +// shares the inode with an immutable release file, and the snapshot is only +// ever read, so the two stay identical by construction. A link across +// filesystems, or onto one that does not support them, falls back to copying. +func linkOrCopyFile(sourceName, targetName string, expectedSize int64) error { + if err := os.Link(sourceName, targetName); err == nil { + info, statErr := os.Lstat(targetName) + if statErr != nil { + return statErr + } + if !info.Mode().IsRegular() || info.Size() != expectedSize { + return fmt.Errorf("repository file %q changed while snapshotting", sourceName) + } + return nil + } + return copyFile(sourceName, targetName, expectedSize) +} + func copyFile(sourceName, targetName string, expectedSize int64) error { source, err := os.Open(sourceName) if err != nil { diff --git a/internal/factscache/factscache.go b/internal/factscache/factscache.go new file mode 100644 index 0000000..2a41ef5 --- /dev/null +++ b/internal/factscache/factscache.go @@ -0,0 +1,86 @@ +// Package factscache memoises native package facts for the duration of one +// process. +// +// Parsing a package is the single most expensive step in a build: a Debian +// inspection decompresses the entire data archive, up to 256 MiB, only to total +// the installed size. A plan or apply re-derives the same facts several times +// over — once assembling the build input and again verifying the staged tree. +// +// Facts are keyed by format and content digest, which is safe because the +// digest is verified against the lock before anything is cached, and the bytes +// behind a verified digest cannot differ within a run. Nothing is persisted: +// an on-disk cache would have to be trusted across runs, and this is a tool +// whose value comes from re-deriving facts from bytes rather than believing a +// previous answer. +package factscache + +import ( + "sync" + + "github.com/shellcell/snailmail/internal/domain" +) + +// maxEntries bounds the cache so a very large workspace cannot grow it without +// limit. Facts are small, so this is generous; passing it simply stops caching +// rather than evicting, which keeps behaviour independent of insertion order. +const maxEntries = 4096 + +var cache struct { + sync.RWMutex + facts map[string]domain.PackageFacts +} + +func key(format, digest string) string { + return format + "\x00" + digest +} + +// Lookup returns previously derived facts for verified content. +func Lookup(format, digest string) (domain.PackageFacts, bool) { + if digest == "" { + return domain.PackageFacts{}, false + } + cache.RLock() + defer cache.RUnlock() + facts, found := cache.facts[key(format, digest)] + if !found { + return domain.PackageFacts{}, false + } + return clone(facts), true +} + +// Store records facts derived from content whose digest has been verified. +func Store(format, digest string, facts domain.PackageFacts) { + if digest == "" { + return + } + cache.Lock() + defer cache.Unlock() + if cache.facts == nil { + cache.facts = make(map[string]domain.PackageFacts) + } + if len(cache.facts) >= maxEntries { + return + } + cache.facts[key(format, digest)] = clone(facts) +} + +// Reset drops every entry. Tests that rewrite content behind a digest use it. +func Reset() { + cache.Lock() + defer cache.Unlock() + cache.facts = nil +} + +// clone keeps the cached value independent of what a caller does with the +// slice and map it hands back. +func clone(facts domain.PackageFacts) domain.PackageFacts { + facts.Requirements = append([]string(nil), facts.Requirements...) + if facts.Fields != nil { + fields := make(map[string]string, len(facts.Fields)) + for name, value := range facts.Fields { + fields[name] = value + } + facts.Fields = fields + } + return facts +} diff --git a/internal/state/cas.go b/internal/state/cas.go index 02b8297..3b96caa 100644 --- a/internal/state/cas.go +++ b/internal/state/cas.go @@ -20,6 +20,7 @@ import ( "github.com/shellcell/snailmail/formats/helm" "github.com/shellcell/snailmail/formats/pypi" "github.com/shellcell/snailmail/internal/domain" + "github.com/shellcell/snailmail/internal/factscache" ) func PutArtifact(root, format, sourceName string) (domain.Blob, error) { @@ -194,12 +195,17 @@ func LoadBlobContext(ctx context.Context, root, format string, locked LockedBlob if len(locked.SHA256) != sha256.Size*2 { return domain.Blob{}, "", errors.New("locked blob has invalid SHA-256 length") } + // The relative path is built from an already-validated hex digest, so there + // is no caller-supplied component to sanitise, and the os.Root handle below + // refuses to traverse a symlink out of the workspace. Running the general + // path check as well would repeat that containment once per blob. relativeName := filepath.Join(".snailmail", "cas", "sha256", locked.SHA256[:2], locked.SHA256) - name, err := WorkspacePath(root, filepath.ToSlash(relativeName)) + resolvedRoot, err := ResolveWorkspaceRoot(root) if err != nil { - return domain.Blob{}, "", err + return domain.Blob{}, "", fmt.Errorf("%w: resolve workspace root: %v", blob.ErrUnavailable, err) } - rootHandle, err := os.OpenRoot(root) + name := filepath.Join(resolvedRoot, relativeName) + rootHandle, err := os.OpenRoot(resolvedRoot) if err != nil { return domain.Blob{}, "", fmt.Errorf("%w: open workspace root: %v", blob.ErrUnavailable, err) } @@ -222,10 +228,6 @@ func LoadBlobContext(ctx context.Context, root, format string, locked LockedBlob return result, name, nil } -func ValidateLockedBlobFile(name, format string, locked LockedBlob) (domain.Blob, error) { - return validateLockedBlob(name, format, locked) -} - func ValidateLockedBlobReference(format string, locked LockedBlob) error { decoded, err := hex.DecodeString(locked.SHA256) if err != nil || len(decoded) != sha256.Size || locked.SHA256 != strings.ToLower(locked.SHA256) { @@ -241,10 +243,6 @@ func ValidateLockedBlobReference(format string, locked LockedBlob) error { return nil } -func ValidateLockedBlobOpen(file *os.File, info os.FileInfo, name, format string, locked LockedBlob) (domain.Blob, error) { - return validateLockedBlobOpen(file, info, name, format, locked) -} - func ValidateLockedBlobOpenContext(ctx context.Context, file *os.File, info os.FileInfo, name, format string, locked LockedBlob) (domain.Blob, error) { return validateLockedBlobOpenContext(ctx, file, info, name, format, locked) } @@ -269,14 +267,11 @@ func EnsureBlob(ctx context.Context, root, format string, locked LockedBlob, sto return domain.Blob{}, "", localErr } relativeName := filepath.Join(".snailmail", "cas", "sha256", locked.SHA256[:2], locked.SHA256) - name, err = WorkspacePath(root, filepath.ToSlash(relativeName)) - if err != nil { - return domain.Blob{}, "", err - } - resolvedRoot, err := filepath.EvalSymlinks(root) + resolvedRoot, err := ResolveWorkspaceRoot(root) if err != nil { return domain.Blob{}, "", err } + name = filepath.Join(resolvedRoot, relativeName) rootHandle, err := os.OpenRoot(resolvedRoot) if err != nil { return domain.Blob{}, "", err @@ -385,25 +380,36 @@ func validateLockedBlobOpenContext(ctx context.Context, file *os.File, pathInfo if err != nil { return domain.Blob{}, fmt.Errorf("%w: read blob sha256:%s: %w", blob.ErrUnavailable, locked.SHA256, err) } - if _, err := file.Seek(0, io.SeekStart); err != nil { - return domain.Blob{}, fmt.Errorf("%w: seek blob sha256:%s: %w", blob.ErrUnavailable, locked.SHA256, err) - } - facts, err := inspect(format, locked.Filename, contextReaderAt{ctx: ctx, reader: file}, size) - if err != nil { - if ctx.Err() != nil { - return domain.Blob{}, ctx.Err() - } - return domain.Blob{}, fmt.Errorf("%w: inspect blob sha256:%s: %v", blob.ErrCorrupt, locked.SHA256, err) - } validated := domain.Blob{ Filename: locked.Filename, Size: size, MD5: hex.EncodeToString(md5Hash.Sum(nil)), SHA1: hex.EncodeToString(sha1Hash.Sum(nil)), SHA256: hex.EncodeToString(sha256Hash.Sum(nil)), - Facts: facts, } - if size != info.Size() || validated.Size != locked.Size || validated.SHA256 != locked.SHA256 || (locked.MD5 != "" && validated.MD5 != locked.MD5) || (locked.SHA1 != "" && validated.SHA1 != locked.SHA1) || facts.Architecture != locked.Architecture { + // Establish that these bytes are exactly the locked content before consulting + // any memo, so a cached parse is only ever reused for content this call has + // itself just verified. + if size != info.Size() || validated.Size != locked.Size || validated.SHA256 != locked.SHA256 || + (locked.MD5 != "" && validated.MD5 != locked.MD5) || (locked.SHA1 != "" && validated.SHA1 != locked.SHA1) { + return domain.Blob{}, fmt.Errorf("%w: blob sha256:%s disagrees with its lock", blob.ErrCorrupt, locked.SHA256) + } + facts, cached := factscache.Lookup(format, validated.SHA256) + if !cached { + if _, err := file.Seek(0, io.SeekStart); err != nil { + return domain.Blob{}, fmt.Errorf("%w: seek blob sha256:%s: %w", blob.ErrUnavailable, locked.SHA256, err) + } + facts, err = inspect(format, locked.Filename, contextReaderAt{ctx: ctx, reader: file}, size) + if err != nil { + if ctx.Err() != nil { + return domain.Blob{}, ctx.Err() + } + return domain.Blob{}, fmt.Errorf("%w: inspect blob sha256:%s: %v", blob.ErrCorrupt, locked.SHA256, err) + } + factscache.Store(format, validated.SHA256, facts) + } + validated.Facts = facts + if facts.Architecture != locked.Architecture { return domain.Blob{}, fmt.Errorf("%w: blob sha256:%s disagrees with its lock", blob.ErrCorrupt, locked.SHA256) } if err := ctx.Err(); err != nil { diff --git a/internal/state/cas_bench_test.go b/internal/state/cas_bench_test.go new file mode 100644 index 0000000..f29ce00 --- /dev/null +++ b/internal/state/cas_bench_test.go @@ -0,0 +1,37 @@ +package state + +import ( + "testing" + + "github.com/shellcell/snailmail/internal/factscache" + "github.com/shellcell/snailmail/internal/testutil" +) + +// A single apply validates the same artifact several times: once assembling +// the build input and again for each verification pass over the staged tree. +// +// The synthetic package here is small, so this understates the effect on real +// packages: the memoised step decompresses a Debian data archive in full, up +// to 256 MiB, purely to total the installed size. +func BenchmarkRepeatedBlobValidation(b *testing.B) { + root := b.TempDir() + source, err := testutil.WriteDeb(b.TempDir(), "snail-demo", "1.2.3", "amd64", nil) + if err != nil { + b.Fatal(err) + } + blob, err := PutArtifact(root, "deb", source) + if err != nil { + b.Fatal(err) + } + locked := ToLockedBlob(blob) + b.ReportAllocs() + for b.Loop() { + factscache.Reset() + // Four validations, as one apply performs. + for range 4 { + if _, _, err := LoadBlob(root, "deb", locked); err != nil { + b.Fatal(err) + } + } + } +} diff --git a/internal/state/cas_containment_test.go b/internal/state/cas_containment_test.go new file mode 100644 index 0000000..5cc5fee --- /dev/null +++ b/internal/state/cas_containment_test.go @@ -0,0 +1,66 @@ +package state + +import ( + "os" + "path/filepath" + "testing" + + "github.com/shellcell/snailmail/internal/testutil" +) + +// The blob path no longer runs the general workspace path check, so the os.Root +// handle is what keeps a CAS lookup inside the workspace. These hold that line. +func TestLoadBlobRejectsCASDirectorySymlinkedOutOfWorkspace(t *testing.T) { + root := t.TempDir() + outside := t.TempDir() + + source, err := testutil.WriteWheel(t.TempDir(), "snail-demo", "1.2.3", ">=3.9") + if err != nil { + t.Fatal(err) + } + blob, err := PutArtifact(root, "pypi", source) + if err != nil { + t.Fatal(err) + } + locked := ToLockedBlob(blob) + + // Move the whole shard outside the workspace and leave a symlink behind. + shard := filepath.Join(root, ".snailmail", "cas", "sha256", locked.SHA256[:2]) + relocated := filepath.Join(outside, "shard") + if err := os.Rename(shard, relocated); err != nil { + t.Fatal(err) + } + if err := os.Symlink(relocated, shard); err != nil { + t.Fatal(err) + } + if _, _, err := LoadBlob(root, "pypi", locked); err == nil { + t.Fatal("a CAS shard symlinked out of the workspace was accepted") + } +} + +func TestLoadBlobRejectsBlobThatIsItselfASymlink(t *testing.T) { + root := t.TempDir() + outside := t.TempDir() + + source, err := testutil.WriteWheel(t.TempDir(), "snail-demo", "1.2.3", ">=3.9") + if err != nil { + t.Fatal(err) + } + blob, err := PutArtifact(root, "pypi", source) + if err != nil { + t.Fatal(err) + } + locked := ToLockedBlob(blob) + + stored := filepath.Join(root, ".snailmail", "cas", "sha256", locked.SHA256[:2], locked.SHA256) + relocated := filepath.Join(outside, "object") + if err := os.Rename(stored, relocated); err != nil { + t.Fatal(err) + } + if err := os.Symlink(relocated, stored); err != nil { + t.Fatal(err) + } + if _, _, err := LoadBlob(root, "pypi", locked); err == nil { + t.Fatal("a CAS object that is a symlink was accepted") + } +} diff --git a/internal/state/factscache_test.go b/internal/state/factscache_test.go new file mode 100644 index 0000000..d0b49bd --- /dev/null +++ b/internal/state/factscache_test.go @@ -0,0 +1,50 @@ +package state + +import ( + "os" + "path/filepath" + "testing" + + "github.com/shellcell/snailmail/internal/factscache" + "github.com/shellcell/snailmail/internal/testutil" +) + +// A memo keyed by digest must never let corrupt bytes through: the digest is +// re-derived from the file on every call, and only content that matches its +// lock is allowed to consult previously derived facts. +func TestLockedBlobValidationRejectsCorruptBytesDespiteCachedFacts(t *testing.T) { + factscache.Reset() + t.Cleanup(factscache.Reset) + + root := t.TempDir() + source, err := testutil.WriteWheel(t.TempDir(), "snail-demo", "1.2.3", ">=3.9") + if err != nil { + t.Fatal(err) + } + + blob, err := PutArtifact(root, "pypi", source) + if err != nil { + t.Fatal(err) + } + locked := ToLockedBlob(blob) + + // Prime the memo through a successful validation. + if _, _, err := LoadBlob(root, "pypi", locked); err != nil { + t.Fatal(err) + } + if _, found := factscache.Lookup("pypi", locked.SHA256); !found { + t.Fatal("expected verified facts to be memoised") + } + + // Corrupt the stored object while keeping the lock's digest. + stored := filepath.Join(root, ".snailmail", "cas", "sha256", locked.SHA256[:2], locked.SHA256) + if err := os.Chmod(stored, 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(stored, []byte("not a wheel at all"), 0o600); err != nil { + t.Fatal(err) + } + if _, _, err := LoadBlob(root, "pypi", locked); err == nil { + t.Fatal("corrupt CAS object was accepted because facts were cached") + } +} diff --git a/internal/state/git.go b/internal/state/git.go index 72abe06..f0abaa1 100644 --- a/internal/state/git.go +++ b/internal/state/git.go @@ -1,6 +1,7 @@ package state import ( + "bufio" "bytes" "context" "errors" @@ -10,8 +11,11 @@ import ( "os/exec" "path/filepath" "sort" + "strconv" "strings" + "sync" "syscall" + "time" ) func AcquireWorkspaceLock(root string) (func(), error) { @@ -51,10 +55,6 @@ func RequireGitRepositoryContext(ctx context.Context, root string) error { return nil } -func RequireCompleteGitHistory(root string) error { - return requireCompleteGitHistoryContext(context.Background(), root) -} - func RequireCompleteGitHistoryContext(ctx context.Context, root string) error { return requireCompleteGitHistoryContext(ctx, root) } @@ -124,19 +124,19 @@ func requireCleanGitContext(ctx context.Context, root string, allowedUntracked m } func validateGitStatusAllowingUntracked(status string, allowedUntracked, authoritative map[string]bool) error { - for _, line := range strings.Split(status, "\n") { - if line == "" { - continue - } - if len(line) < 4 { - return errors.New("cannot parse Git status") - } - name := filepath.ToSlash(strings.TrimSpace(line[3:])) - if strings.HasPrefix(line, "?? ") && allowedUntracked[name] { + entries, err := parseGitStatus(status) + if err != nil { + return err + } + for _, entry := range entries { + if entry.code == "??" && len(entry.paths) == 1 && allowedUntracked[entry.paths[0]] { continue } - if err := validateGitStatus(line, nil, authoritative); err != nil { - return err + for _, name := range entry.paths { + if entry.code == "??" && !authoritative[name] && !isAuthoritativePath(name) { + continue + } + return fmt.Errorf("workspace has uncommitted authoritative or tracked changes at %q", name) } } return nil @@ -461,6 +461,24 @@ func AssertGitRevisionContext(ctx context.Context, root, expected string) error return nil } +// AssertGitHeadRevision checks only that HEAD still names the expected commit. +// +// It is for callers holding the locks AcquireGitRevisionLock takes, which +// already prevent Git from moving the branch, updating the index or checking +// anything out. Under those locks the full workspace validation cannot find +// anything new, and repeating it once per repository made a publication cost +// grow with the square of the repository count. +func AssertGitHeadRevision(root, expected string) error { + current, err := gitOutput(root, "rev-parse", "HEAD") + if err != nil { + return fmt.Errorf("read Git revision during publication: %w", err) + } + if current != expected { + return errors.New("Git revision changed during operation") + } + return nil +} + // AcquireGitRevisionLock blocks normal branch, index, and checkout updates while // a verified target is switched to the tree authorized by expectedRevision. func AcquireGitRevisionLock(root, expectedRevision string) (func(), error) { @@ -489,11 +507,12 @@ func AcquireGitRevisionLock(root, expectedRevision string) (func(), error) { releases[index]() } } - for _, name := range []string{indexPath + ".lock", headPath + ".lock", refPath + ".lock"} { + names := []string{indexPath + ".lock", headPath + ".lock", refPath + ".lock"} + for _, name := range names { release, err := acquireGitLock(name) if err != nil { releaseAll() - return nil, errors.New("Git changed or is busy before publication") + return nil, fmt.Errorf("Git changed or is busy before publication: %w", describeGitLockConflict(name, names, err)) } releases = append(releases, release) } @@ -545,30 +564,41 @@ func requireAuthoritativeFilesCommitted(root, revision string, authoritative, al return requireAuthoritativeFilesCommittedContext(context.Background(), root, revision, authoritative, allowedChanges) } +// requireAuthoritativeFilesCommittedContext proves that every authoritative +// file is present in revision and identical to the worktree. +// +// Both sides are resolved in one batch each. The worktree side is hashed by Git +// from the file itself rather than through the index, so neither +// assume-unchanged nor skip-worktree can hide a change, and hashing by path +// applies the same .gitattributes and core.autocrlf clean filters that produced +// the committed blob — comparing raw worktree bytes would report every file as +// changed in any repository that converts line endings. func requireAuthoritativeFilesCommittedContext(ctx context.Context, root, revision string, authoritative, allowedChanges map[string]bool) error { names := make([]string, 0, len(authoritative)) for name := range authoritative { - names = append(names, name) + if !allowedChanges[name] { + names = append(names, name) + } } sort.Strings(names) + if len(names) == 0 { + return nil + } + prefix, err := gitPrefix(ctx, root) + if err != nil { + return err + } + treePaths := make([]string, 0, len(names)) for _, name := range names { - if err := ctx.Err(); err != nil { - return err - } - if allowedChanges[name] { - continue - } - treePath, err := workspaceGitPath(root, name) - if err != nil { - return err - } - expected, err := gitOutputContext(ctx, root, "rev-parse", revision+":"+treePath) - if err != nil { - if ctx.Err() != nil { - return ctx.Err() - } - return fmt.Errorf("authoritative state %q is not committed to Git", name) + // hash-object reads its path list as newline-separated text, so a name + // carrying a newline could inject an entry. Authoritative names come from + // validated manifest fields and fixed globs, but a file on disk can still + // be named adversarially. + if strings.ContainsAny(name, "\n\r") { + return fmt.Errorf("authoritative state %q has an unusable name", name) } + // A symlink with the same bytes is not a committed regular file, and a + // content comparison alone would not tell them apart. workspacePath, err := WorkspacePath(root, name) if err != nil { return err @@ -577,40 +607,220 @@ func requireAuthoritativeFilesCommittedContext(ctx context.Context, root, revisi if err != nil || !info.Mode().IsRegular() { return fmt.Errorf("authoritative state %q is not a regular committed file", name) } - content, err := os.ReadFile(workspacePath) - if err != nil { - return err - } - actual, err := gitHashObjectContext(ctx, root, content) - if err != nil { - if ctx.Err() != nil { - return ctx.Err() - } - return err + treePaths = append(treePaths, filepath.ToSlash(filepath.Join(filepath.FromSlash(prefix), filepath.FromSlash(name)))) + } + committed, err := revisionBlobIDs(ctx, root, revision, treePaths) + if err != nil { + return err + } + for index, name := range names { + if committed[index] == "" { + return fmt.Errorf("authoritative state %q is not committed to Git", name) } - if actual != expected { + } + worktree, err := worktreeBlobIDs(ctx, root, treePaths) + if err != nil { + return err + } + for index, name := range names { + if worktree[index] != committed[index] { return fmt.Errorf("authoritative state %q differs from Git", name) } } return nil } -func validateGitStatus(status string, allowedChanges, authoritative map[string]bool) error { - for _, line := range strings.Split(status, "\n") { - if line == "" { - continue +// revisionBlobIDs resolves every tree path in revision through one cat-file +// batch, reporting an empty ID for a path revision does not contain. +func revisionBlobIDs(ctx context.Context, root, revision string, treePaths []string) ([]string, error) { + var query bytes.Buffer + for _, treePath := range treePaths { + query.WriteString(revision + ":" + treePath + "\n") + } + command := gitCommandContext(ctx, root, "cat-file", "--batch-check") + command.Stdin = &query + output, err := command.Output() + if err != nil { + if ctx.Err() != nil { + return nil, ctx.Err() } - if len(line) < 4 { - return errors.New("cannot parse Git status") + return nil, fmt.Errorf("resolve committed authoritative state: %w", err) + } + lines := strings.Split(strings.TrimRight(string(output), "\n"), "\n") + if len(lines) != len(treePaths) { + return nil, errors.New("cannot resolve committed authoritative state") + } + identifiers := make([]string, len(treePaths)) + for index, line := range lines { + // A resolvable object answers " "; anything else, such + // as " missing", means revision does not contain that path. + fields := strings.Fields(line) + if len(fields) == 3 && fields[1] == "blob" { + identifiers[index] = fields[0] } - name := filepath.ToSlash(strings.TrimSpace(line[3:])) - if allowedChanges[name] { + } + return identifiers, nil +} + +// maxBatchObjectSize bounds a single object read through catFileBatch. Ledger +// blobs are the only use, and they are line-oriented records. +const maxBatchObjectSize = 64 << 20 + +type batchObject struct { + id string + content []byte +} + +// catFileBatch reads every requested revspec through one cat-file process +// rather than one `git show` per object. Objects the repository does not +// contain come back with an empty id. +func catFileBatch(ctx context.Context, root string, revspecs []string) ([]batchObject, error) { + if len(revspecs) == 0 { + return nil, nil + } + var query bytes.Buffer + for _, revspec := range revspecs { + query.WriteString(revspec) + query.WriteString("\n") + } + command := gitCommandContext(ctx, root, "cat-file", "--batch") + command.Stdin = &query + output, err := command.StdoutPipe() + if err != nil { + return nil, err + } + if err := command.Start(); err != nil { + return nil, err + } + objects, readErr := readCatFileBatch(bufio.NewReaderSize(output, 64<<10), len(revspecs)) + if readErr != nil { + _ = command.Process.Kill() + _ = command.Wait() + if ctx.Err() != nil { + return nil, ctx.Err() + } + return nil, readErr + } + if err := command.Wait(); err != nil { + if ctx.Err() != nil { + return nil, ctx.Err() + } + return nil, fmt.Errorf("read Git objects: %w", err) + } + return objects, nil +} + +func readCatFileBatch(reader *bufio.Reader, expected int) ([]batchObject, error) { + objects := make([]batchObject, 0, expected) + for len(objects) < expected { + header, err := reader.ReadString('\n') + if err != nil { + return nil, fmt.Errorf("read Git object header: %w", err) + } + fields := strings.Fields(strings.TrimSuffix(header, "\n")) + // " missing" for anything the repository does not contain. + if len(fields) == 2 && fields[1] == "missing" { + objects = append(objects, batchObject{}) continue } - if strings.HasPrefix(line, "?? ") && !authoritative[name] && !isAuthoritativePath(name) { + if len(fields) != 3 || fields[1] != "blob" { + return nil, errors.New("unexpected Git object header") + } + size, err := strconv.ParseInt(fields[2], 10, 64) + if err != nil || size < 0 || size > maxBatchObjectSize { + return nil, errors.New("Git object size is invalid or exceeds the read limit") + } + content := make([]byte, size) + if _, err := io.ReadFull(reader, content); err != nil { + return nil, fmt.Errorf("read Git object: %w", err) + } + // cat-file --batch terminates each object with a newline. + if _, err := reader.Discard(1); err != nil { + return nil, fmt.Errorf("read Git object terminator: %w", err) + } + objects = append(objects, batchObject{id: fields[0], content: content}) + } + return objects, nil +} + +// worktreeBlobIDs hashes each worktree file as Git would when committing it, +// in one batch. Reading the files rather than the index is what keeps +// assume-unchanged and skip-worktree from hiding a modification. Unlike most +// commands, hash-object resolves --stdin-paths entries against the repository +// root rather than the working directory, so these must be tree paths. +func worktreeBlobIDs(ctx context.Context, root string, treePaths []string) ([]string, error) { + var query bytes.Buffer + for _, treePath := range treePaths { + query.WriteString(treePath) + query.WriteString("\n") + } + command := gitCommandContext(ctx, root, "hash-object", "--stdin-paths") + command.Stdin = &query + output, err := command.Output() + if err != nil { + if ctx.Err() != nil { + return nil, ctx.Err() + } + return nil, fmt.Errorf("hash authoritative worktree state: %w", err) + } + lines := strings.Split(strings.TrimRight(string(output), "\n"), "\n") + if len(lines) != len(treePaths) { + return nil, errors.New("cannot hash authoritative worktree state") + } + for index := range lines { + lines[index] = strings.TrimSpace(lines[index]) + } + return lines, nil +} + +type gitStatusEntry struct { + code string + paths []string +} + +// parseGitStatus reads NUL-delimited porcelain v1 records. The NUL format is +// what makes paths trustworthy: it never quotes or escapes, so core.quotePath +// cannot change the bytes, and a rename carries its original path as a separate +// NUL-terminated field instead of an ambiguous "old -> new" string. +func parseGitStatus(status string) ([]gitStatusEntry, error) { + records := strings.Split(status, "\x00") + var entries []gitStatusEntry + for index := 0; index < len(records); index++ { + record := records[index] + if record == "" { continue } - return fmt.Errorf("workspace has uncommitted authoritative or tracked changes at %q", name) + if len(record) < 4 { + return nil, errors.New("cannot parse Git status") + } + entry := gitStatusEntry{code: record[:2], paths: []string{filepath.ToSlash(record[3:])}} + if entry.code[0] == 'R' || entry.code[0] == 'C' || entry.code[1] == 'R' || entry.code[1] == 'C' { + index++ + if index >= len(records) { + return nil, errors.New("cannot parse Git status") + } + entry.paths = append(entry.paths, filepath.ToSlash(records[index])) + } + entries = append(entries, entry) + } + return entries, nil +} + +func validateGitStatus(status string, allowedChanges, authoritative map[string]bool) error { + entries, err := parseGitStatus(status) + if err != nil { + return err + } + for _, entry := range entries { + for _, name := range entry.paths { + if allowedChanges[name] { + continue + } + if entry.code == "??" && !authoritative[name] && !isAuthoritativePath(name) { + continue + } + return fmt.Errorf("workspace has uncommitted authoritative or tracked changes at %q", name) + } } return nil } @@ -699,8 +909,38 @@ func publicationWorkspacePaths(repositories []string) map[string]bool { return paths } +// gitPrefix returns the workspace's path prefix inside its Git repository. +// The prefix cannot change while a workspace operation holds its lock, and it +// is consulted once per authoritative path, so it is resolved once per root +// rather than through a subprocess per lookup. +var gitPrefixes struct { + sync.Mutex + byRoot map[string]string +} + +func gitPrefix(ctx context.Context, root string) (string, error) { + gitPrefixes.Lock() + cached, found := gitPrefixes.byRoot[root] + gitPrefixes.Unlock() + if found { + return cached, nil + } + prefix, err := gitOutputContext(ctx, root, "rev-parse", "--show-prefix") + if err != nil { + return "", err + } + prefix = filepath.ToSlash(prefix) + gitPrefixes.Lock() + if gitPrefixes.byRoot == nil { + gitPrefixes.byRoot = make(map[string]string) + } + gitPrefixes.byRoot[root] = prefix + gitPrefixes.Unlock() + return prefix, nil +} + func workspaceGitPath(root, relative string) (string, error) { - prefix, err := gitOutput(root, "rev-parse", "--show-prefix") + prefix, err := gitPrefix(context.Background(), root) if err != nil { return "", err } @@ -708,11 +948,10 @@ func workspaceGitPath(root, relative string) (string, error) { } func workspaceRelativeGitPath(root, name string) (string, bool, error) { - prefix, err := gitOutput(root, "rev-parse", "--show-prefix") + prefix, err := gitPrefix(context.Background(), root) if err != nil { return "", false, err } - prefix = filepath.ToSlash(prefix) name = filepath.ToSlash(name) if prefix == "" { return name, true, nil @@ -803,11 +1042,23 @@ func resolveGitPath(root, name string) (string, error) { return filepath.Clean(resolved), nil } +// gitLockOwnerPrefix marks a lock file this process created, so a lock left +// behind by a crash can be told apart from one Git is genuinely holding. +const gitLockOwnerPrefix = "snailmail publication lock" + func acquireGitLock(name string) (func(), error) { file, err := os.OpenFile(name, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o600) if err != nil { return nil, err } + // Git only reads a lock file it created itself, so recording who holds this + // one is free and turns an opaque "File exists" into something actionable. + owner := fmt.Sprintf("%s\npid=%d\nsince=%s\n", gitLockOwnerPrefix, os.Getpid(), time.Now().UTC().Format(time.RFC3339)) + if _, err := file.WriteString(owner); err != nil { + _ = file.Close() + _ = os.Remove(name) + return nil, err + } if err := file.Close(); err != nil { _ = os.Remove(name) return nil, err @@ -815,6 +1066,53 @@ func acquireGitLock(name string) (func(), error) { return func() { _ = os.Remove(name) }, nil } +// describeGitLockConflict explains which lock blocked publication and, when it +// was left behind by an interrupted snailmail run, says so and names every file +// that has to be removed to recover. +func describeGitLockConflict(blocked string, names []string, cause error) error { + if !errors.Is(cause, os.ErrExist) { + return fmt.Errorf("%s: %w", blocked, cause) + } + content, readErr := os.ReadFile(blocked) + if readErr != nil || !strings.HasPrefix(string(content), gitLockOwnerPrefix) { + return fmt.Errorf("%s is held by another Git process", blocked) + } + details := strings.ReplaceAll(strings.TrimSpace(string(content)), "\n", " ") + if owner, found := gitLockOwnerProcess(content); found && gitLockOwnerIsRunning(owner) { + return fmt.Errorf("%s is held by a running snailmail process (%s)", blocked, details) + } + return fmt.Errorf("%s was left behind by a snailmail run that did not finish (%s); "+ + "if no snailmail process is publishing this repository, remove %s to recover", + blocked, details, strings.Join(names, ", ")) +} + +func gitLockOwnerProcess(content []byte) (int, bool) { + for _, line := range strings.Split(string(content), "\n") { + if value, found := strings.CutPrefix(line, "pid="); found { + pid, err := strconv.Atoi(strings.TrimSpace(value)) + return pid, err == nil && pid > 0 + } + } + return 0, false +} + +// gitLockOwnerIsRunning is a variable so tests can decide the answer instead of +// depending on which process ids happen to exist. Signalling a live process +// also answers differently for root and for an ordinary user, so no literal pid +// gives the same result everywhere. +var gitLockOwnerIsRunning = processExists + +// processExists reports whether a process id is live. A recycled id can make +// this a false positive, which is why the caller only uses it to soften the +// wording rather than to remove anything. +func processExists(pid int) bool { + process, err := os.FindProcess(pid) + if err != nil { + return false + } + return process.Signal(syscall.Signal(0)) == nil +} + func copyGitIndex(indexPath string) (string, error) { source, err := os.Open(indexPath) if err != nil { @@ -885,9 +1183,24 @@ func gitCommand(root string, arguments ...string) *exec.Cmd { return gitCommandContext(context.Background(), root, arguments...) } +// gitConfigOverrides neutralise user configuration that would otherwise change +// output this package parses. They are applied as -c overrides rather than by +// disabling global and system configuration wholesale, so that settings the +// repository genuinely needs — safe.directory in particular, which Git accepts +// only from those files — keep working. +var gitConfigOverrides = []string{ + // Prepends signature verification output to `log --format=%B`. + "-c", "log.showSignature=false", + // Can answer from a stale daemon view of the worktree. + "-c", "core.fsmonitor=", + // Quoting would change the path bytes read back from status output. + "-c", "core.quotePath=false", +} + func gitCommandContext(ctx context.Context, root string, arguments ...string) *exec.Cmd { - command := exec.CommandContext(ctx, "git", append([]string{"-C", root}, arguments...)...) - command.Env = replaceEnvironment(os.Environ(), "GIT_OPTIONAL_LOCKS", "0") + full := append([]string{"-C", root}, gitConfigOverrides...) + command := exec.CommandContext(ctx, "git", append(full, arguments...)...) + command.Env = replaceEnvironment(os.Environ(), "GIT_OPTIONAL_LOCKS", "0", "GIT_TERMINAL_PROMPT", "0") return command } @@ -912,11 +1225,11 @@ func gitStatusOutput(root string) (string, error) { } func gitStatusOutputContext(ctx context.Context, root string) (string, error) { - output, err := gitCommandContext(ctx, root, "status", "--porcelain", "--untracked-files=all").Output() + output, err := gitCommandContext(ctx, root, "status", "--porcelain=v1", "-z", "--untracked-files=all", "--no-renames").Output() if err != nil { return "", err } - return strings.TrimRight(string(output), "\r\n"), nil + return string(output), nil } func gitOutputEnv(root string, environment []string, arguments ...string) (string, error) { @@ -952,8 +1265,15 @@ func replaceEnvironment(environment []string, values ...string) []string { } result = append(result, entry) } - for key, value := range replacements { - result = append(result, key+"="+value) + // Map iteration order is random; sort so the child environment is a + // deterministic function of its inputs. + keys := make([]string, 0, len(replacements)) + for key := range replacements { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + result = append(result, key+"="+replacements[key]) } return result } diff --git a/internal/state/git_status_test.go b/internal/state/git_status_test.go new file mode 100644 index 0000000..f3d9411 --- /dev/null +++ b/internal/state/git_status_test.go @@ -0,0 +1,49 @@ +package state + +import "testing" + +func TestParseGitStatusReadsRenameAndSpacedPaths(t *testing.T) { + // Porcelain v1 -z emits a rename as the new path followed by the original + // path in its own NUL-terminated field, and never quotes or escapes. + status := "R repos/new name.lock.toml\x00repos/old name.lock.toml\x00 M snailmail.toml\x00?? notes.txt\x00" + entries, err := parseGitStatus(status) + if err != nil { + t.Fatal(err) + } + if len(entries) != 3 { + t.Fatalf("parsed %d entries, want 3: %+v", len(entries), entries) + } + rename := entries[0] + if rename.code != "R " || len(rename.paths) != 2 || + rename.paths[0] != "repos/new name.lock.toml" || rename.paths[1] != "repos/old name.lock.toml" { + t.Fatalf("rename entry parsed as %+v", rename) + } + if entries[1].code != " M" || entries[1].paths[0] != "snailmail.toml" { + t.Fatalf("modification entry parsed as %+v", entries[1]) + } + if entries[2].code != "??" || entries[2].paths[0] != "notes.txt" { + t.Fatalf("untracked entry parsed as %+v", entries[2]) + } +} + +func TestValidateGitStatusRejectsRenamedAuthoritativeState(t *testing.T) { + // The old newline parser produced the single name "a -> b", which matched no + // allowlist entry and no authoritative path, so a renamed lock could slip + // through as an unrecognised entry. + status := "R repos/renamed.lock.toml\x00repos/pypi.lock.toml\x00" + authoritative := map[string]bool{"repos/pypi.lock.toml": true} + if err := validateGitStatus(status, nil, authoritative); err == nil { + t.Fatal("expected a renamed authoritative lock to be rejected") + } +} + +func TestValidateGitStatusAllowsPermittedUntrackedPath(t *testing.T) { + status := "?? publications/pypi.jsonl\x00" + allowed := map[string]bool{"publications/pypi.jsonl": true} + if err := validateGitStatusAllowingUntracked(status, allowed, nil); err != nil { + t.Fatalf("permitted untracked ledger was rejected: %v", err) + } + if err := validateGitStatusAllowingUntracked(status, nil, nil); err == nil { + t.Fatal("expected an unpermitted untracked ledger to be rejected") + } +} diff --git a/internal/state/gitlock_test.go b/internal/state/gitlock_test.go new file mode 100644 index 0000000..c70f85e --- /dev/null +++ b/internal/state/gitlock_test.go @@ -0,0 +1,102 @@ +package state + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func withGitLockOwnerRunning(t *testing.T, running func(int) bool) { + t.Helper() + previous := gitLockOwnerIsRunning + gitLockOwnerIsRunning = running + t.Cleanup(func() { gitLockOwnerIsRunning = previous }) +} + +// A lock whose owner is still alive is reported as busy, not as recoverable +// wreckage, so the message must not invite anyone to delete it. +func TestGitLockConflictReportsRunningOwnerAsBusy(t *testing.T) { + directory := t.TempDir() + blocked := filepath.Join(directory, "index.lock") + withGitLockOwnerRunning(t, func(int) bool { return true }) + held := gitLockOwnerPrefix + "\npid=4242\nsince=2026-01-01T00:00:00Z\n" + if err := os.WriteFile(blocked, []byte(held), 0o600); err != nil { + t.Fatal(err) + } + _, err := acquireGitLock(blocked) + if err == nil { + t.Fatal("expected acquiring an existing lock to fail") + } + message := describeGitLockConflict(blocked, []string{blocked}, err).Error() + if !strings.Contains(message, "running snailmail process") { + t.Fatalf("message %q does not report a live owner", message) + } + if strings.Contains(message, "remove") { + t.Fatalf("a lock a live process holds must not be recommended for removal: %q", message) + } +} + +func TestGitLockRecordsItsOwner(t *testing.T) { + name := filepath.Join(t.TempDir(), "index.lock") + release, err := acquireGitLock(name) + if err != nil { + t.Fatal(err) + } + defer release() + content, err := os.ReadFile(name) + if err != nil { + t.Fatal(err) + } + if !strings.HasPrefix(string(content), gitLockOwnerPrefix) { + t.Fatalf("lock file does not record its owner: %q", content) + } + pid, found := gitLockOwnerProcess(content) + if !found || pid != os.Getpid() { + t.Fatalf("recorded pid = %d found=%v, want %d", pid, found, os.Getpid()) + } +} + +// A crash leaves lock files behind that block every later Git operation. The +// failure has to say that snailmail left them and which ones to remove, +// otherwise the operator only sees Git's opaque "File exists". +func TestGitLockConflictExplainsAbandonedLock(t *testing.T) { + directory := t.TempDir() + blocked := filepath.Join(directory, "index.lock") + names := []string{blocked, filepath.Join(directory, "HEAD.lock")} + + withGitLockOwnerRunning(t, func(int) bool { return false }) + abandoned := gitLockOwnerPrefix + "\npid=4242\nsince=2026-01-01T00:00:00Z\n" + if err := os.WriteFile(blocked, []byte(abandoned), 0o600); err != nil { + t.Fatal(err) + } + _, err := acquireGitLock(blocked) + if err == nil { + t.Fatal("expected acquiring an existing lock to fail") + } + message := describeGitLockConflict(blocked, names, err).Error() + for _, want := range []string{"snailmail", "remove", blocked, names[1]} { + if !strings.Contains(message, want) { + t.Fatalf("message %q does not mention %q", message, want) + } + } +} + +func TestGitLockConflictDoesNotClaimForeignLocks(t *testing.T) { + directory := t.TempDir() + blocked := filepath.Join(directory, "index.lock") + if err := os.WriteFile(blocked, []byte("some other git process"), 0o600); err != nil { + t.Fatal(err) + } + _, err := acquireGitLock(blocked) + if err == nil { + t.Fatal("expected acquiring an existing lock to fail") + } + message := describeGitLockConflict(blocked, []string{blocked}, err).Error() + if strings.Contains(message, "remove") { + t.Fatalf("a lock snailmail does not own must not be recommended for removal: %q", message) + } + if !strings.Contains(message, "another Git process") { + t.Fatalf("message %q does not attribute the lock to Git", message) + } +} diff --git a/internal/state/ledger.go b/internal/state/ledger.go index 7359be0..242f81d 100644 --- a/internal/state/ledger.go +++ b/internal/state/ledger.go @@ -38,8 +38,13 @@ func LoadLedgerHistory(root, repository string) ([]PublicationRecord, error) { return LoadLedgerHistoryContext(context.Background(), root, repository) } +// LoadLedgerHistoryContext replays the publication history reachable from HEAD. +// Scoping to the current history is deliberate: a record that only ever existed +// on an abandoned branch or an old tag was never part of this line of history, +// and folding those in reported perfectly good ledgers as having "removed an +// immutable record". func LoadLedgerHistoryContext(ctx context.Context, root, repository string) ([]PublicationRecord, error) { - return loadLedgerHistoryContext(ctx, root, repository, "--all") + return loadLedgerHistoryContext(ctx, root, repository, "HEAD") } func LoadLedgerHistoryAtRevisionContext(ctx context.Context, root, repository, revision string) ([]PublicationRecord, error) { @@ -75,20 +80,40 @@ func loadLedgerHistoryContext(ctx context.Context, root, repository, revision st if ctx.Err() != nil { return nil, ctx.Err() } + // A workspace can record artifacts before its first commit, and naming a + // revision fails outright on an unborn HEAD where a whole-repository walk + // would simply have been empty. + if _, verifyErr := gitOutputContext(ctx, root, "rev-parse", "--verify", "--quiet", revision+"^{commit}"); verifyErr != nil { + return records, nil + } return nil, fmt.Errorf("read publication history: %w", err) } - for _, revision := range strings.Fields(output) { + commits := strings.Fields(output) + revspecs := make([]string, 0, len(commits)) + for _, commit := range commits { + revspecs = append(revspecs, commit+":"+treePath) + } + // One cat-file batch instead of a `git show` process per historical commit, + // which grew without bound as a workspace accumulated publications. + objects, err := catFileBatch(ctx, root, revspecs) + if err != nil { + return nil, fmt.Errorf("read publication history: %w", err) + } + replayed := make(map[string]bool, len(objects)) + for index, object := range objects { if err := ctx.Err(); err != nil { return nil, err } - content, err := execGitShowContext(ctx, root, revision, treePath) - if err != nil { - if ctx.Err() != nil { - return nil, ctx.Err() - } - return nil, fmt.Errorf("read publication ledger at %s: %w", revision, err) + if object.id == "" { + return nil, fmt.Errorf("read publication ledger at %s", commits[index]) } - historical, err := parseLedger(bytes.NewReader(content)) + // Successive commits often carry identical ledger bytes; parsing each + // distinct blob once keeps the replay linear in content, not commits. + if replayed[object.id] { + continue + } + replayed[object.id] = true + historical, err := parseLedger(bytes.NewReader(object.content)) if err != nil { return nil, err } @@ -142,10 +167,6 @@ func parseLedger(reader io.Reader) ([]PublicationRecord, error) { return records, nil } -func recordIdentity(record PublicationRecord) string { - return record.Repository + "\x00" + record.PlanID + "\x00" + record.ChangeID + "\x00" + record.Package + "\x00" + record.Version + "\x00" + strings.Join(record.BlobSHA256, ",") + "\x00" + record.TreeSHA256 + "\x00" + record.RecordedAt -} - func publicationRecordKey(record PublicationRecord) string { return record.PlanID + "\x00" + record.ChangeID + "\x00" + record.Package + "\x00" + record.Version } @@ -284,22 +305,6 @@ func PreparePublicationRecords(root, baseRevision, repository, planID, changeID, return atomicWrite(name, expectedContent, 0o644) } -func ValidatePreparedPublicationLedger(root, baseRevision, repository, planID, changeID, treeSHA, recordedAt string, lock RepositoryLock) error { - expectedContent, err := expectedPublicationLedger(root, baseRevision, repository, planID, changeID, treeSHA, recordedAt, lock) - if err != nil { - return err - } - name, err := ledgerPath(root, repository) - if err != nil { - return err - } - actualContent, err := os.ReadFile(name) - if err != nil { - return err - } - return comparePublicationLedger(repository, expectedContent, actualContent) -} - func ValidateCommittedPublicationLedger(root, baseRevision, committedRevision, repository, planID, changeID, treeSHA, recordedAt string, lock RepositoryLock) error { expectedContent, err := expectedPublicationLedger(root, baseRevision, repository, planID, changeID, treeSHA, recordedAt, lock) if err != nil { diff --git a/internal/state/ledger_history_test.go b/internal/state/ledger_history_test.go new file mode 100644 index 0000000..14c7dd9 --- /dev/null +++ b/internal/state/ledger_history_test.go @@ -0,0 +1,83 @@ +package state + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "testing" +) + +func runGit(t *testing.T, root string, arguments ...string) { + t.Helper() + full := append([]string{"-C", root, + "-c", "user.name=snailmail-test", "-c", "user.email=test@example.invalid"}, arguments...) + if output, err := exec.Command("git", full...).CombinedOutput(); err != nil { + t.Fatalf("git %v: %v: %s", arguments, err, output) + } +} + +func writeLedger(t *testing.T, root, repository, content string) { + t.Helper() + directory := filepath.Join(root, "publications") + if err := os.MkdirAll(directory, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(directory, repository+".jsonl"), []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} + +func ledgerLine(planID, treeSHA, pkg, version string) string { + return `{"schema_version":1,"plan_id":"` + planID + `","change_id":"pypi:` + treeSHA[:12] + + `","repository":"pypi","package":"` + pkg + `","version":"` + version + + `","blob_sha256":["` + treeSHA + `"],"tree_sha256":"` + treeSHA + `","recorded_at":"2026-01-01T00:00:00Z"}` + "\n" +} + +// A record that only ever existed on an abandoned branch was never part of this +// line of history. Replaying every ref folded it in and then reported the +// perfectly good worktree ledger as having removed an immutable record. +func TestLedgerHistoryIgnoresAbandonedBranch(t *testing.T) { + root := t.TempDir() + runGit(t, root, "init") + planID := "11" + "00000000000000000000000000000000000000000000000000000000000000"[:62] + treeSHA := "22" + "00000000000000000000000000000000000000000000000000000000000000"[:62] + kept := ledgerLine(planID, treeSHA, "kept", "1.0.0") + writeLedger(t, root, "pypi", kept) + runGit(t, root, "add", "-A") + runGit(t, root, "commit", "-m", "record kept publication") + + runGit(t, root, "checkout", "-q", "-b", "abandoned") + otherPlan := "33" + "00000000000000000000000000000000000000000000000000000000000000"[:62] + otherTree := "44" + "00000000000000000000000000000000000000000000000000000000000000"[:62] + writeLedger(t, root, "pypi", kept+ledgerLine(otherPlan, otherTree, "abandoned", "9.9.9")) + runGit(t, root, "add", "-A") + runGit(t, root, "commit", "-m", "record abandoned publication") + + runGit(t, root, "checkout", "-q", "-") + writeLedger(t, root, "pypi", kept) + + records, err := LoadLedgerHistoryContext(context.Background(), root, "pypi") + if err != nil { + t.Fatalf("history replay rejected a workspace with an abandoned branch: %v", err) + } + if len(records) != 1 || records[0].Package != "kept" { + t.Fatalf("replayed %d records, want only the reachable one: %+v", len(records), records) + } +} + +// Records committed on the current line of history are still immutable. +func TestLedgerHistoryRejectsRemovedReachableRecord(t *testing.T) { + root := t.TempDir() + runGit(t, root, "init") + planID := "55" + "00000000000000000000000000000000000000000000000000000000000000"[:62] + treeSHA := "66" + "00000000000000000000000000000000000000000000000000000000000000"[:62] + writeLedger(t, root, "pypi", ledgerLine(planID, treeSHA, "kept", "1.0.0")) + runGit(t, root, "add", "-A") + runGit(t, root, "commit", "-m", "record publication") + + writeLedger(t, root, "pypi", "") + if _, err := LoadLedgerHistoryContext(context.Background(), root, "pypi"); err == nil { + t.Fatal("expected removal of a reachable immutable record to be rejected") + } +} diff --git a/internal/state/ledger_test.go b/internal/state/ledger_test.go index ab7661d..8ce9187 100644 --- a/internal/state/ledger_test.go +++ b/internal/state/ledger_test.go @@ -97,3 +97,9 @@ func TestPublicationRecordIdentityIncludesAuditFields(t *testing.T) { t.Fatal("publication identity ignored its audit timestamp") } } + +// recordIdentity distinguishes two records by every field a ledger line +// carries; it exists for these assertions, not for production use. +func recordIdentity(record PublicationRecord) string { + return record.Repository + "\x00" + record.PlanID + "\x00" + record.ChangeID + "\x00" + record.Package + "\x00" + record.Version + "\x00" + strings.Join(record.BlobSHA256, ",") + "\x00" + record.TreeSHA256 + "\x00" + record.RecordedAt +} diff --git a/internal/state/store.go b/internal/state/store.go index 1e549d5..298f723 100644 --- a/internal/state/store.go +++ b/internal/state/store.go @@ -17,6 +17,7 @@ import ( "regexp" "sort" "strings" + "sync" "time" "unicode" @@ -868,7 +869,9 @@ func AddBlob(lock *RepositoryLock, format, track, distro string, blob LockedBlob if !placementExists { lock.Placement = append(lock.Placement, Placement{Package: packageName, Version: version, Track: track, Distro: distro}) } - canonicalizeLock(lock) + // Canonical order is a property of the written file, and WriteLock + // establishes it. Sorting here instead re-sorted the whole lock once per + // added artifact, which made `snailmail add` quadratic in an existing lock. return !blobExists || !placementExists, nil } @@ -1190,11 +1193,40 @@ func ValidateArtifactOrigin(origin ArtifactOrigin) error { return nil } +// resolvedRoots caches the symlink resolution of a workspace root. A root +// cannot change while an operation holds the workspace lock, and resolving it +// is otherwise repeated for every path, including once per blob in a build. +var resolvedRoots struct { + sync.Mutex + byRoot map[string]string +} + +// ResolveWorkspaceRoot returns the workspace root with symlinks resolved. +func ResolveWorkspaceRoot(root string) (string, error) { + resolvedRoots.Lock() + cached, found := resolvedRoots.byRoot[root] + resolvedRoots.Unlock() + if found { + return cached, nil + } + resolved, err := filepath.EvalSymlinks(root) + if err != nil { + return "", err + } + resolvedRoots.Lock() + if resolvedRoots.byRoot == nil { + resolvedRoots.byRoot = make(map[string]string) + } + resolvedRoots.byRoot[root] = resolved + resolvedRoots.Unlock() + return resolved, nil +} + func WorkspacePath(root, relative string) (string, error) { if err := validateRelativePath(relative); err != nil { return "", err } - resolvedRoot, err := filepath.EvalSymlinks(root) + resolvedRoot, err := ResolveWorkspaceRoot(root) if err != nil { return "", err } @@ -1243,7 +1275,7 @@ func ensureGitignore(root string) error { if err != nil && !errors.Is(err, os.ErrNotExist) { return err } - if strings.Contains(string(content), ".snailmail/") { + if hasIgnoreRule(content, ".snailmail/") { return nil } if len(content) != 0 && content[len(content)-1] != '\n' { @@ -1252,3 +1284,16 @@ func ensureGitignore(root string) error { content = append(content, []byte(block)...) return atomicWrite(name, content, 0o644) } + +// hasIgnoreRule reports whether rule is present as its own directive. A +// substring search also matched the rule inside a comment or, worse, inside a +// "!.snailmail/" negation that re-includes exactly what must stay ignored. +func hasIgnoreRule(content []byte, rule string) bool { + for _, line := range strings.Split(string(content), "\n") { + line = strings.TrimSpace(strings.TrimSuffix(line, "\r")) + if line == rule || line == "/"+rule { + return true + } + } + return false +} diff --git a/internal/wire/signers.go b/internal/wire/signers.go index 8266851..a690702 100644 --- a/internal/wire/signers.go +++ b/internal/wire/signers.go @@ -1,7 +1,7 @@ package wire import ( - "errors" + "fmt" "os" "path/filepath" @@ -10,6 +10,10 @@ import ( const SigningPassphraseEnvironment = "SNAILMAIL_KEY_PASSPHRASE" +// MinimumSigningPassphraseBytes is the documented floor for a signing key +// passphrase, which should come from a secret manager rather than be typed. +const MinimumSigningPassphraseBytes = 24 + func NewSignerStore() (*filesigner.Store, error) { dataHome := os.Getenv("XDG_DATA_HOME") if dataHome == "" { @@ -21,8 +25,8 @@ func NewSignerStore() (*filesigner.Store, error) { } return filesigner.New(filepath.Join(dataHome, "snailmail", "private-keys"), func() ([]byte, error) { value := os.Getenv(SigningPassphraseEnvironment) - if len(value) < 16 { - return nil, errors.New("signing key passphrase environment must contain at least 16 bytes") + if len(value) < MinimumSigningPassphraseBytes { + return nil, fmt.Errorf("%s must contain at least %d bytes", SigningPassphraseEnvironment, MinimumSigningPassphraseBytes) } return []byte(value), nil })