diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 99fd43d..6c8665d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -49,6 +49,15 @@ jobs: sudo apt-get update -qq sudo apt-get install -y -qq qemu-user-static binfmt-support test -e /proc/sys/fs/binfmt_misc/qemu-aarch64 + # Client verification skips where the machine cannot run the container it + # needs, so that a developer on a runtime without a CPU cgroup sees what + # was checked rather than a daemon error. A skip here would be + # indistinguishable from a pass, and every client test would silently stop + # running if a runner image lost the capability. This makes their absence + # a failure, for the same reason binfmt_misc is asserted above. + - name: Require real client verification + if: runner.os == 'Linux' + run: echo "SNAILMAIL_REQUIRE_CLIENT_VERIFICATION=1" >> "$GITHUB_ENV" - run: make fmt - run: make vet - run: make lint diff --git a/PLAN.md b/PLAN.md index 6eb915b..caf893e 100644 --- a/PLAN.md +++ b/PLAN.md @@ -829,6 +829,13 @@ Private material sits behind `KeyRef` with pluggable backends — Actions secret `pass`/1Password, age/SOPS in-repo, KMS, hardware token. Where the backend can sign remotely, the engine never holds key material. +Two exist. `file` owns a key: it generates one and is the only thing that can +produce it again, which is what a workstation wants. `env` owns nothing and +resolves a key the environment supplies, which is what a CI runner needs, since +the runner is discarded after every job and the key comes from a store that +already holds it. Neither signs remotely, so the engine still holds material +briefly in both; the backends that would not are the ones left. + ## 9. Operations The verbs, and which part of the model each one touches: @@ -1394,8 +1401,7 @@ snailmail keys new|publish|audit|rotate snailmail approval-key | blob-store | render | serve ``` -Planned: `import ` (adopt an existing repository into a manifest), `ui`, -`server`. Every command that reports a result accepts `--json`. +Planned: `server`. `import ` and `ui` are implemented. Every command that reports a result accepts `--json`. ## 14. Phases @@ -1442,18 +1448,64 @@ rotation, plan-resolved deterministic signatures verified through apt Tier 1 is complete: rpm and apk landed, and every format the knowledge bundle marks signable is now signed — Debian's `InRelease` and `Release.gpg`, a yum `repomd.xml.asc`, an Alpine index per architecture, and a Helm `.prov` per -chart, each verified by running the real client. Signing lives behind a -`formats.Signer` interface rather than a switch in the engine, so a format -either satisfies it or does not. +chart, each verified by running the real client with the signature check on. +Signing lives behind a `formats.Signer` interface rather than a switch in the +engine, so a format either satisfies it or does not. + +That last clause was only true of Debian for a while. rpm, apk and Helm each +signed correctly and were then verified by a client told not to check: dnf ran +with `repo_gpgcheck=0`, apk with `--allow-untrusted`, and `helm pull` never read +the `.prov` beside the chart. So a signature no client would accept verified +green, which is the one thing repository signing exists to prevent. The +verifiers now install the way the generated install instructions have always +told a consumer to, and `internal/app/signed_verification_test.go` holds every +signable format to it, because the hole was not that any one script was wrong +but that nothing tied a format's signing to its verification. The format-and-host coupling that this phase owed is closed for its first cases: GitHub Pages serves signed Debian, rpm, apk, raw and Helm alongside PyPI. The matrix itself is declared in `host/support.go` rather than inferred, so the gaps that remain are readable rather than discovered — and each undeclared pair -records why. Remaining: additional key backends and the TUI. The next formats -are nix cache, cargo, go, and maven, in that order. `import` is implemented, and -so is S3 beyond PyPI — see the paragraph below, which was written when it landed -and outlived this sentence. +records why. The next formats are nix cache, cargo, go, and maven, in that +order. + +Both things this plan calls a TUI are built, and they are different things. +§13's is the products-by-destinations matrix: `status --matrix` renders it once +and `ui` draws it full screen, from one model over engine.StatusWorkspace, so +the two cannot come to disagree about where a product is published. It is a +viewer and not a console — everything that changes a workspace is a reviewed +diff and a plan, and a keystroke is neither. Raw terminal handling is termios +through golang.org/x/sys, already a dependency, so a full-screen view costs no +new one. + +§7's is the setup wizard, built as it requires: it produces an argument list and nothing else, so what configures a repository is the same command +either way, and it prints that command. Making it a flag-filler needed setup's +flags declared once and read twice -- by the command that acts on them and the +wizard that asks for them -- which is what stops the parallel config writer §7 +warns about. It is line-based rather than a full-screen interface, which needs +no dependency and lets a scripted session be a test. + +Doing it surfaced a flag-grouping bug: --output was offered for every host, but +an object store and Pages both refuse a path outright, so on those two it was a +flag that could only make the repository invalid. Its own help had always named +local and rsync. `import` is implemented, and so is S3 beyond PyPI — see +the paragraph below, which was written when it landed and outlived this +sentence. + +Key backends are no longer a single item. `KeyRef.Backend` was carried through +every layer and read by nothing: `wire.NewSignerStore` returned the file store +itself, and both the adapter and the manifest validator hard-rejected any other +value. Selecting a backend is the composition root's job, so that is where the +dispatch now lives, and the field decides something. + +The second backend is `env`, which resolves a key the environment supplies +rather than one it owns — the CI case §8 names first among the backends private +material sits behind. It generates nothing and deletes nothing, and says so when +asked: a backend that reported success for either would let a rotation believe +it had retired a key that every job still receives. What remains of §8's list is +the backends that sign without releasing the key at all — KMS and a hardware +token — which are a different shape of work, because the engine would assemble +the OpenPGP packet around a signature made elsewhere. S3 beyond PyPI turned out not to be a declaration. The adapter no longer hardcodes `simple/index.html`: `host.Repository` carries `CommitPaths`, filled diff --git a/README.md b/README.md index e3a7791..193d20b 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,16 @@ go run ./cmd/snailmail apply `plan` writes what it intends to do; `apply` does it and records a publication ledger entry. Nothing reaches a host until a plan has been applied. +`status --matrix` reports products against destinations — which package is +published where, and at what version — and `snailmail ui` draws the same matrix +full screen, refreshing on `r`. Both read what `status --json` emits, so neither +can show something the JSON does not say. + +`setup ` with nothing else asks for what it needs, offering only the +flags the chosen host uses, and prints the command it ran — which is the one to +keep for CI. It fills flags and nothing more, so there is no second way to +configure a repository. Add `--interactive` to ask when input is not a terminal. + Each command prints the one that usually comes next, so the sequence can be followed without this page. The `git commit` in the middle is not incidental: desired state is reviewed as a diff, and `plan` reads committed state only. diff --git a/adapters/signer/env/env.go b/adapters/signer/env/env.go new file mode 100644 index 0000000..f73f7f4 --- /dev/null +++ b/adapters/signer/env/env.go @@ -0,0 +1,207 @@ +// Package envsigner reads a signing key that was generated somewhere else and +// handed to this process by its environment. +// +// The file backend owns a key: it generates one, writes it under the user's +// data directory, and is the only thing that can produce it again. That is the +// right shape for a workstation and the wrong one for CI, where the runner is +// discarded after every job and the key has to arrive from a secret store that +// already holds it. PLAN.md §8 names that case first among the backends private +// material sits behind. +// +// So this backend generates nothing. It resolves a key that already exists, in +// the same encrypted armored form the file backend writes, supplied either +// directly or as a path to a mounted file. +package envsigner + +import ( + "bytes" + "context" + "errors" + "fmt" + "os" + "regexp" + "strings" + "time" + + "github.com/shellcell/snailmail/signer" + apkrsa "github.com/shellcell/snailmail/signer/apkrsa" + openpgpsigner "github.com/shellcell/snailmail/signer/openpgp" +) + +// Backend is the name a key reference carries to be resolved here. +const Backend = "env" + +// maxMaterialBytes bounds what is read, so a variable or a path pointing at +// something enormous fails cleanly rather than being pulled into memory. An +// RSA4096 private key in the armored form the file backend writes is a few +// kilobytes. +const maxMaterialBytes = 1 << 20 + +// referencePattern is the reference shape both backends share: the workspace +// that owns the key, then the key's own name. +// +// Only the name is used here. The workspace half is validated and otherwise +// ignored, because an environment variable named after a 64-character digest is +// not something anyone would configure. That means one runner serving two +// workspaces with a same-named key would hand both the same material -- and what +// catches it is the fingerprint, not the reference: planning a signed repository +// resolves the key and refuses when "private signer identity differs from +// committed public key". +var referencePattern = regexp.MustCompile(`^([a-f0-9]{64})/([a-z][a-z0-9-]*)$`) + +type Store struct { + lookup func(string) (string, bool) + readFile func(string) ([]byte, error) + passphrase func() ([]byte, error) +} + +// New reads keys from the process environment. +func New(passphrase func() ([]byte, error)) (*Store, error) { + if passphrase == nil { + return nil, errors.New("a passphrase provider is required") + } + return &Store{lookup: os.LookupEnv, readFile: os.ReadFile, passphrase: passphrase}, nil +} + +// Variable is the environment variable a key's material is read from, and +// Variable+"_FILE" is the path form. +// +// It is derived from the key's name rather than its reference because the +// reference begins with a workspace digest, which nothing would want to type +// into a CI configuration. Key names cannot contain an underscore, so mapping a +// hyphen onto one cannot make two names collide. +// +// A runner that serves more than one workspace therefore has to keep their +// variables apart itself, by running each job with only the keys it needs. +func Variable(name string) string { + return "SNAILMAIL_KEY_" + strings.ToUpper(strings.ReplaceAll(name, "-", "_")) +} + +// Generate refuses. A key this backend can see is one that already existed. +func (store *Store) Generate(context.Context, signer.Ref, signer.Algorithm, string, time.Time, time.Duration) (signer.Generated, error) { + return signer.Generated{}, errors.New( + "the env backend cannot generate a signing key: create one with the file backend, or with whatever already holds it, and supply it through the environment") +} + +// Delete refuses for the same reason. Removing the variable is the environment's +// business, and reporting success without doing anything would let a rotation +// believe it had retired a key that is still being handed to every job. +func (store *Store) Delete(_ context.Context, ref signer.Ref) error { + name, err := keyName(ref) + if err != nil { + return err + } + return fmt.Errorf("the env backend cannot delete a signing key: unset %s where it is configured", Variable(name)) +} + +func (store *Store) Public(ctx context.Context, ref signer.Ref) (signer.Generated, error) { + loaded, err := store.load(ctx, ref) + if err != nil { + return signer.Generated{}, err + } + defer loaded.Close() + // Both signers expose their public forms; the interface carries only what + // signing needs, so this asks for the rest. + type publisher interface { + Public() (signer.Generated, error) + } + public, ok := loaded.(publisher) + if !ok { + return signer.Generated{}, errors.New("signing key cannot produce public forms") + } + return public.Public() +} + +func (store *Store) Resolve(ctx context.Context, ref signer.Ref) (signer.Signer, error) { + return store.load(ctx, ref) +} + +func (store *Store) load(ctx context.Context, ref signer.Ref) (signer.Signer, error) { + name, err := keyName(ref) + if err != nil { + return nil, err + } + if err := ctx.Err(); err != nil { + return nil, err + } + content, err := store.material(name) + if err != nil { + return nil, err + } + defer wipe(content) + passphrase, err := store.passphrase() + if err != nil { + return nil, err + } + defer wipe(passphrase) + // The supplied form says which kind of key it is, exactly as the stored form + // does for the file backend, so a workspace holding both resolves each + // correctly without recording the algorithm a second time. + if bytes.Contains(content, []byte("SNAILMAIL ENCRYPTED APK PRIVATE KEY")) { + return apkrsa.Open(content, passphrase, signer.Identity{}) + } + return openpgpsigner.Load(content, passphrase) +} + +// material reads the key, preferring the file form. +// +// A container's environment is readable through the runtime's inspection API +// for as long as the container exists, however the value was supplied, so a +// mounted file that can be read once and removed is the better of the two. The +// value form stays supported because it is what most CI secret integrations +// offer directly. This mirrors how the passphrase itself is supplied. +func (store *Store) material(name string) ([]byte, error) { + variable := Variable(name) + if path, present := store.lookup(variable + "_FILE"); present && path != "" { + if _, both := store.lookup(variable); both { + return nil, fmt.Errorf("set %s or %s, not both", variable, variable+"_FILE") + } + content, err := store.readFile(path) + if err != nil { + return nil, fmt.Errorf("read %s: %w", variable+"_FILE", err) + } + return bounded(bytes.TrimRight(content, "\r\n"), variable+"_FILE") + } + value, present := store.lookup(variable) + if !present || value == "" { + return nil, absent(variable) + } + return bounded([]byte(value), variable) +} + +// absent reports a key the environment did not supply. +// +// It wraps ErrNotFound because callers branch on it -- `keys new` treats it as +// "create one" -- but it names the variable, because the likeliest way to reach +// it is a CI job that did not pass the secret, and "signing key not found" sends +// whoever reads it looking in the wrong place. +func absent(variable string) error { + return fmt.Errorf("%w: set %s or %s to the encrypted private key", signer.ErrNotFound, variable, variable+"_FILE") +} + +func bounded(content []byte, source string) ([]byte, error) { + if len(content) == 0 { + return nil, absent(strings.TrimSuffix(source, "_FILE")) + } + if len(content) > maxMaterialBytes { + return nil, fmt.Errorf("%s exceeds %d bytes", source, maxMaterialBytes) + } + return content, nil +} + +func keyName(ref signer.Ref) (string, error) { + if ref.Backend != Backend { + return "", errors.New("invalid env signing key reference") + } + match := referencePattern.FindStringSubmatch(ref.ID) + if match == nil { + return "", errors.New("invalid env signing key reference") + } + return match[2], nil +} + +func wipe(value []byte) { + for index := range value { + value[index] = 0 + } +} diff --git a/adapters/signer/env/env_test.go b/adapters/signer/env/env_test.go new file mode 100644 index 0000000..7449510 --- /dev/null +++ b/adapters/signer/env/env_test.go @@ -0,0 +1,190 @@ +package envsigner + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/shellcell/snailmail/signer" + openpgpsigner "github.com/shellcell/snailmail/signer/openpgp" +) + +const passphrase = "environment-backend-test-passphrase" + +const workspace = "1111111111111111111111111111111111111111111111111111111111111111" + +func reference(name string) signer.Ref { + return signer.Ref{Backend: Backend, ID: workspace + "/" + name} +} + +// Generating an RSA4096 key costs a second or two, and every test below wants +// the same one. +var ( + sharedOnce sync.Once + sharedMaterial []byte + sharedIdentity signer.Identity +) + +func encryptedKey(t *testing.T) ([]byte, signer.Identity) { + t.Helper() + sharedOnce.Do(func() { + generated, err := openpgpsigner.Generate("archive", time.Unix(1_700_000_000, 0).UTC(), 48*time.Hour, []byte(passphrase)) + if err != nil { + t.Fatal(err) + } + sharedMaterial, sharedIdentity = generated.PrivateArmor, generated.Identity + }) + return sharedMaterial, sharedIdentity +} + +func storeWith(t *testing.T, environment map[string]string) *Store { + t.Helper() + store, err := New(func() ([]byte, error) { return []byte(passphrase), nil }) + if err != nil { + t.Fatal(err) + } + store.lookup = func(name string) (string, bool) { + value, present := environment[name] + return value, present + } + return store +} + +// The point of the backend: a key that exists elsewhere is usable here, without +// this process ever writing private material to disk. +func TestAKeySuppliedByTheEnvironmentSigns(t *testing.T) { + material, identity := encryptedKey(t) + for _, form := range []string{"value", "file"} { + t.Run(form, func(t *testing.T) { + environment := map[string]string{} + if form == "value" { + environment[Variable("archive")] = string(material) + } else { + name := filepath.Join(t.TempDir(), "archive.asc") + if err := os.WriteFile(name, material, 0o600); err != nil { + t.Fatal(err) + } + environment[Variable("archive")+"_FILE"] = name + } + store := storeWith(t, environment) + resolved, err := store.Resolve(context.Background(), reference("archive")) + if err != nil { + t.Fatalf("a key supplied as a %s was not resolved: %v", form, err) + } + defer resolved.Close() + resolvedIdentity, err := resolved.Identity(context.Background()) + if err != nil { + t.Fatal(err) + } + if resolvedIdentity.Fingerprint != identity.Fingerprint { + t.Errorf("resolved fingerprint %q, want %q", resolvedIdentity.Fingerprint, identity.Fingerprint) + } + response, err := store.Public(context.Background(), reference("archive")) + if err != nil || response.Identity.Fingerprint != identity.Fingerprint { + t.Errorf("public forms = %q err=%v", response.Identity.Fingerprint, err) + } + }) + } +} + +// A trailing newline is what a here-doc, an editor, or `gh secret` leaves on a +// mounted file, and it is not part of the key. +func TestAMountedKeySurvivesATrailingNewline(t *testing.T) { + material, identity := encryptedKey(t) + name := filepath.Join(t.TempDir(), "archive.asc") + if err := os.WriteFile(name, append(append([]byte(nil), material...), '\n'), 0o600); err != nil { + t.Fatal(err) + } + store := storeWith(t, map[string]string{Variable("archive") + "_FILE": name}) + resolved, err := store.Resolve(context.Background(), reference("archive")) + if err != nil { + t.Fatalf("a key with a trailing newline was refused: %v", err) + } + defer resolved.Close() + resolvedIdentity, err := resolved.Identity(context.Background()) + if err != nil || resolvedIdentity.Fingerprint != identity.Fingerprint { + t.Errorf("identity = %q err=%v", resolvedIdentity.Fingerprint, err) + } +} + +// The likeliest failure in CI is a job that did not pass the secret, and the +// error has to name what to set. It still wraps ErrNotFound, because `keys new` +// branches on that to decide whether a key has to be created. +func TestAnUnsuppliedKeyIsNotFoundAndNamesItsVariable(t *testing.T) { + store := storeWith(t, map[string]string{}) + _, err := store.Resolve(context.Background(), reference("apt-archive")) + if !errors.Is(err, signer.ErrNotFound) { + t.Fatalf("error = %v, want it to wrap ErrNotFound", err) + } + if !strings.Contains(err.Error(), "SNAILMAIL_KEY_APT_ARCHIVE") { + t.Errorf("error = %v, want the variable named", err) + } +} + +// Two sources for one key is a configuration mistake worth reporting, not a +// precedence rule worth guessing at. +func TestSupplyingBothFormsIsRefused(t *testing.T) { + material, _ := encryptedKey(t) + name := filepath.Join(t.TempDir(), "archive.asc") + if err := os.WriteFile(name, material, 0o600); err != nil { + t.Fatal(err) + } + store := storeWith(t, map[string]string{ + Variable("archive"): string(material), Variable("archive") + "_FILE": name, + }) + _, err := store.Resolve(context.Background(), reference("archive")) + if err == nil || !strings.Contains(err.Error(), "not both") { + t.Fatalf("error = %v, want both forms refused", err) + } +} + +// This backend owns nothing, so both of the operations that would imply +// ownership refuse -- and say what to do instead. Reporting success for either +// would let a rotation believe it had retired a key that every job still gets. +func TestTheBackendRefusesToOwnAKey(t *testing.T) { + store := storeWith(t, map[string]string{}) + _, err := store.Generate(context.Background(), reference("archive"), signer.AlgorithmOpenPGPRSA4096, + "archive", time.Unix(1_700_000_000, 0).UTC(), time.Hour) + if err == nil || !strings.Contains(err.Error(), "cannot generate") { + t.Errorf("Generate error = %v, want a refusal", err) + } + err = store.Delete(context.Background(), reference("apt-archive")) + if err == nil || !strings.Contains(err.Error(), "SNAILMAIL_KEY_APT_ARCHIVE") { + t.Errorf("Delete error = %v, want a refusal naming the variable", err) + } +} + +// The variable is what an operator types into a CI configuration, so its shape +// is part of the interface rather than an implementation detail. +func TestVariableNamesAreDerivedFromTheKeyName(t *testing.T) { + for name, want := range map[string]string{ + "archive": "SNAILMAIL_KEY_ARCHIVE", + "apt-archive": "SNAILMAIL_KEY_APT_ARCHIVE", + "alpine-signing-2027": "SNAILMAIL_KEY_ALPINE_SIGNING_2027", + } { + if got := Variable(name); got != want { + t.Errorf("Variable(%q) = %q, want %q", name, got, want) + } + } +} + +// A reference for another backend must not resolve here, or the composition +// root's dispatch would be advisory. +func TestAReferenceForAnotherBackendIsRefused(t *testing.T) { + store := storeWith(t, map[string]string{}) + for _, ref := range []signer.Ref{ + {Backend: "file", ID: workspace + "/archive"}, + {Backend: Backend, ID: "archive"}, + {Backend: Backend, ID: "../../etc/archive"}, + {Backend: Backend, ID: workspace + "/Archive"}, + } { + if _, err := store.Resolve(context.Background(), ref); err == nil || errors.Is(err, signer.ErrNotFound) { + t.Errorf("reference %+v was accepted: %v", ref, err) + } + } +} diff --git a/cmd/snailmail/main.go b/cmd/snailmail/main.go index 566f3d5..493bc85 100644 --- a/cmd/snailmail/main.go +++ b/cmd/snailmail/main.go @@ -33,7 +33,7 @@ const defaultGeneratedAt = "1970-01-01T00:00:00Z" func main() { ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer stop() - err := run(ctx, os.Args[1:], os.Stdout, os.Stderr) + err := runWithInput(ctx, os.Args[1:], os.Stdin, os.Stdout, os.Stderr) // Asking for help is not a failure. The flag package reports it as one, and // every `--help` ended with "snailmail: flag: help requested" printed under // the usage the reader had just asked for. @@ -114,7 +114,13 @@ func exitNote(code int) string { return "" } +// run is what every caller but main uses: there is nobody to answer a prompt, +// so setup's wizard cannot engage and no test can hang on one. func run(ctx context.Context, args []string, stdout, stderr io.Writer) error { + return runWithInput(ctx, args, nil, stdout, stderr) +} + +func runWithInput(ctx context.Context, args []string, stdin io.Reader, stdout, stderr io.Writer) error { if len(args) == 0 { printUsage(stdout) return nil @@ -125,7 +131,7 @@ func run(ctx context.Context, args []string, stdout, stderr io.Writer) error { case "init": return runInit(args[1:], stdout, stderr) case "setup": - return runSetup(args[1:], stdout, stderr) + return runSetup(args[1:], stdin, stdout, stderr) case "add": return runAdd(ctx, args[1:], stdout, stderr) case "promote": @@ -136,6 +142,8 @@ func run(ctx context.Context, args []string, stdout, stderr io.Writer) error { return runPrune(args[1:], stdout, stderr) case "check": return runCheck(ctx, args[1:], stdout, stderr) + case "ui": + return runUI(ctx, args[1:], stdin, stdout, stderr) case "status": return runStatus(ctx, args[1:], stdout, stderr) case "site": @@ -231,74 +239,53 @@ func runInit(args []string, stdout, stderr io.Writer) error { return nil } -func runSetup(args []string, stdout, stderr io.Writer) error { +func runSetup(args []string, stdin io.Reader, stdout, stderr io.Writer) error { if len(args) == 0 { return errors.New("usage: snailmail setup --name NAME --host [host options]") } format := args[0] - flags := newCommandFlags("setup "+format, stderr).withWorkspace().withJSON() - name := flags.String("name", "", "repository name") - output := flags.String("output", "", - "published directory: workspace-relative for a local host, an absolute remote path for rsync") - hostType := flags.String("host", "local", "host type: local, s3, rsync, or github-pages") - visibility := flags.String("visibility", "public", "repository visibility") - gatePolicy := flags.String("gate", "auto", "publication gate: auto, pr, or approval") - approvalKeys := flags.String("approval-keys", "", "comma-separated allowed Ed25519 public keys") - signingKey := flags.String("signing-key", "", "repository signing key name") - // Named for the format being configured. It said "Debian" while applying to - // every format snailmail signs, so an rpm repository was refused for want of - // a flag whose help said it was for something else. - allowUnsigned := flags.Bool("allow-unsigned", false, - fmt.Sprintf("explicitly allow a new unsigned %s repository", format)) - track := flags.String("track", "stable", "rendered placement track") - keep := flags.Int("keep", 0, - "publications to retain when collecting; 0 uses the default and can be changed later in snailmail.toml") - sshTarget := flags.String("target", "", "ssh destination for an rsync host, such as deploy@packages.example") - bucket := flags.String("bucket", "", "S3 bucket") - prefix := flags.String("prefix", "", "S3 object prefix") - region := flags.String("region", "", "AWS region") - endpoint := flags.String("endpoint", "", "optional S3 API endpoint") - canonicalEndpoint := flags.String("base-url", "", "canonical public repository URL") - usePathStyle := flags.Bool("use-path-style", false, "use path-style S3 requests") - readAuth := flags.String("read-auth", "", "private read authentication: basic") - credentialBroker := flags.String("credential-broker", "", "non-secret credential broker reference") - githubRepository := flags.String("github-repo", "", "GitHub Pages repository (owner/name)") - githubBranch := flags.String("branch", "", "GitHub Pages publish branch") - githubPreviewRepository := flags.String("github-preview-repo", "", "companion preview Pages repository (owner/name)") - githubPreviewBranch := flags.String("preview-branch", "", "preview Pages publish branch") - githubPreviewEndpoint := flags.String("preview-url", "", "companion preview Pages base URL") - suite := flags.String("suite", "stable", "Debian suite") - component := flags.String("component", "main", "Debian component") - defaultArchitectures := "amd64" - if format == "apk" { - // Alpine names the same machine x86_64, and an index under the wrong - // name is one no client looks for. - defaultArchitectures = "x86_64" + if wantsSetupWizard(args[1:], stdin) { + filled, err := setupWizard(format, workspaceFromArguments(args[1:]), stdin, stdout) + if err != nil { + return err + } + // The wizard fills flags and stops. What runs is the command below, on + // the arguments it produced, so there is one code path and the operator + // can see and keep the command that configured their repository. + // + // How the command was invoked was already given and is not the wizard's + // to ask about: --workspace says where this is happening and --json how + // to report it. Those are kept; --interactive is dropped, having done + // its work. + invocation := withoutInteractive(args[1:]) + reportDetail(stdout, "snailmail setup %s %s", format, strings.Join(append(invocation, filled...), " ")) + args = append(append([]string{format}, invocation...), filled...) } - architectures := flags.String("architectures", defaultArchitectures, "comma-separated architectures the repository serves") + flags := newCommandFlags("setup "+format, stderr).withWorkspace().withJSON() + declared := declareSetupFlags(flags, format) // The host is read before parsing only to decide which flags to describe; - // *hostType below is what actually configures anything. + // declared.hostType below is what actually configures anything. flags.set.Usage = func() { writeSetupUsage(stderr, flags.set, format, hostFromArguments(args[1:])) } if err := flags.parse(args[1:]); err != nil { return err } - if err := rejectInapplicableSetupFlags(flags.set, format, *hostType); err != nil { + if err := rejectInapplicableSetupFlags(flags.set, format, *declared.hostType); err != nil { return err } - resolvedApprovalKeys := splitList(*approvalKeys) + resolvedApprovalKeys := splitList(*declared.approvalKeys) sort.Strings(resolvedApprovalKeys) if err := engine.SetupRepository(engine.SetupRepositoryRequest{ - 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, - Target: *sshTarget, Keep: *keep, - Region: *region, Endpoint: *endpoint, CanonicalEndpoint: *canonicalEndpoint, - UsePathStyle: *usePathStyle, - ReadAuth: *readAuth, CredentialBroker: *credentialBroker, - RemoteRepository: *githubRepository, Branch: *githubBranch, PreviewRepository: *githubPreviewRepository, - PreviewBranch: *githubPreviewBranch, PreviewEndpoint: *githubPreviewEndpoint, - Track: *track, Suite: *suite, Component: *component, Architectures: splitList(*architectures), + Root: flags.Root(), Name: *declared.name, Format: format, Output: *declared.output, + HostType: *declared.hostType, Visibility: *declared.visibility, Gate: *declared.gatePolicy, ApprovalKeys: resolvedApprovalKeys, SigningKey: *declared.signingKey, AllowUnsigned: *declared.allowUnsigned, Bucket: *declared.bucket, Prefix: *declared.prefix, + Target: *declared.sshTarget, Keep: *declared.keep, + Region: *declared.region, Endpoint: *declared.endpoint, CanonicalEndpoint: *declared.canonicalEndpoint, + UsePathStyle: *declared.usePathStyle, + ReadAuth: *declared.readAuth, CredentialBroker: *declared.credentialBroker, + RemoteRepository: *declared.githubRepository, Branch: *declared.githubBranch, PreviewRepository: *declared.githubPreviewRepository, + PreviewBranch: *declared.githubPreviewBranch, PreviewEndpoint: *declared.githubPreviewEndpoint, + Track: *declared.track, Suite: *declared.suite, Component: *declared.component, Architectures: splitList(*declared.architectures), }); err != nil { return err } @@ -306,17 +293,17 @@ func runSetup(args []string, stdout, stderr io.Writer) error { // otherwise. Keyed on having an endpoint rather than on a list of host names, // which had left the rsync host — which has a --base-url like any other // remote — reporting a far-side filesystem path nobody types into a browser. - target := *output - if *canonicalEndpoint != "" { - target = *canonicalEndpoint + target := *declared.output + if *declared.canonicalEndpoint != "" { + target = *declared.canonicalEndpoint } - if done, err := flags.emit(stdout, setupResult{Repository: *name, Format: format, Target: target}); done || err != nil { + if done, err := flags.emit(stdout, setupResult{Repository: *declared.name, Format: format, Target: target}); done || err != nil { return err } printBrand(stdout) - reportDone(stdout, "configured %s repository %s", format, *name) + reportDone(stdout, "configured %s repository %s", format, *declared.name) reportDetail(stdout, "desired state will publish to %s", target) - suggestNext(stdout, flags, "snailmail add "+*name+" ./path/to/artifact", + suggestNext(stdout, flags, "snailmail add "+*declared.name+" ./path/to/artifact", "record an artifact") return nil } @@ -328,12 +315,13 @@ func runKeys(ctx context.Context, args []string, stdout, stderr io.Writer) error switch args[0] { case "new": if len(args) < 2 { - return errors.New("usage: snailmail keys new NAME [--algo openpgp-rsa4096] [--expires-in 17520h]") + return errors.New("usage: snailmail keys new NAME [--algo openpgp-rsa4096] [--expires-in 17520h] [--backend file|env]") } name := args[1] flags := newCommandFlags("keys new", stderr).withWorkspace().withJSON() algorithm := flags.String("algo", "openpgp-rsa4096", "signing algorithm") expiresIn := flags.Duration("expires-in", 2*365*24*time.Hour, "key validity duration") + backend := flags.String("backend", "file", "where the private key lives: file, or env for one supplied by the environment") if err := flags.parse(args[2:]); err != nil { return err } @@ -341,7 +329,8 @@ func runKeys(ctx context.Context, args []string, stdout, stderr io.Writer) error if err != nil { return err } - result, err := engine.NewKey(ctx, engine.NewKeyRequest{Root: flags.Root(), Name: name, Algorithm: *algorithm, ExpiresIn: *expiresIn, Keys: store}) + result, err := engine.NewKey(ctx, engine.NewKeyRequest{Root: flags.Root(), Name: name, Algorithm: *algorithm, + Backend: *backend, ExpiresIn: *expiresIn, Keys: store}) if err != nil { return err } @@ -349,7 +338,11 @@ func runKeys(ctx context.Context, args []string, stdout, stderr io.Writer) error return err } printBrand(stdout) - reportDone(stdout, "generated signing key %s", result.Name) + if result.Created { + reportDone(stdout, "generated signing key %s", result.Name) + } else { + reportDone(stdout, "recorded existing signing key %s", result.Name) + } reportDetail(stdout, "fingerprint %s; expires %s", result.Fingerprint, result.ExpiresAt) fmt.Fprintf(stdout, " key reference %s\n", result.Reference) return nil @@ -760,6 +753,7 @@ func runRollout(ctx context.Context, args []string, stdout, stderr io.Writer) er func runStatus(ctx context.Context, args []string, stdout, stderr io.Writer) error { flags := newCommandFlags("status", stderr).withWorkspace().withJSON() + matrix := flags.Bool("matrix", false, "report products against destinations rather than destination by destination") if err := flags.parse(args); err != nil { return err } @@ -770,6 +764,13 @@ func runStatus(ctx context.Context, args []string, stdout, stderr io.Writer) err if done, err := flags.emit(stdout, result); done || err != nil { return err } + if *matrix { + printBrand(stdout) + reportDone(stdout, "workspace %s at %s", result.Workspace, shortDigest(result.GitRevision)) + fmt.Fprintln(stdout) + writeStatusMatrix(stdout, buildStatusMatrix(result)) + return nil + } printBrand(stdout) reportDone(stdout, "workspace %s at %s", result.Workspace, shortDigest(result.GitRevision)) @@ -1608,7 +1609,8 @@ func printUsage(output io.Writer) { "add [--name NAME --version VERSION] REPOSITORY ARTIFACT...", "plan [--out snailmail-plan.json]", "apply [--plan snailmail-plan.json] [--dry-run]", - "status [--workspace DIR] [--json]", + "status [--workspace DIR] [--json] [--matrix]", + "ui [--workspace DIR] (the same matrix, full screen)", }}, {"Curating what is published", []string{ "promote [--track stable] [--distro DISTRO] REPOSITORY PACKAGE VERSION", @@ -1624,7 +1626,7 @@ func printUsage(output io.Writer) { "doctor [--format auto|pypi|deb|helm] [--json] URL", }}, {"Signing and review gates", []string{ - "keys new NAME [--algo openpgp-rsa4096]", + "keys new NAME [--algo openpgp-rsa4096] [--backend file|env]", "keys attach REPOSITORY --key NAME", "keys publish NAME", "keys rotate REPOSITORY --successor KEY [--minimum-refresh 720h]", diff --git a/cmd/snailmail/matrix.go b/cmd/snailmail/matrix.go new file mode 100644 index 0000000..c631e01 --- /dev/null +++ b/cmd/snailmail/matrix.go @@ -0,0 +1,187 @@ +package main + +import ( + "fmt" + "io" + "sort" + "strings" + + "github.com/shellcell/snailmail/engine" +) + +// The status matrix: products down, destinations across. +// +// PLAN §13 calls this the headline view — "Products × destinations" — and says +// it is served by `status --json` and the rendered page, and later by a TUI over +// the same engine. This is that view, and both the one-shot render and the +// interactive one are built from it, so they cannot come to disagree about what +// is published where. +// +// It is derived from engine.StatusWorkspaceResult and nothing else. That result +// is what `status --json` emits, so anything visible here is answerable from the +// JSON, and nothing here reads the workspace a second time. + +// Markers a cell can carry. A published product needs no decoration; the two +// things worth interrupting a reader for are "this is not live" and "this is +// live but something about it is unfinished". +const ( + markerNone = "" + markerUnpublished = "✗" + markerUnfinished = "⚠" +) + +type matrixCell struct { + // text is the version, the number of versions where a destination carries + // several, or an em dash where it carries none. + text string + marker string +} + +type matrixRow struct { + product string + cells []matrixCell +} + +type statusMatrix struct { + destinations []string + rows []matrixRow + // notes explains every marker that actually appears, so the legend describes + // this matrix rather than listing states a reader cannot see. + notes []string +} + +// buildStatusMatrix arranges a status result as products by destinations. +func buildStatusMatrix(result engine.StatusWorkspaceResult) statusMatrix { + matrix := statusMatrix{} + products := map[string]bool{} + for _, repository := range result.Repositories { + matrix.destinations = append(matrix.destinations, repository.Name) + for _, visible := range repository.VisiblePackages { + products[visible.Name] = true + } + } + sort.Strings(matrix.destinations) + + named := make([]string, 0, len(products)) + for product := range products { + named = append(named, product) + } + sort.Strings(named) + + byName := make(map[string]engine.StatusRepository, len(result.Repositories)) + for _, repository := range result.Repositories { + byName[repository.Name] = repository + } + seen := map[string]bool{} + for _, product := range named { + row := matrixRow{product: product} + for _, destination := range matrix.destinations { + cell := cellFor(byName[destination], product) + if cell.marker != markerNone { + seen[cell.marker] = true + } + row.cells = append(row.cells, cell) + } + matrix.rows = append(matrix.rows, row) + } + if seen[markerUnpublished] { + matrix.notes = append(matrix.notes, + markerUnpublished+" in the reviewed state; this destination has published nothing yet") + } + if seen[markerUnfinished] { + matrix.notes = append(matrix.notes, + markerUnfinished+" in the reviewed state; no publication of this destination has included it yet") + } + return matrix +} + +// cellFor says what one destination carries of one product. +// +// Versions are ordered lexically by the engine, which is not version order — +// 0.10.0 sorts below 0.9.0 — so a destination carrying several is reported as a +// count rather than by naming one of them as though it were the current one. +func cellFor(repository engine.StatusRepository, product string) matrixCell { + versions := make([]string, 0, 1) + incomplete := false + for _, visible := range repository.VisiblePackages { + if visible.Name != product { + continue + } + versions = append(versions, visible.Version) + if visible.Binding != "complete" { + incomplete = true + } + } + switch { + case len(versions) == 0: + return matrixCell{text: "—"} + case len(versions) > 1: + return matrixCell{text: fmt.Sprintf("%d versions", len(versions)), marker: productMarker(repository, incomplete)} + } + return matrixCell{text: versions[0], marker: productMarker(repository, incomplete)} +} + +// productMarker decorates one cell. +// +// The two markers say different things and the difference is the point. +// "unrecorded" means no receipt was ever written for this destination, so +// nothing it carries is live. An incomplete binding is per package-version: no +// publication record binds this one, so this product is not live although the +// destination is. +// +// The repository-wide VisibleBindingState is deliberately not consulted. It is +// true when any version is unbound, so reading it here marked every product in +// a destination because one of them was new. +func productMarker(repository engine.StatusRepository, incomplete bool) string { + if repository.Deployment.State == "unrecorded" { + return markerUnpublished + } + if incomplete { + return markerUnfinished + } + return markerNone +} + +// writeStatusMatrix renders the matrix as an aligned table. +func writeStatusMatrix(out io.Writer, matrix statusMatrix) { + if len(matrix.rows) == 0 { + fmt.Fprintln(out, " no products are placed on any destination yet") + return + } + product := len("product") + for _, row := range matrix.rows { + product = max(product, len(row.product)) + } + widths := make([]int, len(matrix.destinations)) + for index, destination := range matrix.destinations { + widths[index] = len(destination) + for _, row := range matrix.rows { + widths[index] = max(widths[index], len([]rune(row.cells[index].text))) + } + } + + var header strings.Builder + fmt.Fprintf(&header, " %-*s", product, "product") + for index, destination := range matrix.destinations { + fmt.Fprintf(&header, " %-*s", widths[index]+2, destination) + } + fmt.Fprintln(out, strings.TrimRight(header.String(), " ")) + + for _, row := range matrix.rows { + var line strings.Builder + fmt.Fprintf(&line, " %-*s", product, row.product) + for index, cell := range row.cells { + // The marker sits after the padded version, so a terminal that gives + // it a double width eats into the gutter rather than the next column. + padded := cell.text + strings.Repeat(" ", widths[index]-len([]rune(cell.text))) + fmt.Fprintf(&line, " %s %-1s", padded, cell.marker) + } + fmt.Fprintln(out, strings.TrimRight(line.String(), " ")) + } + for _, note := range matrix.notes { + fmt.Fprintf(out, "\n %s", note) + } + if len(matrix.notes) != 0 { + fmt.Fprintln(out) + } +} diff --git a/cmd/snailmail/matrix_test.go b/cmd/snailmail/matrix_test.go new file mode 100644 index 0000000..3692c7a --- /dev/null +++ b/cmd/snailmail/matrix_test.go @@ -0,0 +1,143 @@ +package main + +import ( + "strings" + "testing" + + "github.com/shellcell/snailmail/engine" +) + +func destination(name string, deployed bool, packages ...engine.StatusPackage) engine.StatusRepository { + repository := engine.StatusRepository{ + Name: name, VisiblePackages: packages, VisibleBindingState: "complete", + Deployment: engine.StatusDeployment{State: "unrecorded"}, + } + if deployed { + repository.Deployment.State = "recorded" + } + for _, visible := range packages { + if visible.Binding != "complete" { + repository.VisibleBindingState = "incomplete" + } + } + return repository +} + +func published(name, version string) engine.StatusPackage { + return engine.StatusPackage{Name: name, Version: version, Binding: "complete"} +} + +func unbound(name, version string) engine.StatusPackage { + return engine.StatusPackage{Name: name, Version: version, Binding: "incomplete"} +} + +// The matrix is products down and destinations across, which is the whole point +// of it: the repository-by-repository view already exists, and cannot answer +// "where is this product, and at what version" without the reader doing the +// transposition themselves. +func TestTheMatrixPutsOneProductOnOneRow(t *testing.T) { + matrix := buildStatusMatrix(engine.StatusWorkspaceResult{Repositories: []engine.StatusRepository{ + destination("yum", true, published("ttysvg", "0.1.2")), + destination("apt", true, published("ttysvg", "0.1.2"), published("exex", "0.3.2")), + }}) + if strings.Join(matrix.destinations, ",") != "apt,yum" { + t.Fatalf("destinations = %v, want them sorted across", matrix.destinations) + } + if len(matrix.rows) != 2 || matrix.rows[0].product != "exex" || matrix.rows[1].product != "ttysvg" { + t.Fatalf("rows = %+v, want one sorted row per product", matrix.rows) + } + // exex is on apt and not on yum, which is the answer the transposition exists + // to give. + if matrix.rows[0].cells[0].text != "0.3.2" || matrix.rows[0].cells[1].text != "—" { + t.Errorf("exex row = %+v, want it present on apt and absent from yum", matrix.rows[0].cells) + } +} + +// The two markers say different things, and a reader acts differently on each: +// one destination has published nothing, the other has published but not this. +func TestTheMarkersSeparateAnUnpublishedDestinationFromAnUnpublishedProduct(t *testing.T) { + matrix := buildStatusMatrix(engine.StatusWorkspaceResult{Repositories: []engine.StatusRepository{ + destination("never", false, published("ttysvg", "0.1.2")), + destination("live", true, published("ttysvg", "0.1.2"), unbound("exex", "0.3.2")), + }}) + cells := map[string]matrixCell{} + for _, row := range matrix.rows { + for index, destination := range matrix.destinations { + cells[row.product+"/"+destination] = row.cells[index] + } + } + if got := cells["ttysvg/never"].marker; got != markerUnpublished { + t.Errorf("a destination that has published nothing marks its products %q, want %q", got, markerUnpublished) + } + if got := cells["exex/live"].marker; got != markerUnfinished { + t.Errorf("an unbound product marks %q, want %q", got, markerUnfinished) + } + // The one that matters: a live, bound product beside an unbound one is not + // marked. VisibleBindingState is true for the whole destination as soon as + // any version is unbound, so reading it per cell marked everything. + if got := cells["ttysvg/live"].marker; got != markerNone { + t.Errorf("a published product beside an unpublished one is marked %q, want no marker", got) + } +} + +// The legend describes this matrix. A key listing states the reader cannot see +// is noise, and one omitting a marker they can see is worse. +func TestTheLegendExplainsExactlyTheMarkersShown(t *testing.T) { + clean := buildStatusMatrix(engine.StatusWorkspaceResult{Repositories: []engine.StatusRepository{ + destination("apt", true, published("ttysvg", "0.1.2")), + }}) + if len(clean.notes) != 0 { + t.Errorf("a matrix with no markers carries a legend: %v", clean.notes) + } + marked := buildStatusMatrix(engine.StatusWorkspaceResult{Repositories: []engine.StatusRepository{ + destination("apt", true, unbound("exex", "0.3.2")), + }}) + if len(marked.notes) != 1 || !strings.Contains(marked.notes[0], markerUnfinished) { + t.Errorf("legend = %v, want the one marker shown explained", marked.notes) + } +} + +// Versions are ordered lexically by the engine, where 0.10.0 sorts below 0.9.0. +// Naming one of several as though it were the current one would be a guess +// presented as a fact. +func TestSeveralVersionsAreCountedRatherThanPickedFrom(t *testing.T) { + matrix := buildStatusMatrix(engine.StatusWorkspaceResult{Repositories: []engine.StatusRepository{ + destination("apt", true, published("exex", "0.9.0"), published("exex", "0.10.0")), + }}) + cell := matrix.rows[0].cells[0] + if cell.text != "2 versions" { + t.Errorf("cell = %q, want a count rather than one of the versions", cell.text) + } +} + +// The rendered table has to line up, or the reader cannot follow a row across. +func TestTheRenderedTableAligns(t *testing.T) { + var out strings.Builder + writeStatusMatrix(&out, buildStatusMatrix(engine.StatusWorkspaceResult{Repositories: []engine.StatusRepository{ + destination("a-very-long-destination-name", true, published("p", "1.0.0")), + destination("b", true, unbound("a-very-long-product-name", "2.0.0")), + }})) + lines := strings.Split(strings.TrimRight(out.String(), "\n"), "\n") + header := lines[0] + column := strings.Index(header, "b") + if column <= 0 { + t.Fatalf("no second destination column in %q", header) + } + for _, line := range lines[1:] { + if strings.TrimSpace(line) == "" || strings.HasPrefix(strings.TrimSpace(line), markerUnfinished) { + continue + } + if len([]rune(line)) < column { + t.Errorf("row %q is shorter than the column it should reach", line) + } + } +} + +// An empty workspace is a normal state, not a blank screen. +func TestAWorkspaceWithNothingPlacedSaysSo(t *testing.T) { + var out strings.Builder + writeStatusMatrix(&out, buildStatusMatrix(engine.StatusWorkspaceResult{})) + if !strings.Contains(out.String(), "no products") { + t.Errorf("output = %q, want it to say there is nothing placed", out.String()) + } +} diff --git a/cmd/snailmail/rawmode_bsd.go b/cmd/snailmail/rawmode_bsd.go new file mode 100644 index 0000000..ac35a42 --- /dev/null +++ b/cmd/snailmail/rawmode_bsd.go @@ -0,0 +1,10 @@ +//go:build darwin || freebsd + +package main + +import "golang.org/x/sys/unix" + +const ( + getTermios = unix.TIOCGETA + setTermios = unix.TIOCSETA +) diff --git a/cmd/snailmail/rawmode_linux.go b/cmd/snailmail/rawmode_linux.go new file mode 100644 index 0000000..a4c8e20 --- /dev/null +++ b/cmd/snailmail/rawmode_linux.go @@ -0,0 +1,11 @@ +//go:build linux + +package main + +import "golang.org/x/sys/unix" + +// Linux names the termios ioctls differently from the BSDs. +const ( + getTermios = unix.TCGETS + setTermios = unix.TCSETS +) diff --git a/cmd/snailmail/rawmode_other.go b/cmd/snailmail/rawmode_other.go new file mode 100644 index 0000000..2bbe3db --- /dev/null +++ b/cmd/snailmail/rawmode_other.go @@ -0,0 +1,16 @@ +//go:build !linux && !darwin && !freebsd + +package main + +import ( + "errors" + "os" +) + +// Every platform this is distributed for is covered above. Anywhere else the +// one-shot `status --matrix` still works, which is the same view. +func rawTerminal(*os.File) (func(), error) { + return nil, errors.New("a full-screen view is not implemented on this platform; use status --matrix") +} + +func terminalSize(*os.File) (rows, columns int) { return 24, 80 } diff --git a/cmd/snailmail/rawmode_unix.go b/cmd/snailmail/rawmode_unix.go new file mode 100644 index 0000000..4dbd794 --- /dev/null +++ b/cmd/snailmail/rawmode_unix.go @@ -0,0 +1,42 @@ +//go:build linux || darwin || freebsd + +package main + +import ( + "errors" + "os" + + "golang.org/x/sys/unix" +) + +// rawTerminal puts a terminal into the mode a full-screen view needs: keys +// delivered as they are pressed, and not echoed back over the drawing. +// +// It returns what restores the terminal. Nothing else in this file may fail +// without that having run: a process that exits from raw mode leaves the +// operator with a shell that does not echo what they type. +func rawTerminal(file *os.File) (func(), error) { + previous, err := unix.IoctlGetTermios(int(file.Fd()), getTermios) + if err != nil { + return nil, errors.New("ui needs a terminal to draw on; use status --matrix for the same view") + } + raw := *previous + raw.Lflag &^= unix.ECHO | unix.ICANON | unix.ISIG + raw.Iflag &^= unix.IXON | unix.ICRNL + // One key is enough to wake the reader, and it waits rather than spinning. + raw.Cc[unix.VMIN], raw.Cc[unix.VTIME] = 1, 0 + if err := unix.IoctlSetTermios(int(file.Fd()), setTermios, &raw); err != nil { + return nil, err + } + return func() { _ = unix.IoctlSetTermios(int(file.Fd()), setTermios, previous) }, nil +} + +// terminalSize reports the drawing area, falling back to something usable when +// the terminal will not say. +func terminalSize(file *os.File) (rows, columns int) { + size, err := unix.IoctlGetWinsize(int(file.Fd()), unix.TIOCGWINSZ) + if err != nil || size.Row == 0 || size.Col == 0 { + return 24, 80 + } + return int(size.Row), int(size.Col) +} diff --git a/cmd/snailmail/setupflags.go b/cmd/snailmail/setupflags.go index 1210671..7a73dc7 100644 --- a/cmd/snailmail/setupflags.go +++ b/cmd/snailmail/setupflags.go @@ -31,7 +31,7 @@ type flagGroup struct { func setupFlagGroups(format, hostType string) []flagGroup { groups := []flagGroup{{ title: "Repository", - flags: []string{"name", "output", "track", "keep", "visibility"}, + flags: []string{"name", "track", "keep", "visibility"}, }} if selected, err := formats.For(format); err == nil && selected.ImplementsSigning() { @@ -56,6 +56,16 @@ func setupFlagGroups(format, hostType string) []flagGroup { hostFlags := []string{"host", "base-url"} switch hostType { + case "local", "rsync": + // --output is where the tree is written, which only a filesystem + // destination has. An object store is addressed by bucket and prefix, + // and Pages by repository and branch; both refuse a path outright, so + // listing it for them offered a flag that could only ever make the + // repository invalid. The flag's own help has always said so, naming + // local and rsync. + hostFlags = append(hostFlags, "output") + } + switch hostType { case "s3": hostFlags = append(hostFlags, "bucket", "prefix", "region", "endpoint", "use-path-style", "read-auth", "credential-broker") @@ -83,6 +93,7 @@ var setupFlagOwner = map[string]string{ "architectures": "a Debian or Alpine repository", "signing-key": "a format whose repositories snailmail signs", "allow-unsigned": "a format whose repositories snailmail signs", + "output": "the local and rsync hosts, which write to a filesystem", "target": "the rsync host", "bucket": "the s3 host", "prefix": "the s3 host", @@ -168,6 +179,21 @@ func writeSetupUsage(output io.Writer, set *flag.FlagSet, format, hostType strin // text can describe the host being configured. Parsing proper still decides the // value; this only chooses which flags to write about. func hostFromArguments(arguments []string) string { + return namedArgument(arguments, "host", "local") +} + +// workspaceFromArguments finds the workspace root before the flags are parsed, +// which the wizard needs in order to offer the signing keys this workspace +// already holds. +func workspaceFromArguments(arguments []string) string { + root := namedArgument(arguments, "workspace", "") + if root == "" { + root = namedArgument(arguments, "root", ".") + } + return root +} + +func namedArgument(arguments []string, wanted, fallback string) string { for index, argument := range arguments { if argument == "--" { break @@ -176,12 +202,75 @@ func hostFromArguments(arguments []string) string { continue } name := strings.TrimLeft(argument, "-") - if value, found := strings.CutPrefix(name, "host="); found { + if value, found := strings.CutPrefix(name, wanted+"="); found { return value } - if name == "host" && index+1 < len(arguments) { + if name == wanted && index+1 < len(arguments) { return arguments[index+1] } } - return "local" + return fallback +} + +// setupFlags holds what `setup` was given. It exists so the flags are declared +// once and read twice: by the command that acts on them, and by the wizard that +// asks for them. +// +// PLAN §7 makes that a rule rather than a convenience — the wizard is a +// flag-filler over the non-interactive command, "never a second code path". A +// wizard with its own notion of which flags exist would be exactly the parallel +// config writer the rule is written against. +type setupFlags struct { + name, output, hostType, visibility *string + gatePolicy, approvalKeys, signingKey *string + track, sshTarget, bucket, prefix, region *string + endpoint, canonicalEndpoint, readAuth *string + credentialBroker, githubRepository, githubBranch *string + githubPreviewRepository, githubPreviewBranch *string + githubPreviewEndpoint, suite, component *string + architectures *string + allowUnsigned, usePathStyle *bool + keep *int +} + +// declareSetupFlags registers every flag `setup` accepts. +func declareSetupFlags(flags *commandFlags, format string) *setupFlags { + defaultArchitectures := "amd64" + if format == "apk" { + // Alpine names the same machine x86_64, and an index under the wrong + // name is one no client looks for. + defaultArchitectures = "x86_64" + } + return &setupFlags{ + name: flags.String("name", "", "repository name"), + output: flags.String("output", "", "published directory: workspace-relative for a local host, an absolute remote path for rsync"), + // Named for the format being configured. It said "Debian" while applying + // to every format snailmail signs, so an rpm repository was refused for + // want of a flag whose help said it was for something else. + allowUnsigned: flags.Bool("allow-unsigned", false, fmt.Sprintf("explicitly allow a new unsigned %s repository", format)), + hostType: flags.String("host", "local", "host type: local, s3, rsync, or github-pages"), + visibility: flags.String("visibility", "public", "repository visibility"), + gatePolicy: flags.String("gate", "auto", "publication gate: auto, pr, or approval"), + approvalKeys: flags.String("approval-keys", "", "comma-separated allowed Ed25519 public keys"), + signingKey: flags.String("signing-key", "", "repository signing key name"), + track: flags.String("track", "stable", "rendered placement track"), + keep: flags.Int("keep", 0, "publications to retain when collecting; 0 uses the default and can be changed later in snailmail.toml"), + sshTarget: flags.String("target", "", "ssh destination for an rsync host, such as deploy@packages.example"), + bucket: flags.String("bucket", "", "S3 bucket"), + prefix: flags.String("prefix", "", "S3 object prefix"), + region: flags.String("region", "", "AWS region"), + endpoint: flags.String("endpoint", "", "optional S3 API endpoint"), + canonicalEndpoint: flags.String("base-url", "", "canonical public repository URL"), + usePathStyle: flags.Bool("use-path-style", false, "use path-style S3 requests"), + readAuth: flags.String("read-auth", "", "private read authentication: basic"), + credentialBroker: flags.String("credential-broker", "", "non-secret credential broker reference"), + githubRepository: flags.String("github-repo", "", "GitHub Pages repository (owner/name)"), + githubBranch: flags.String("branch", "", "GitHub Pages publish branch"), + githubPreviewRepository: flags.String("github-preview-repo", "", "companion preview Pages repository (owner/name)"), + githubPreviewBranch: flags.String("preview-branch", "", "preview Pages publish branch"), + githubPreviewEndpoint: flags.String("preview-url", "", "companion preview Pages base URL"), + suite: flags.String("suite", "stable", "Debian suite"), + component: flags.String("component", "main", "Debian component"), + architectures: flags.String("architectures", defaultArchitectures, "comma-separated architectures the repository serves"), + } } diff --git a/cmd/snailmail/setupflags_test.go b/cmd/snailmail/setupflags_test.go index e999998..2f29ddb 100644 --- a/cmd/snailmail/setupflags_test.go +++ b/cmd/snailmail/setupflags_test.go @@ -16,22 +16,28 @@ import ( // flag added to the s3 group has to be added here to appear anywhere. func TestSetupFlagsAreScopedToFormatAndHost(t *testing.T) { common := []string{ - "name", "output", "track", "keep", "visibility", + "name", "track", "keep", "visibility", "host", "base-url", "gate", "approval-keys", "json", "workspace", } signing := []string{"signing-key", "allow-unsigned"} + // --output is where a tree is written, which only a filesystem destination + // has. It was listed for every host until the wizard asked for it on a Pages + // repository and setup refused the answer: an object store and Pages both + // reject a path outright, so offering it there could only produce an invalid + // repository. + filesystem := []string{"output"} for _, testcase := range []struct { format string host string extra []string }{ - {"pypi", "local", nil}, - {"raw", "local", nil}, - {"helm", "local", signing}, - {"deb", "local", append(append([]string{}, signing...), "suite", "component", "architectures")}, - {"apk", "local", append(append([]string{}, signing...), "architectures")}, - {"rpm", "rsync", append(append([]string{}, signing...), "target")}, + {"pypi", "local", filesystem}, + {"raw", "local", filesystem}, + {"helm", "local", append(append([]string{}, signing...), filesystem...)}, + {"deb", "local", append(append([]string{}, signing...), "output", "suite", "component", "architectures")}, + {"apk", "local", append(append([]string{}, signing...), "output", "architectures")}, + {"rpm", "rsync", append(append([]string{}, signing...), "output", "target")}, {"pypi", "s3", []string{ "bucket", "prefix", "region", "endpoint", "use-path-style", "read-auth", "credential-broker", diff --git a/cmd/snailmail/setupwizard.go b/cmd/snailmail/setupwizard.go new file mode 100644 index 0000000..7982527 --- /dev/null +++ b/cmd/snailmail/setupwizard.go @@ -0,0 +1,252 @@ +package main + +import ( + "bufio" + "errors" + "fmt" + "io" + "os" + "sort" + "strings" + + "github.com/shellcell/snailmail/formats" + "github.com/shellcell/snailmail/host" + "github.com/shellcell/snailmail/internal/state" +) + +// The setup wizard. +// +// PLAN.md §7 states the rule this file exists to obey: every prompt is also a +// flag, and the wizard is a flag-filler over the non-interactive command, never +// a second code path. "Without this the wizard is untestable, unusable in CI, +// and will eventually grow a parallel, subtly different config writer." +// +// So it is taken literally. The only thing this produces is an argument list, +// which runSetup then parses exactly as if it had been typed. There is nowhere +// for a second config writer to appear, because nothing here writes anything. +// It also means the wizard can show the command it is about to run, which is +// the command to paste into CI. +// +// The questions come from setupFlagGroups, which the help text is already +// rendered from, and their prompts and defaults come from the registered flags +// themselves. A flag added to setup is therefore asked about without anyone +// remembering to add it here, and cannot be described one way in --help and +// another way in the wizard. + +// interactiveFlag forces the wizard on. Without it the wizard engages only when +// setup was given nothing to work with and someone is there to answer, so a +// script that calls setup wrongly still gets an error instead of hanging on a +// prompt nobody will ever read. +const interactiveFlag = "--interactive" + +// wantsSetupWizard reports whether `setup ` should ask rather than fail. +func wantsSetupWizard(arguments []string, in io.Reader) bool { + for _, argument := range arguments { + if argument == interactiveFlag || argument == "-interactive" { + return true + } + } + return len(arguments) == 0 && isTerminal(in) +} + +// isTerminal reports whether input comes from someone typing. +func isTerminal(in io.Reader) bool { + file, ok := in.(*os.File) + if !ok { + return false + } + info, err := file.Stat() + return err == nil && info.Mode()&os.ModeCharDevice != 0 +} + +// choicesFor lists the values a flag accepts, where they are enumerable. A flag +// with no entry is asked as free text with its default offered. +// +// These come from the same declarations the validator rejects against, so a +// wizard cannot offer a value that setup then refuses. +func choicesFor(flagName, root string) []string { + switch flagName { + case "host": + return host.KnownHostTypes() + case "visibility": + return []string{"public", "private"} + case "gate": + return []string{"auto", "pr", "approval"} + case "signing-key": + return signingKeyChoices(root) + } + return nil +} + +// signingKeyChoices lists the keys this workspace already holds. A repository +// references a key by name, and `keys new` is what creates one, so the wizard +// offers what exists rather than pretending it can generate one as a side +// effect of configuring a repository. +func signingKeyChoices(root string) []string { + manifest, err := state.LoadManifest(root) + if err != nil { + return nil + } + names := make([]string, 0, len(manifest.Keys)) + for name := range manifest.Keys { + names = append(names, name) + } + sort.Strings(names) + return names +} + +// setupWizard asks for the flags that apply and returns them as arguments. +func setupWizard(format, root string, in io.Reader, out io.Writer) ([]string, error) { + if _, err := formats.For(format); err != nil { + return nil, err + } + reader := bufio.NewReader(in) + fmt.Fprintf(out, "\nConfiguring a %s repository. Enter accepts the default in [brackets].\n", format) + + // The host decides which of the remaining flags exist at all, so it is asked + // first and the rest of the questions are chosen from the answer. + // + // These are the flags setup itself declares, registered into a set that is + // never parsed. Only their names, help and defaults are read, so a flag + // cannot be described one way by --help and another way here. + registered := newCommandFlags("setup "+format, io.Discard) + declareSetupFlags(registered, format) + hostType, err := ask(reader, out, registered, "host", "local", root) + if err != nil { + return nil, err + } + + arguments := []string(nil) + if hostType != registered.set.Lookup("host").DefValue { + arguments = append(arguments, "--host="+hostType) + } + for _, group := range setupFlagGroups(format, hostType) { + if group.title == "Output" { + // --json and --workspace are how the command is invoked, not what the + // repository is. The workspace is already known: it is where we are. + continue + } + fmt.Fprintf(out, "\n%s\n", group.title) + for _, flagName := range group.flags { + if flagName == "host" { + continue // asked above, to choose this group set + } + value, err := ask(reader, out, registered, flagName, "", root) + if err != nil { + return nil, err + } + if value == "" || value == registered.set.Lookup(flagName).DefValue { + continue + } + arguments = append(arguments, "--"+flagName+"="+value) + } + } + return arguments, nil +} + +// ask puts one flag to the operator and returns what they chose. +// +// An empty answer means "leave it out", which lets the flag's own default +// apply rather than restating it in the argument list -- so the command the +// wizard prints is the shortest one that produces this configuration. +func ask(reader *bufio.Reader, out io.Writer, registered *commandFlags, flagName, override, root string) (string, error) { + declared := registered.set.Lookup(flagName) + if declared == nil { + return "", fmt.Errorf("setup has no --%s to ask about", flagName) + } + fallback := declared.DefValue + if override != "" { + fallback = override + } + choices := choicesFor(flagName, root) + if len(choices) > 0 { + return askChoice(reader, out, flagName, declared.Usage, choices, fallback) + } + if declared.DefValue == "false" || declared.DefValue == "true" { + return askBool(reader, out, flagName, declared.Usage, declared.DefValue) + } + fmt.Fprintf(out, " %-18s %s\n", flagName, declared.Usage) + if fallback != "" && fallback != "0" { + fmt.Fprintf(out, " %-18s [%s] ", "", fallback) + } else { + fmt.Fprintf(out, " %-18s ", "") + } + answer, err := readLine(reader, out) + if err != nil { + return "", err + } + return answer, nil +} + +func askChoice(reader *bufio.Reader, out io.Writer, flagName, usage string, choices []string, fallback string) (string, error) { + fmt.Fprintf(out, " %-18s %s\n", flagName, usage) + for index, choice := range choices { + marker := " " + if choice == fallback { + marker = "▸" + } + fmt.Fprintf(out, " %-18s %s %d) %s\n", "", marker, index+1, choice) + } + fmt.Fprintf(out, " %-18s [%s] ", "", fallback) + answer, err := readLine(reader, out) + if err != nil { + return "", err + } + if answer == "" { + return fallback, nil + } + // A number for convenience, the value itself because that is what the flag + // takes and what the printed command will show. + for index, choice := range choices { + if answer == choice || answer == fmt.Sprint(index+1) { + return choice, nil + } + } + return "", fmt.Errorf("--%s does not accept %q; choose one of %s", flagName, answer, strings.Join(choices, ", ")) +} + +func askBool(reader *bufio.Reader, out io.Writer, flagName, usage, fallback string) (string, error) { + fmt.Fprintf(out, " %-18s %s\n", flagName, usage) + fmt.Fprintf(out, " %-18s [%s] (yes/no) ", "", fallback) + answer, err := readLine(reader, out) + if err != nil { + return "", err + } + switch strings.ToLower(answer) { + case "": + if fallback == "true" { + return "true", nil + } + return "", nil + case "y", "yes", "true": + return "true", nil + case "n", "no", "false": + return "", nil + } + return "", fmt.Errorf("--%s takes yes or no, not %q", flagName, answer) +} + +func readLine(reader *bufio.Reader, out io.Writer) (string, error) { + line, err := reader.ReadString('\n') + fmt.Fprintln(out) + if err != nil && (err != io.EOF || line == "") { + if errors.Is(err, io.EOF) { + return "", errors.New("input ended before the wizard had finished asking") + } + return "", err + } + return strings.TrimSpace(line), nil +} + +// withoutInteractive drops the flag that asked for the wizard, leaving the rest +// of the invocation for the command the wizard fills in for. +func withoutInteractive(arguments []string) []string { + kept := make([]string, 0, len(arguments)) + for _, argument := range arguments { + if argument == interactiveFlag || argument == "-interactive" { + continue + } + kept = append(kept, argument) + } + return kept +} diff --git a/cmd/snailmail/setupwizard_test.go b/cmd/snailmail/setupwizard_test.go new file mode 100644 index 0000000..c492f8c --- /dev/null +++ b/cmd/snailmail/setupwizard_test.go @@ -0,0 +1,175 @@ +package main + +import ( + "bytes" + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// wizardWorkspace is an initialised workspace to configure a repository in. +func wizardWorkspace(t *testing.T) string { + t.Helper() + root := t.TempDir() + if output, err := exec.Command("git", "-C", root, "init").CombinedOutput(); err != nil { + t.Fatalf("git init: %v: %s", err, output) + } + var out bytes.Buffer + if err := run(context.Background(), []string{"init", "--name", "wizard", "--workspace", root}, &out, &out); err != nil { + t.Fatalf("init: %v: %s", err, out.String()) + } + return root +} + +// answers scripts a wizard session. Each line is one answer, in the order the +// questions are asked; an empty line takes the default. +func answers(lines ...string) string { return strings.Join(lines, "\n") + "\n" } + +// The rule this whole file exists to keep: the wizard fills flags, and the +// command it prints is the command it ran. +// +// PLAN §7 requires the wizard to be a flag-filler over the non-interactive +// command, "never a second code path", because otherwise it "will eventually +// grow a parallel, subtly different config writer". A repository configured by +// answering questions and one configured by typing the printed command have to +// be the same repository, or that has already happened. +func TestTheWizardsCommandConfiguresWhatTheWizardConfigured(t *testing.T) { + asked := wizardWorkspace(t) + var session bytes.Buffer + // host, name, track, keep, visibility, base-url, output, gate, approval-keys + input := strings.NewReader(answers( + "local", "tools", "", "", "", "https://packages.example/tools", "public/tools", "", "")) + if err := runWithInput(context.Background(), + []string{"setup", "raw", "--interactive", "--workspace", asked}, input, &session, &session); err != nil { + t.Fatalf("wizard: %v\n%s", err, session.String()) + } + printed := printedSetupCommand(t, session.String()) + + typed := wizardWorkspace(t) + var direct bytes.Buffer + if err := run(context.Background(), append(printed, "--workspace", typed), &direct, &direct); err != nil { + t.Fatalf("the command the wizard printed does not run: %v\n%s", err, direct.String()) + } + askedManifest, typedManifest := readWorkspaceManifest(t, asked), readWorkspaceManifest(t, typed) + if askedManifest != typedManifest { + t.Errorf("answering questions and typing the printed command configured different repositories:\n%s\n%s", + askedManifest, typedManifest) + } + if !strings.Contains(askedManifest, "packages.example") { + t.Errorf("the answers were not applied at all:\n%s", askedManifest) + } +} + +// printedSetupCommand recovers the command the wizard reported, as arguments. +func printedSetupCommand(t *testing.T, session string) []string { + t.Helper() + for _, line := range strings.Split(session, "\n") { + index := strings.Index(line, "snailmail setup ") + if index < 0 { + continue + } + return strings.Fields(strings.TrimPrefix(line[index:], "snailmail ")) + } + t.Fatalf("the wizard printed no command:\n%s", session) + return nil +} + +func readWorkspaceManifest(t *testing.T, root string) string { + t.Helper() + content, err := os.ReadFile(filepath.Join(root, "snailmail.toml")) + if err != nil { + t.Fatal(err) + } + // The workspace identity differs between two fresh workspaces and says + // nothing about how the repository was configured. + var kept []string + for _, line := range strings.Split(string(content), "\n") { + if strings.HasPrefix(strings.TrimSpace(line), "id = ") { + continue + } + kept = append(kept, line) + } + return strings.Join(kept, "\n") +} + +// The wizard asks about the flags that apply and no others, which is the same +// question --help answers. A wizard asking for a bucket on a local host would +// be collecting a value setup then refuses by name. +func TestTheWizardAsksOnlyWhatTheChosenHostUses(t *testing.T) { + root := wizardWorkspace(t) + var session bytes.Buffer + input := strings.NewReader(answers("local", "tools", "", "", "", "", "public/tools", "", "")) + if err := runWithInput(context.Background(), + []string{"setup", "raw", "--interactive", "--workspace", root}, input, &session, &session); err != nil { + t.Fatalf("wizard: %v\n%s", err, session.String()) + } + transcript := session.String() + for _, absent := range []string{"bucket", "github-repo", "preview-branch", "target"} { + if strings.Contains(transcript, " "+absent+" ") { + t.Errorf("a local host was asked for --%s, which belongs to another host:\n%s", absent, transcript) + } + } + for _, present := range []string{"name", "output", "visibility", "gate"} { + if !strings.Contains(transcript, " "+present+" ") { + t.Errorf("the wizard never asked for --%s:\n%s", present, transcript) + } + } +} + +// An answer outside the accepted set is refused where it was given, naming what +// would have worked, rather than being carried to a validator that talks about +// stored configuration. +func TestAnUnacceptableAnswerIsRefusedWithItsAlternatives(t *testing.T) { + root := wizardWorkspace(t) + var session bytes.Buffer + input := strings.NewReader(answers("local", "tools", "", "", "somewhere-else")) + err := runWithInput(context.Background(), + []string{"setup", "raw", "--interactive", "--workspace", root}, input, &session, &session) + if err == nil { + t.Fatal("an invalid visibility was accepted") + } + if !strings.Contains(err.Error(), "public") || !strings.Contains(err.Error(), "private") { + t.Errorf("error = %v, want the accepted values named", err) + } +} + +// A script that calls setup wrongly has to fail, not wait forever for an answer +// nobody is there to give. Only an explicit --interactive, or a real terminal, +// starts the wizard. +func TestSetupWithoutSomeoneToAskStillFails(t *testing.T) { + // Bare `setup raw` in the workspace, which is exactly how a script reaches + // the wizard's trigger by accident. With no terminal attached there is + // nobody to answer, so it has to fail rather than block on a prompt. + t.Chdir(wizardWorkspace(t)) + for _, arguments := range [][]string{{"setup", "raw"}, {"setup", "raw", "--host", "local"}} { + var out bytes.Buffer + err := run(context.Background(), arguments, &out, &out) + if err == nil { + t.Fatalf("%v succeeded with nothing configured", arguments) + } + if strings.Contains(out.String(), "Enter accepts the default") { + t.Errorf("%v started the wizard with nothing attached to answer it:\n%s", arguments, out.String()) + } + } +} + +// Every question the wizard asks is a flag setup declares. This is the property +// that makes it a flag-filler rather than a second interface: a question with no +// flag behind it could only be answered into something the command cannot take. +func TestEveryQuestionTheWizardAsksIsAFlag(t *testing.T) { + registered := newCommandFlags("setup deb", &bytes.Buffer{}).withWorkspace().withJSON() + declareSetupFlags(registered, "deb") + for _, hostType := range []string{"local", "s3", "rsync", "github-pages"} { + for _, group := range setupFlagGroups("deb", hostType) { + for _, flagName := range group.flags { + if registered.set.Lookup(flagName) == nil { + t.Errorf("the %s group asks for --%s on the %s host, which setup does not declare", + group.title, flagName, hostType) + } + } + } + } +} diff --git a/cmd/snailmail/ui.go b/cmd/snailmail/ui.go new file mode 100644 index 0000000..b1beed2 --- /dev/null +++ b/cmd/snailmail/ui.go @@ -0,0 +1,146 @@ +package main + +import ( + "context" + "errors" + "fmt" + "io" + "os" + "strings" + + "github.com/shellcell/snailmail/engine" +) + +// `snailmail ui`: the status matrix, full screen. +// +// PLAN §13 lists `ui` as a planned command and calls the matrix the headline +// view, "served today by `status --json` and the rendered page, and later by a +// TUI over the same engine". The engine part is meant literally — this reads +// engine.StatusWorkspace and renders buildStatusMatrix, which is what +// `status --matrix` prints. There is one matrix and two ways to look at it. +// +// It is deliberately a viewer and not a console. Everything that changes a +// workspace is a reviewed diff and a plan, and a keystroke is neither; offering +// "publish" here would be a second way to reach the irreversible half of the +// tool, without the review that makes it safe. +func runUI(ctx context.Context, args []string, stdin io.Reader, stdout, stderr io.Writer) error { + flags := newCommandFlags("ui", stderr).withWorkspace() + if err := flags.parse(args); err != nil { + return err + } + input, ok := stdin.(*os.File) + if !ok { + return errors.New("ui needs a terminal to draw on; use status --matrix for the same view") + } + output, ok := stdout.(*os.File) + if !ok { + return errors.New("ui needs a terminal to draw on; use status --matrix for the same view") + } + restore, err := rawTerminal(input) + if err != nil { + return err + } + // Everything below has to reach this. A process that leaves raw mode set + // hands the operator a shell that does not echo what they type. + defer restore() + + fmt.Fprint(output, enterFullScreen) + defer fmt.Fprint(output, leaveFullScreen) + + result, err := engine.StatusWorkspace(ctx, engine.StatusWorkspaceRequest{Root: flags.Root()}) + if err != nil { + return err + } + for { + rows, columns := terminalSize(output) + fmt.Fprint(output, drawStatusScreen(result, rows, columns)) + key, err := readKey(input) + if err != nil { + return err + } + switch key { + case 'q', 'Q', 0x03, 0x04: // q, Ctrl-C, Ctrl-D + return nil + case 'r', 'R': + refreshed, err := engine.StatusWorkspace(ctx, engine.StatusWorkspaceRequest{Root: flags.Root()}) + if err != nil { + return err + } + result = refreshed + } + if err := ctx.Err(); err != nil { + return nil + } + } +} + +// Terminal control. The alternate screen is what lets the view leave the +// scrollback it found untouched. +const ( + enterFullScreen = "\x1b[?1049h\x1b[?25l" + leaveFullScreen = "\x1b[?25h\x1b[?1049l" + clearScreen = "\x1b[H\x1b[2J" +) + +// drawStatusScreen renders one frame. +// +// It returns the frame rather than writing it, so the whole of what the view +// puts on screen is a pure function of the status result and the terminal size +// — which is what makes it testable without a terminal. +func drawStatusScreen(result engine.StatusWorkspaceResult, rows, columns int) string { + heading := []string{fmt.Sprintf(" snailmail — %s at %s", result.Workspace, shortDigest(result.GitRevision))} + if len(result.Uncommitted) != 0 { + heading = append(heading, fmt.Sprintf(" %d uncommitted %s, not shown below", + len(result.Uncommitted), plural(len(result.Uncommitted), "change", "changes"))) + } + heading = append(heading, "") + footer := []string{"", " r refresh q quit"} + + var body strings.Builder + writeStatusMatrix(&body, buildStatusMatrix(result)) + lines := strings.Split(strings.TrimRight(body.String(), "\n"), "\n") + // What does not fit says so rather than scrolling away above the top of the + // screen, where a reader would take its absence for an answer. + room := rows - len(heading) - len(footer) + if room < 1 { + room = 1 + } + if len(lines) > room { + lines = append(lines[:room-1], fmt.Sprintf(" … %d more %s", + len(lines)-room+1, plural(len(lines)-room+1, "line", "lines"))) + } + + // Every line the frame emits goes through the same truncation. Doing it only + // to the matrix left the heading to wrap on a narrow terminal, which pushes + // the whole frame down by a line it did not account for. + screen := strings.Builder{} + screen.WriteString(clearScreen) + for _, line := range append(append(heading, lines...), footer...) { + screen.WriteString(truncateToWidth(line, columns) + "\r\n") + } + return screen.String() +} + +// truncateToWidth keeps a line inside the terminal, counting runes rather than +// bytes so a marker does not cost four columns. +func truncateToWidth(line string, columns int) string { + runes := []rune(line) + if columns <= 1 || len(runes) <= columns { + return line + } + return string(runes[:columns-1]) + "…" +} + +// readKey waits for one keystroke. +func readKey(input *os.File) (byte, error) { + buffer := make([]byte, 1) + for { + read, err := input.Read(buffer) + if err != nil { + return 0, err + } + if read == 1 { + return buffer[0], nil + } + } +} diff --git a/cmd/snailmail/ui_test.go b/cmd/snailmail/ui_test.go new file mode 100644 index 0000000..2f30c6b --- /dev/null +++ b/cmd/snailmail/ui_test.go @@ -0,0 +1,103 @@ +package main + +import ( + "strings" + "testing" + + "github.com/shellcell/snailmail/engine" +) + +func uiResult(repositories ...engine.StatusRepository) engine.StatusWorkspaceResult { + return engine.StatusWorkspaceResult{ + Workspace: "packages", GitRevision: "abcdef0123456789", Repositories: repositories, + } +} + +// The full-screen view and `status --matrix` are two ways of looking at one +// matrix, which is what PLAN §13 means by a view over the same engine. If they +// could disagree there would be two answers to where a product is published. +func TestTheFullScreenViewShowsTheSameMatrixAsTheOneShot(t *testing.T) { + result := uiResult( + destination("apt", true, published("ttysvg", "0.1.2")), + destination("yum", false, published("exex", "0.3.2")), + ) + var oneShot strings.Builder + writeStatusMatrix(&oneShot, buildStatusMatrix(result)) + screen := drawStatusScreen(result, 40, 100) + for _, line := range strings.Split(strings.TrimRight(oneShot.String(), "\n"), "\n") { + if strings.TrimSpace(line) == "" { + continue + } + if !strings.Contains(screen, line) { + t.Errorf("the full-screen view is missing a row the one-shot shows:\n%q\n%s", line, screen) + } + } +} + +// A terminal too short for the matrix has to say what it is not showing. +// Silently dropping rows would make the view answer "where is this product" +// with a confident nothing. +func TestAScreenTooShortSaysWhatItIsNotShowing(t *testing.T) { + repositories := []engine.StatusRepository{destination("apt", true, + published("a", "1"), published("b", "1"), published("c", "1"), published("d", "1"), published("e", "1"))} + screen := drawStatusScreen(uiResult(repositories...), 9, 80) + if !strings.Contains(screen, "more line") { + t.Errorf("a screen that could not fit the matrix did not say so:\n%s", screen) + } + if strings.Contains(screen, "\n e ") { + t.Errorf("a row past the bottom of the screen was drawn anyway:\n%s", screen) + } +} + +// A narrow terminal must not wrap a row, which would put half of one product's +// destinations on the next line and read as another product. +func TestANarrowScreenTruncatesRatherThanWraps(t *testing.T) { + repositories := []engine.StatusRepository{destination("a-destination-with-a-long-name", true, + published("a-product-with-a-long-name", "1.0.0"))} + const columns = 30 + for _, line := range strings.Split(drawStatusScreen(uiResult(repositories...), 40, columns), "\r\n") { + if len([]rune(stripEscapes(line))) > columns { + t.Errorf("line is %d columns wide in a %d column terminal: %q", + len([]rune(stripEscapes(line))), columns, line) + } + } +} + +// stripEscapes removes the terminal control sequences a frame carries, leaving +// what the reader sees. +func stripEscapes(line string) string { + var kept strings.Builder + for index := 0; index < len(line); index++ { + if line[index] == 0x1b { + for index < len(line) && !strings.ContainsRune("ABCDEFGHJKSTfmnhl", rune(line[index])) { + index++ + } + continue + } + kept.WriteByte(line[index]) + } + return kept.String() +} + +// The view leaves the scrollback it found, and gives the cursor back. A viewer +// that keeps a hidden cursor hands the operator a shell they cannot see. +func TestTheViewRestoresWhatItTookOver(t *testing.T) { + if !strings.Contains(enterFullScreen, "?1049h") || !strings.Contains(leaveFullScreen, "?1049l") { + t.Error("the view does not use the alternate screen, so it overwrites the scrollback") + } + if !strings.Contains(enterFullScreen, "?25l") || !strings.Contains(leaveFullScreen, "?25h") { + t.Error("the view hides the cursor without restoring it") + } +} + +// Uncommitted work changes what the numbers mean, and the reason `status` says +// so first is that a reader comparing this against their working tree needs to +// know before they read it. +func TestTheViewSaysWhenTheWorkspaceIsNotTidy(t *testing.T) { + result := uiResult(destination("apt", true, published("ttysvg", "0.1.2"))) + result.Uncommitted = []string{"repos/apt.lock.toml"} + screen := drawStatusScreen(result, 40, 100) + if !strings.Contains(screen, "uncommitted") { + t.Errorf("the view did not report uncommitted work:\n%s", screen) + } +} diff --git a/docs/signing.md b/docs/signing.md index 9dee984..833c0e7 100644 --- a/docs/signing.md +++ b/docs/signing.md @@ -43,6 +43,57 @@ repositories remain readable but `keys audit` reports them as errors. New unsigned Debian setup requires the explicit `--allow-unsigned` compatibility opt-out. +## Signing where the key comes from somewhere else + +The file backend owns a key: it generates one, writes it under your data +directory, and is the only thing that can produce it again. That is the right +shape for a workstation and the wrong one for CI, where the runner is discarded +after every job and the key has to arrive from a secret store that already holds +it. + +The `env` backend covers that. It generates nothing; it resolves a key that +already exists, in the same encrypted armored form the file backend writes. +Create the key once on a workstation, put it in your secret store, and record it +in the workspace: + +```sh +snailmail keys new archive-signing --backend env +``` + +The material is read from `SNAILMAIL_KEY_`, or from a file named by +`SNAILMAIL_KEY__FILE`. The name is the key's, uppercased, with hyphens +mapped to underscores — `archive-signing` becomes `SNAILMAIL_KEY_ARCHIVE_SIGNING`. +Setting both is rejected rather than resolved silently, and the file form is +preferred for the same reason it is for the passphrase: a container's +environment stays readable through the runtime's inspection API, whereas a file +can be read once and removed. + +The passphrase is supplied exactly as it is for the file backend. Both secrets +have to reach the job, and neither is written to disk by snailmail. + +```yaml +- name: Publish + env: + SNAILMAIL_KEY_PASSPHRASE: ${{ secrets.SNAILMAIL_KEY_PASSPHRASE }} + SNAILMAIL_KEY_ARCHIVE_SIGNING: ${{ secrets.SNAILMAIL_ARCHIVE_SIGNING_KEY }} + run: | + snailmail plan + snailmail apply +``` + +Two things this backend deliberately will not do. It cannot generate a key, +because a key it can see is one that already existed — `keys new --backend env` +records what the environment supplies and fails if nothing does. And it cannot +delete one: unsetting the variable is the environment's business, and reporting +success would let a rotation believe it had retired a key that every job still +receives. + +The variable is named after the key, not after the workspace, so one runner +serving two workspaces with a same-named key would hand both the same material. +Give each job only the keys it needs. If it does go wrong, planning refuses: +resolving a key whose fingerprint differs from the committed public form reports +that the private signer identity differs from it. + Every format the knowledge bundle records as signable is signed, each in the form its own clients read: Debian publishes `InRelease` and `Release.gpg`, yum an armored key beside a detached `repomd.xml.asc`, Alpine one signature per diff --git a/engine/clientverify.go b/engine/clientverify.go index 06143ee..6f81afd 100644 --- a/engine/clientverify.go +++ b/engine/clientverify.go @@ -79,6 +79,14 @@ var clientVerifiers = map[string]clientVerifier{ }) return result.Manifest, err }, + endpoint: func(ctx context.Context, staged string, access host.ClientAccess, request ApplyWorkspaceRequest) error { + image := request.HelmImage + if image == "" { + image = DefaultHelmVerificationImage + } + _, _, err := app.VerifyHelmClientEndpointAccess(ctx, staged, access, request.Runner, image) + return err + }, }, "raw": { staged: func(_ context.Context, repository string, _ ApplyWorkspaceRequest) (buildgraph.RepositoryManifest, error) { @@ -98,6 +106,15 @@ var clientVerifiers = map[string]clientVerifier{ }) return result.Manifest, err }, + endpoint: func(ctx context.Context, staged string, access host.ClientAccess, request ApplyWorkspaceRequest) error { + image := request.RPMImage + if image == "" { + image = DefaultRPMVerificationImage + } + _, _, err := app.VerifyRPMClientEndpointAccess(ctx, staged, access, request.Runner, image, + versionScope(request.VerifyAllVersions)) + return err + }, }, "apk": { staged: func(ctx context.Context, repository string, request ApplyWorkspaceRequest) (buildgraph.RepositoryManifest, error) { @@ -107,6 +124,15 @@ var clientVerifiers = map[string]clientVerifier{ }) return result.Manifest, err }, + endpoint: func(ctx context.Context, staged string, access host.ClientAccess, request ApplyWorkspaceRequest) error { + image := request.APKImage + if image == "" { + image = DefaultAPKVerificationImage + } + _, _, err := app.VerifyAPKClientEndpointAccess(ctx, staged, access, request.Runner, image, + versionScope(request.VerifyAllVersions)) + return err + }, }, } diff --git a/engine/endpoint_support_test.go b/engine/endpoint_support_test.go index ebe7c1e..f44fdea 100644 --- a/engine/endpoint_support_test.go +++ b/engine/endpoint_support_test.go @@ -5,36 +5,82 @@ import ( "strings" "testing" + "github.com/shellcell/snailmail/formats" "github.com/shellcell/snailmail/host" "github.com/shellcell/snailmail/internal/state" ) -// host.Supports declares which formats a remote host can client-verify, and -// verifyEndpointClient is what actually does it. Nothing held the two together, -// so a format could be declared verifiable and route to a default that refuses — -// which rpm and apk on Pages did, turning a working repository into a failing -// one the moment a preview site was configured. +// Publication without client verification would let a remote host commit bytes +// no official client ever installed, which is the guarantee the tool sells. So +// every format a remote host publishes has to reach a client, and this is the +// only package that can check it: host declares which pairs are served, formats +// declares which of them a client can be run against a URL for, and neither can +// see the other. // -// A format without a served-endpoint probe now falls back to verifying the -// staged tree, so no declared pair reaches that refusal. The call below cannot -// succeed — there is no staged tree and no endpoint — but it distinguishes "this -// format has no implementation at all" from any other failure. -func TestNoDeclaredFormatIsRefusedOutright(t *testing.T) { +// The matrix used to restate the second fact per pair, as +// RemoteClientVerification, and it drifted: eight pairs claimed an +// endpoint-served probe their format did not implement. Nothing caught it, +// because the two declarations sat in packages that cannot import each other. +// The fact now lives once, on the format, and this is where the two meet. +// +// The route each pair takes is logged, because which formats are verified only +// against the staged tree — never against what the host actually serves — is +// the coverage gap that is easy to forget and hard to see. +func TestEveryRemotelyPublishedFormatReachesAClient(t *testing.T) { + probes := make(map[string]bool) + for _, format := range formats.All() { + probes[format.Name()] = format.HasEndpointProbe() + } + checked := 0 for _, hostType := range host.KnownHostTypes() { if hostType == "local" { continue } for _, format := range host.SupportedFormats(hostType) { - if !host.Supports(hostType, format).RemoteClientVerification { - continue + checked++ + route := "the staged tree" + if probes[format] { + route = "an endpoint probe" } + t.Logf("%s on %s is verified by %s", format, hostType, route) + // The call cannot succeed: there is no staged tree and no host serving + // the endpoint. It distinguishes "this format has no implementation at + // all" from every other failure, which is what would regress here. err := verifyEndpointClient(context.Background(), state.Repository{Format: format, Host: state.HostConfig{Type: hostType}}, t.TempDir(), host.ClientAccess{Endpoint: "https://example.test/repo"}, ApplyWorkspaceRequest{Hosts: localHosts(), StructuralOnly: true}) - if err != nil && strings.Contains(err.Error(), "client verification is not implemented") { - t.Errorf("host %q declares %q verifiable, but the engine has no probe: %v", hostType, format, err) + if err == nil { + continue + } + // Every way the engine can say it has no client for this pair. A + // format declared in the matrix but absent from the verifier registry + // reaches the third of these, which is why it is listed: the matrix is + // edited by hand and a format can be added to it before it exists. + for _, refusal := range []string{"not implemented", "does not serve", "unsupported repository format"} { + if strings.Contains(err.Error(), refusal) { + t.Errorf("host %q publishes %q, but nothing verifies it: %v", hostType, format, err) + } } } } + if checked == 0 { + t.Fatal("no remote host published anything, so this test checked nothing") + } +} + +// A format the matrix does not know is not silently verifiable. Publishing is +// what gates the attempt now, so a pair no host declares has to be refused +// before any client runs rather than reaching one by omission. +func TestAnUndeclaredPairIsRefusedBeforeAnyClientRuns(t *testing.T) { + err := verifyEndpointClient(context.Background(), + state.Repository{Format: "deb", Host: state.HostConfig{Type: "s3"}}, + t.TempDir(), host.ClientAccess{Endpoint: "https://example.test/repo"}, + ApplyWorkspaceRequest{Hosts: localHosts(), StructuralOnly: true}) + if err == nil { + t.Fatal("an object store was allowed to client-verify Debian, which it cannot serve") + } + if !strings.Contains(err.Error(), "does not serve") { + t.Errorf("error = %v, want the pair named as unserved", err) + } } diff --git a/engine/engine_test.go b/engine/engine_test.go index b5a6205..d14c9ee 100644 --- a/engine/engine_test.go +++ b/engine/engine_test.go @@ -9,6 +9,8 @@ import ( "path/filepath" "reflect" "runtime" + "strings" + "sync" "testing" "time" @@ -431,13 +433,48 @@ func availableContainerRunner(t *testing.T) string { } for _, candidate := range []string{"podman", "docker"} { if _, err := exec.LookPath(candidate); err == nil { + requireVerificationRuntime(t, candidate) return candidate } } - t.Skip("no container runner is available") + testutil.MissingVerificationCapability(t, "no container runner is available", + "neither podman nor docker is on PATH") return "" } +// requireVerificationRuntime skips when the runtime cannot run a container +// shaped the way verification runs one. +// +// Verification bounds what a client may consume -- CPU, memory, processes, file +// size -- because it runs a real package manager over bytes that arrived from +// somewhere else. Rootless Docker has no delegated CPU cgroup, so it refuses +// --cpus outright and every one of these tests fails with a daemon error that +// says nothing about snailmail. Probing once and skipping keeps a developer's +// suite honest about what it did and did not check, and a runtime that can set +// the limits does not skip. +var verificationRuntime struct { + once sync.Once + reason string +} + +func requireVerificationRuntime(t *testing.T, runner string) { + t.Helper() + verificationRuntime.once.Do(func() { + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + probe := exec.CommandContext(ctx, runner, "run", "--rm", "--cpus=2", "--pids-limit=256", + "docker.io/library/alpine@sha256:48b0309ca019d89d40f670aa1bc06e426dc0931948452e8491e3d65087abc07d", + "true") + if output, err := probe.CombinedOutput(); err != nil { + verificationRuntime.reason = strings.TrimSpace(string(output)) + } + }) + if verificationRuntime.reason != "" { + testutil.MissingVerificationCapability(t, + "this runtime cannot run a container with the limits verification uses", verificationRuntime.reason) + } +} + // availablePython finds the interpreter a pip install is run with. Skipped by // -short for the same reason as a container: the install is what costs. func availablePython(t *testing.T) string { @@ -447,7 +484,10 @@ func availablePython(t *testing.T) string { } python, err := exec.LookPath("python3") if err != nil { - t.Skip("python3 is unavailable") + // The same argument as a missing container runtime: pip is what verifies + // a PyPI repository, and a runner that lost python3 would skip every one + // of those tests while still reporting a pass. + testutil.MissingVerificationCapability(t, "python3 is unavailable", err.Error()) } return python } diff --git a/engine/keys.go b/engine/keys.go index 205fbae..db4b8f1 100644 --- a/engine/keys.go +++ b/engine/keys.go @@ -26,6 +26,10 @@ type NewKeyRequest struct { Root string Name string Algorithm string + // Backend names what holds the private key. Empty means the file backend, + // which is the only one that can create a key; "env" records one that + // already exists and is supplied to this process by its environment. + Backend string CreatedAt time.Time ExpiresIn time.Duration Keys signer.Generator @@ -36,6 +40,11 @@ type NewKeyResult struct { Fingerprint string `json:"fingerprint"` ExpiresAt string `json:"expires_at"` Reference string `json:"reference"` + // Created distinguishes a key this command made from one it only recorded. + // The env backend cannot make a key, so saying "generated" for it would tell + // an operator that fresh private material exists somewhere when what + // happened is that an existing key was written into the manifest. + Created bool `json:"created"` } type PublishKeyRequest struct { @@ -110,7 +119,7 @@ func NewKey(ctx context.Context, request NewKeyRequest) (NewKeyResult, error) { if _, exists := manifest.Keys[request.Name]; exists { return NewKeyResult{}, fmt.Errorf("signing key %q already exists", request.Name) } - prepared, err := prepareSigningKey(ctx, manifest, request.Name, signer.Algorithm(request.Algorithm), request.CreatedAt, request.ExpiresIn, request.Keys) + prepared, err := prepareSigningKey(ctx, manifest, request.Name, signer.Algorithm(request.Algorithm), request.Backend, request.CreatedAt, request.ExpiresIn, request.Keys) if err != nil { return NewKeyResult{}, err } @@ -136,7 +145,9 @@ func NewKey(ctx context.Context, request NewKeyRequest) (NewKeyResult, error) { return NewKeyResult{}, err } committed = true - return keyResult(request.Name, prepared.key), nil + result := keyResult(request.Name, prepared.key) + result.Created = prepared.createdPrivate + return result, nil } func PublishKey(ctx context.Context, request PublishKeyRequest) (NewKeyResult, error) { @@ -344,8 +355,11 @@ func publicArmorPath(name, algorithm string) string { return "keys/" + name + ".asc" } -func prepareSigningKey(ctx context.Context, manifest state.Manifest, name string, algorithm signer.Algorithm, createdAt time.Time, expiresIn time.Duration, keys signer.Generator) (preparedSigningKey, error) { - ref := signer.Ref{Backend: "file", ID: manifest.Workspace.ID + "/" + name} +func prepareSigningKey(ctx context.Context, manifest state.Manifest, name string, algorithm signer.Algorithm, backend string, createdAt time.Time, expiresIn time.Duration, keys signer.Generator) (preparedSigningKey, error) { + if backend == "" { + backend = state.DefaultKeyBackend + } + ref := signer.Ref{Backend: backend, ID: manifest.Workspace.ID + "/" + name} generated, err := keys.Public(ctx, ref) createdPrivate := false if errors.Is(err, signer.ErrNotFound) { diff --git a/engine/rotation.go b/engine/rotation.go index 92b493f..1f6a56c 100644 --- a/engine/rotation.go +++ b/engine/rotation.go @@ -152,7 +152,7 @@ func RotateKey(ctx context.Context, request RotateKeyRequest) (RotateKeyResult, } // A rotation keeps the algorithm the repository already uses; a client // that trusts one kind of key cannot verify another. - candidate, err := prepareSigningKey(ctx, manifest, request.Successor, signer.Algorithm(baseKey.Algorithm), now, expiresIn, request.Keys) + candidate, err := prepareSigningKey(ctx, manifest, request.Successor, signer.Algorithm(baseKey.Algorithm), baseKey.Ref.Backend, now, expiresIn, request.Keys) if err != nil { return RotateKeyResult{}, err } diff --git a/engine/signed_client_test.go b/engine/signed_client_test.go new file mode 100644 index 0000000..244eafb --- /dev/null +++ b/engine/signed_client_test.go @@ -0,0 +1,326 @@ +package engine + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "net" + "net/http" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + filesigner "github.com/shellcell/snailmail/adapters/signer/file" + "github.com/shellcell/snailmail/internal/buildgraph" + "github.com/shellcell/snailmail/internal/testutil" + "github.com/shellcell/snailmail/signer" +) + +// A signature is only worth writing if the client accepts it, and until these +// existed nothing checked that. +// +// Every signable format was covered by two things that both stop short. The +// format packages verify their own signatures, which proves snailmail agrees +// with itself. The client verifiers install with a real apt, dnf, apk or helm, +// which proves the tree is installable -- but three of the four used to install +// with the signature check turned off, so a repository no consumer could trust +// verified green, and the tests could not tell. +// +// These run the actual client against a repository this workspace signed. That +// is the only check that fails when the two disagree: a keyring in a format the +// client cannot parse, a detached signature over the wrong bytes, a key +// published under a name the index does not name. +func TestASignedRepositoryInstallsWithItsSignatureChecked(t *testing.T) { + for _, format := range signableFormats { + t.Run(format, func(t *testing.T) { + runner := availableContainerRunner(t) + requireVerificationNetwork(t, format, runner) + published := publishSignedRepository(t, format) + installed, err := verifySignedRepository(t, format, published, runner) + if err != nil { + t.Fatalf("the %s client refused a repository this workspace signed: %v", format, err) + } + if installed == 0 { + t.Fatal("no verification case ran, so nothing was installed") + } + }) + } +} + +// The check that would have caught the bug these tests were written for: break +// the signature, leave the repository otherwise usable, and the client has to be +// the thing that notices. +// +// Only rpm and apk are here, and the reason is worth stating. formats.VerifyStructure +// is a no-op for both -- they are the two formats with no structural check at +// all -- so their client is the only thing standing between a broken signature +// and a consumer. deb and helm re-apply signing during structural verification +// and reject a corrupted signature before any container starts, which means a +// test cannot make their client the sole detector; what their client run proves +// is the other direction, that apt and helm accept a signature snailmail +// considers correct, and that is the test above. +// +// The corruption is deliberately narrow. Breaking an index or an archive makes +// every client fail for reasons that have nothing to do with trust, which is a +// test that passes whether the signature is checked or not -- the first draft of +// this did exactly that for apk, and only mutation testing showed it. +func TestAClientRefusesASignatureThatDoesNotVerify(t *testing.T) { + for format, corrupt := range clientDetectedSignatureBreaks { + t.Run(format, func(t *testing.T) { + runner := availableContainerRunner(t) + requireVerificationNetwork(t, format, runner) + published := publishSignedRepository(t, format) + corrupt(t, published) + rewriteManifestForContents(t, published) + if _, err := verifySignedRepository(t, format, published, runner); err == nil { + t.Fatalf("the %s client accepted a repository whose signature does not verify", format) + } + }) + } +} + +// How to break each format's trust without breaking anything else. +var clientDetectedSignatureBreaks = map[string]func(t *testing.T, repository string){ + // A detached signature: repomd.xml and every index it names stay valid, so + // dnf can read the whole repository and the only thing wrong is the .asc. + "rpm": func(t *testing.T, repository string) { + t.Helper() + editRepositoryFile(t, repository, "repodata/repomd.xml.asc", flipArmoredBody) + }, + // apk's signature is a member inside APKINDEX.tar.gz, so corrupting it + // corrupts the index. The published key is the other half of the same + // binding and can be broken on its own: the index still parses and still + // carries a valid signature, made by a key that is no longer the one on + // offer. That is also the likeliest way to get this wrong for real. + "apk": func(t *testing.T, repository string) { + t.Helper() + manifest, _, err := buildgraph.ReadManifest(repository) + if err != nil { + t.Fatal(err) + } + editRepositoryFile(t, repository, manifest.Install.SigningKeyPath, flipArmoredBody) + }, +} + +func editRepositoryFile(t *testing.T, repository, path string, edit func([]byte) []byte) { + t.Helper() + name := filepath.Join(repository, filepath.FromSlash(path)) + content, err := os.ReadFile(name) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(name, edit(content), 0o644); err != nil { + t.Fatal(err) + } +} + +// flipArmoredBody changes what an armored blob says without changing what it is, +// so a client parses it and rejects it rather than failing to recognise it. The +// checksum line is left alone: corrupting that is a parse error, not a bad +// signature. +func flipArmoredBody(content []byte) []byte { + lines := strings.Split(string(content), "\n") + for index, line := range lines { + if len(line) <= 40 || strings.HasPrefix(line, "-----") || strings.Contains(line, "=") { + continue + } + replacement := byte('B') + if line[0] == 'B' { + replacement = 'C' + } + lines[index] = string(replacement) + line[1:] + return []byte(strings.Join(lines, "\n")) + } + return content +} + +var signableFormats = []string{"deb", "rpm", "apk", "helm"} + +// requireVerificationNetwork skips a format whose client cannot be reached on +// this machine. +// +// Only Helm needs anything: it has no mounted-directory mode, because a chart +// repository is an HTTP resource, so its tree is served from this process and +// the container has to share the host's network namespace to read it. Rootless +// Docker's "host" network is its own namespace rather than the host's, so the +// listener here is unreachable and the failure says nothing about signing. +// +// Every other format installs from a mounted directory and needs no network at +// all, which is why they are not probed. +func requireVerificationNetwork(t *testing.T, format, runner string) { + t.Helper() + if format != "helm" { + return + } + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + server := &http.Server{Handler: http.HandlerFunc(func(writer http.ResponseWriter, _ *http.Request) { + _, _ = writer.Write([]byte("reachable")) + }), ReadHeaderTimeout: 5 * time.Second} + go func() { _ = server.Serve(listener) }() + defer func() { _ = server.Close() }() + + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + probe := exec.CommandContext(ctx, runner, "run", "--rm", "--network=host", + "docker.io/library/alpine@sha256:48b0309ca019d89d40f670aa1bc06e426dc0931948452e8491e3d65087abc07d", + "wget", "-q", "-T", "5", "-O", "-", "http://"+listener.Addr().String()) + if output, err := probe.CombinedOutput(); err != nil { + testutil.MissingVerificationCapability(t, + "this runtime cannot reach a host listener from a container, which Helm verification needs", + strings.TrimSpace(string(output))) + } +} + +// publishSignedRepository takes a workspace all the way to a signed tree on +// disk, through the same calls an operator makes. +func publishSignedRepository(t *testing.T, format string) string { + t.Helper() + root := t.TempDir() + if output, err := exec.Command("git", "-C", root, "init").CombinedOutput(); err != nil { + t.Fatalf("git init: %v: %s", err, output) + } + if err := InitWorkspace(InitWorkspaceRequest{Root: root, Name: "signed-" + format}); err != nil { + t.Fatal(err) + } + passphrase := []byte("signed-client-verification-passphrase") + store, err := filesigner.New(t.TempDir(), func() ([]byte, error) { return append([]byte(nil), passphrase...), nil }) + if err != nil { + t.Fatal(err) + } + createdAt := time.Now().UTC().Truncate(time.Second).Add(-time.Hour) + algorithm := signer.AlgorithmOpenPGPRSA4096 + if format == "apk" { + // apk verifies a bare RSA signature over each index rather than an + // OpenPGP one, so it is the only format whose key is a different kind. + algorithm = signer.AlgorithmAPKRSA4096 + } + if _, err := NewKey(context.Background(), NewKeyRequest{ + Root: root, Name: "archive", Algorithm: algorithm, + CreatedAt: createdAt, ExpiresIn: 365 * 24 * time.Hour, Keys: store, + }); err != nil { + t.Fatal(err) + } + setup := SetupRepositoryRequest{ + Root: root, Name: format, Format: format, HostType: "local", + Output: filepath.ToSlash(filepath.Join("public", format)), SigningKey: "archive", + } + switch format { + case "deb": + setup.Suite, setup.Component, setup.Architectures = "stable", "main", []string{"amd64"} + case "apk": + setup.Architectures = []string{"x86_64"} + } + if err := SetupRepository(setup); err != nil { + t.Fatal(err) + } + artifact := workspaceArtifact(t, root, format, "1.2.3") + if _, err := AddArtifacts(AddArtifactsRequest{Root: root, Repository: format, Artifacts: []string{artifact}}); err != nil { + t.Fatal(err) + } + commitWorkspace(t, root, "publish a signed "+format+" repository") + + planName := filepath.Join(root, "signed.json") + // Structural: the client run is the assertion each test makes for itself, + // and running it twice would double the slowest part of the suite. + if _, err := PlanWorkspace(context.Background(), PlanWorkspaceRequest{Hosts: localHosts(), + Root: root, Output: planName, createdAt: createdAt.Add(time.Hour), GeneratedAt: createdAt.Add(time.Hour), + ExpiresIn: time.Hour, VerificationMode: "structural", Signers: store, + }); err != nil { + t.Fatal(err) + } + if _, err := ApplyWorkspace(context.Background(), ApplyWorkspaceRequest{Hosts: localHosts(), + Root: root, Plan: planName, now: createdAt.Add(time.Hour + time.Minute), StructuralOnly: true, + }); err != nil { + t.Fatal(err) + } + published, err := filepath.EvalSymlinks(filepath.Join(root, "public", format)) + if err != nil { + t.Fatal(err) + } + manifest, _, err := buildgraph.ReadManifest(published) + if err != nil { + t.Fatal(err) + } + if manifest.Install.SigningKeyPath == "" || len(manifest.Signatures) == 0 { + t.Fatalf("the published %s repository carries no signature: %+v", format, manifest.Install) + } + return published +} + +func verifySignedRepository(t *testing.T, format, repository, runner string) (int, error) { + t.Helper() + switch format { + case "deb": + result, err := VerifyDeb(context.Background(), VerifyDebRequest{Repository: repository, Runner: runner}) + return result.InstalledCases, err + case "rpm": + result, err := VerifyRPM(context.Background(), VerifyRPMRequest{Repository: repository, Runner: runner}) + return result.InstalledCases, err + case "apk": + result, err := VerifyAPK(context.Background(), VerifyAPKRequest{Repository: repository, Runner: runner}) + return result.InstalledCases, err + case "helm": + result, err := VerifyHelm(context.Background(), VerifyHelmRequest{Repository: repository, Runner: runner}) + return result.InstalledCases, err + } + t.Fatalf("no client verifier for %q", format) + return 0, nil +} + +// rewriteManifestForContents makes the manifest describe the bytes on disk, so +// the structural check passes and the client is the only thing left to object. +func rewriteManifestForContents(t *testing.T, repository string) { + t.Helper() + name := filepath.Join(repository, buildgraph.ManifestFilename) + content, err := os.ReadFile(name) + if err != nil { + t.Fatal(err) + } + var manifest map[string]any + if err := json.Unmarshal(content, &manifest); err != nil { + t.Fatal(err) + } + files, _ := manifest["files"].([]any) + entries := make([]buildgraph.ManifestFile, 0, len(files)) + for _, raw := range files { + file, _ := raw.(map[string]any) + path, _ := file["path"].(string) + size, digest := hashRepositoryEntry(t, repository, path) + file["size"], file["sha256"] = size, digest + entries = append(entries, buildgraph.ManifestFile{Path: path, Size: size, SHA256: digest}) + } + // Signature digests are recorded twice; both have to agree with the bytes. + if signatures, ok := manifest["signatures"].([]any); ok { + for _, raw := range signatures { + signature, _ := raw.(map[string]any) + path, _ := signature["path"].(string) + _, digest := hashRepositoryEntry(t, repository, path) + signature["sha256"] = digest + } + } + manifest["tree_sha256"] = buildgraph.TreeDigest(entries) + rewritten, err := json.MarshalIndent(manifest, "", " ") + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(name, append(rewritten, '\n'), 0o644); err != nil { + t.Fatal(err) + } +} + +func hashRepositoryEntry(t *testing.T, repository, path string) (int64, string) { + t.Helper() + content, err := os.ReadFile(filepath.Join(repository, filepath.FromSlash(path))) + if err != nil { + t.Fatal(err) + } + digest := sha256.Sum256(content) + return int64(len(content)), hex.EncodeToString(digest[:]) +} diff --git a/engine/workspace.go b/engine/workspace.go index b60f6c9..3c29f67 100644 --- a/engine/workspace.go +++ b/engine/workspace.go @@ -1309,9 +1309,8 @@ func verifyCanonicalClient(ctx context.Context, root string, repository state.Re // host is serving. Each format needs its own client, so the pair must be // declared before the attempt rather than discovered by a nil dispatch. func verifyEndpointClient(ctx context.Context, repository state.Repository, staged string, access host.ClientAccess, request ApplyWorkspaceRequest) error { - if !host.Supports(repository.Host.Type, repository.Format).RemoteClientVerification { - return fmt.Errorf("client verification is not implemented for format %q on host %q", - repository.Format, repository.Host.Type) + if !host.Supports(repository.Host.Type, repository.Format).Publish { + return fmt.Errorf("host %q does not serve format %q", repository.Host.Type, repository.Format) } // A format with no endpoint probe is verified against the staged tree // instead, which is what happens anyway when no preview site is configured. @@ -1645,7 +1644,7 @@ func repositoryHostIdentity(repository state.Repository) (string, error) { } func repositoryInstallDocDigest(root, name string, repository state.Repository, keys map[string]state.SigningKey) (string, error) { - if !host.Supports(repository.Host.Type, repository.Format).InstallDocument { + if !state.PublishesInstallDocument(repository) { return "", nil } if err := state.ValidateInstallDocument(root, name, repository, keys); err != nil { @@ -1838,9 +1837,10 @@ func validateApplyPlan(plan state.Plan) error { return fmt.Errorf("plan repository %q has no canonical endpoint", repository.Name) } // The same predicate the digest is computed under, rather than a second - // list of host names beside it. These had drifted: rsync generates an - // install document and this did not check it. - if host.Supports(repository.Host.Type, repository.Format).InstallDocument && !hexdigest.ValidSHA256(repository.InstallDocSHA256) { + // list of host names beside it. These had drifted twice: the list never + // gained rsync, and the matrix that replaced it could not see a local + // repository that had been given a public URL. + if state.PublishesInstallDocument(state.Repository{Host: repository.Host}) && !hexdigest.ValidSHA256(repository.InstallDocSHA256) { return fmt.Errorf("plan repository %q has an invalid install document digest", repository.Name) } // Required of a host that reports one. A forged plan claiming otherwise @@ -2307,9 +2307,8 @@ func (preparation *applyPreparation) stageOnHosts(prepared []applyRepository) er } continue } - if !host.Supports(item.hostRepository.Type, item.repository.Format).RemoteClientVerification { - return fmt.Errorf("host preview verification is not implemented for format %q on host %q", - item.repository.Format, item.hostRepository.Type) + if !host.Supports(item.hostRepository.Type, item.repository.Format).Publish { + return fmt.Errorf("host %q does not serve format %q", item.hostRepository.Type, item.repository.Format) } access := item.hostStage.Access if access.Endpoint == "" { diff --git a/engine/workspace_test.go b/engine/workspace_test.go index fa4836a..b62dcfd 100644 --- a/engine/workspace_test.go +++ b/engine/workspace_test.go @@ -1845,6 +1845,11 @@ func workspaceArtifact(t *testing.T, root, format, version string) string { name, err = testutil.WriteDeb(directory, "snail-demo", version+"-1", "amd64", nil) case "helm": name, err = testutil.WriteHelmChart(directory, "snail-demo", version) + case "rpm", "apk": + // These two are checked in rather than generated: building a real .rpm or + // .apk means reproducing a signed header layout, and the point of the + // fixture is what the repository does with a package, not how one is made. + name, err = copyFormatTestdata(directory, format) default: t.Fatalf("unknown fixture format %q", format) } @@ -1854,6 +1859,28 @@ func workspaceArtifact(t *testing.T, root, format, version string) string { return name } +// copyFormatTestdata places a format's checked-in sample package where a +// workspace fixture can add it from. +func copyFormatTestdata(directory, format string) (string, error) { + sources := map[string]string{ + "rpm": filepath.Join("..", "formats", "rpm", "testdata", "snail-demo-1.2.3-4.noarch.rpm"), + "apk": filepath.Join("..", "formats", "apk", "testdata", "snail-demo-1.2.3-r4.apk"), + } + source, known := sources[format] + if !known { + return "", fmt.Errorf("no checked-in sample package for %q", format) + } + content, err := os.ReadFile(source) + if err != nil { + return "", err + } + name := filepath.Join(directory, filepath.Base(source)) + if err := os.WriteFile(name, content, 0o644); err != nil { + return "", err + } + return name, nil +} + type staticHostResolver struct { host host.Host } diff --git a/formats/apk/signature.go b/formats/apk/signature.go index b2c746b..180eed5 100644 --- a/formats/apk/signature.go +++ b/formats/apk/signature.go @@ -8,6 +8,7 @@ import ( "encoding/hex" "errors" "fmt" + "io" "path" "regexp" "sort" @@ -190,3 +191,47 @@ func SignedKeyName(keyName string) string { } return keyName + ".rsa.pub" } + +// signatureEntryPrefix is what names the tar entry a signature stream holds. +// apk reads the key's filename out of the rest of the name. +const signatureEntryPrefix = ".SIGN." + +// UnsignedIndex returns the index a signature stream was prepended to, or the +// content unchanged when none was. +// +// A signed index is two gzip members: the signature, then the index it covers. +// Anything comparing a published index against a rebuild has to compare like +// with like, and a rebuild from package bytes alone produces the second member +// only -- the first depends on a private key. +// +// The split is exact rather than approximate. gzip reads through a bytes.Reader +// without buffering ahead, because bytes.Reader is an io.ByteReader, so after +// one member the reader sits precisely on the boundary. +func UnsignedIndex(content []byte) ([]byte, error) { + reader := bytes.NewReader(content) + member, err := gzip.NewReader(reader) + if err != nil { + return nil, fmt.Errorf("read APKINDEX: %w", err) + } + member.Multistream(false) + header, err := tar.NewReader(member).Next() + if err != nil { + // A single member holding no readable tar entry is not a signature, so + // there is nothing to strip. + return content, nil + } + if !strings.HasPrefix(path.Base(header.Name), signatureEntryPrefix) { + return content, nil + } + if _, err := io.Copy(io.Discard, member); err != nil { + return nil, fmt.Errorf("read APKINDEX signature member: %w", err) + } + if err := member.Close(); err != nil { + return nil, fmt.Errorf("close APKINDEX signature member: %w", err) + } + consumed := len(content) - reader.Len() + if consumed <= 0 || consumed >= len(content) { + return nil, errors.New("APKINDEX signature member covers the whole archive") + } + return content[consumed:], nil +} diff --git a/formats/conformance_test.go b/formats/conformance_test.go index b1f48ed..8ac9382 100644 --- a/formats/conformance_test.go +++ b/formats/conformance_test.go @@ -355,7 +355,7 @@ func TestASignedRepositoryInstallsItsKeyFirst(t *testing.T) { func TestEndpointProbeSupport(t *testing.T) { want := map[string]bool{ "pypi": true, "deb": true, "raw": true, - "helm": false, "rpm": false, "apk": false, + "rpm": true, "helm": true, "apk": true, } for _, format := range All() { expected, known := want[format.Name()] diff --git a/formats/instructions.go b/formats/instructions.go index fdc5fb4..861660a 100644 --- a/formats/instructions.go +++ b/formats/instructions.go @@ -245,6 +245,6 @@ func (rawFormat) HasEndpointProbe() bool { return true } // The remaining three are verified against the staged tree instead. Their // clients can read a URL, but no endpoint probe is implemented for them yet, and // claiming one would have the engine skip the check it does have. -func (helmFormat) HasEndpointProbe() bool { return false } -func (rpmFormat) HasEndpointProbe() bool { return false } -func (apkFormat) HasEndpointProbe() bool { return false } +func (helmFormat) HasEndpointProbe() bool { return true } +func (rpmFormat) HasEndpointProbe() bool { return true } +func (apkFormat) HasEndpointProbe() bool { return true } diff --git a/formats/structure_test.go b/formats/structure_test.go new file mode 100644 index 0000000..c55b085 --- /dev/null +++ b/formats/structure_test.go @@ -0,0 +1,348 @@ +package formats + +import ( + "bytes" + "compress/gzip" + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "io" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + "time" + + "github.com/shellcell/snailmail/formats/apk" + "github.com/shellcell/snailmail/formats/rpm" + "github.com/shellcell/snailmail/internal/buildgraph" + "github.com/shellcell/snailmail/internal/domain" + "github.com/shellcell/snailmail/signer" + "github.com/shellcell/snailmail/signer/apkrsa" +) + +// A structure check that checks nothing passes every test written about it, and +// two of these were no-ops for long enough to ship. +// +// A path no format recognises is the cheapest thing every implementation must +// reject, and the one a no-op cannot: returning nil for a tree containing a file +// the rebuild would never produce is the whole failure, in miniature. +func TestNoFormatAcceptsAPathItDoesNotRecognise(t *testing.T) { + for _, format := range All() { + manifest := buildgraph.RepositoryManifest{ + Format: format.ID(), + GeneratedAt: time.Unix(0, 0).UTC().Format(time.RFC3339), + Files: []buildgraph.ManifestFile{{ + Path: "not-a-path-any-format-produces/payload.bin", Size: 3, + SHA256: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + }}, + } + if _, err := format.VerifyStructure(t.TempDir(), manifest); err == nil { + t.Errorf("format %q accepted a tree containing a path it cannot have produced, "+ + "so its structure check verifies nothing", format.Name()) + } + } +} + +// The check itself: an index that disagrees with the packages beside it has to +// be refused, without a client and without a network. +// +// rpm and apk are here because they are where this was missing. Both are also +// the formats whose signature is left to the client, so if the structure check +// does not hold they have nothing at all between a bad build and a consumer. +func TestARepositoryWhoseIndexDisagreesWithItsPackagesIsRefused(t *testing.T) { + t.Run("rpm", func(t *testing.T) { + root := writeRPMRepository(t) + // Point the index at a package that is not where it says. This is the + // failure the client verifier's own comment names -- a location dnf + // resolves differently -- and a tree digest cannot see it, because every + // byte is still exactly what the manifest recorded. + editGzip(t, findFile(t, root, "repodata", "-primary.xml.gz"), func(body string) string { + return strings.Replace(body, `href="Packages/`, `href="packages/`, 1) + }) + rewriteManifestDigests(t, root) + if _, err := (rpmFormat{}).VerifyStructure(root, readManifest(t, root)); err == nil { + t.Fatal("a yum repository whose primary index misdescribes its package was accepted") + } + }) + + t.Run("apk", func(t *testing.T) { + root := writeAPKRepository(t) + // Change the size the index reports for the package it lists. + editGzip(t, filepath.Join(root, "x86_64", apk.IndexFilename), func(body string) string { + return strings.Replace(body, "\nS:", "\nS:9", 1) + }) + rewriteManifestDigests(t, root) + if _, err := (apkFormat{}).VerifyStructure(root, readManifest(t, root)); err == nil { + t.Fatal("an Alpine repository whose index misreports a package size was accepted") + } + }) +} + +// And the same trees unedited must pass, or the test above proves only that the +// check rejects everything. +func TestARepositoryMatchingItsPackagesIsAccepted(t *testing.T) { + root := writeRPMRepository(t) + if _, err := (rpmFormat{}).VerifyStructure(root, readManifest(t, root)); err != nil { + t.Fatalf("a freshly built yum repository was refused: %v", err) + } + root = writeAPKRepository(t) + if _, err := (apkFormat{}).VerifyStructure(root, readManifest(t, root)); err != nil { + t.Fatalf("a freshly built Alpine repository was refused: %v", err) + } +} + +func writeRPMRepository(t *testing.T) string { + t.Helper() + name := "snail-demo-1.2.3-4.noarch.rpm" + blob := fixtureBlob(t, "rpm", name) + artifact, err := rpm.Build([]domain.Blob{blob}, rpm.BuildOptions{GeneratedAt: time.Unix(0, 0).UTC()}) + if err != nil { + t.Fatal(err) + } + return writeArtifact(t, artifact, readFixture(t, "rpm", name)) +} + +func writeAPKRepository(t *testing.T) string { + t.Helper() + name := "snail-demo-1.2.3-r4.apk" + blob := fixtureBlob(t, "apk", name) + artifact, err := apk.Build([]domain.Blob{blob}, apk.BuildOptions{ + Architectures: []string{"x86_64"}, GeneratedAt: time.Unix(0, 0).UTC(), + }) + if err != nil { + t.Fatal(err) + } + return writeArtifact(t, artifact, readFixture(t, "apk", name)) +} + +// writeSignedAPKRepository lays out a repository whose index carries a real +// signature, which is what makes the index differ from a rebuild of it. +func writeSignedAPKRepository(t *testing.T) string { + t.Helper() + at := time.Unix(1_700_000_100, 0).UTC() + generated, err := apkrsa.Generate(time.Unix(1_700_000_000, 0).UTC(), 48*time.Hour, []byte(signingPassphrase)) + if err != nil { + t.Fatal(err) + } + local, err := apkrsa.Open(generated.PrivateArmor, []byte(signingPassphrase), generated.Identity) + if err != nil { + t.Fatal(err) + } + defer local.Close() + + name := "snail-demo-1.2.3-r4.apk" + repository := Repository{Architectures: []string{"x86_64"}} + artifact, err := apk.Build([]domain.Blob{fixtureBlob(t, "apk", name)}, apk.BuildOptions{ + Architectures: repository.Architectures, GeneratedAt: at, + }) + if err != nil { + t.Fatal(err) + } + signing, err := SignerFor("apk") + if err != nil { + t.Fatal(err) + } + shape, err := signing.SigningShape(repository, nil) + if err != nil { + t.Fatal(err) + } + payloads, err := signing.SigningPayloads(artifact, repository, shape, nil) + if err != nil { + t.Fatal(err) + } + signatures := make(map[string][]byte, len(shape.Outputs)) + for _, output := range shape.Outputs { + response, err := local.Sign(context.Background(), signer.Request{ + Scheme: output.Scheme, Payload: payloads[output.Path], CreatedAt: at, + }) + if err != nil { + t.Fatal(err) + } + signatures[output.Path] = response.Content + } + signed, err := signing.PlaceSignatures(artifact, repository, SigningMaterial{ + Fingerprint: generated.Identity.Fingerprint, + PublicArmor: generated.PublicArmor, PublicArmorPath: "keys/alpine.rsa.pub", + SignatureTime: at, Signatures: signatures, + }) + if err != nil { + t.Fatal(err) + } + return writeArtifactAt(t, signed, readFixture(t, "apk", name), at) +} + +// readFixture is the package bytes themselves; the build is given only their +// digest, so materialising the tree needs them back. +func readFixture(t *testing.T, format, name string) []byte { + t.Helper() + content, err := os.ReadFile(filepath.Join(format, "testdata", name)) + if err != nil { + t.Fatal(err) + } + return content +} + +// writeArtifact lays a rendered artifact out on disk the way a publication does, +// with the package bytes filled in from the blob the build was given. +func writeArtifact(t *testing.T, artifact domain.RepositoryArtifact, packageBytes []byte) string { + t.Helper() + return writeArtifactAt(t, artifact, packageBytes, time.Unix(0, 0).UTC()) +} + +func writeArtifactAt(t *testing.T, artifact domain.RepositoryArtifact, packageBytes []byte, generatedAt time.Time) string { + t.Helper() + finalized, manifest, err := buildgraph.Finalize(artifact, generatedAt) + if err != nil { + t.Fatal(err) + } + root := t.TempDir() + for _, file := range finalized.Files { + content := file.Content + if file.BlobSHA256 != "" { + content = packageBytes + } + name := filepath.Join(root, filepath.FromSlash(file.Path)) + if err := os.MkdirAll(filepath.Dir(name), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(name, content, 0o644); err != nil { + t.Fatal(err) + } + } + encoded, err := json.MarshalIndent(manifest, "", " ") + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, buildgraph.ManifestFilename), append(encoded, '\n'), 0o644); err != nil { + t.Fatal(err) + } + return root +} + +// rewriteManifestDigests makes the manifest describe the bytes on disk, so an +// edited index is refused for what it says rather than for its digest. Without +// this the tree check rejects it first and the structure check is never asked. +func rewriteManifestDigests(t *testing.T, root string) { + t.Helper() + manifest := readManifest(t, root) + for index, file := range manifest.Files { + content, err := os.ReadFile(filepath.Join(root, filepath.FromSlash(file.Path))) + if err != nil { + t.Fatal(err) + } + digest := sha256.Sum256(content) + manifest.Files[index].Size = int64(len(content)) + manifest.Files[index].SHA256 = hex.EncodeToString(digest[:]) + } + manifest.TreeSHA256 = buildgraph.TreeDigest(manifest.Files) + encoded, err := json.MarshalIndent(manifest, "", " ") + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, buildgraph.ManifestFilename), append(encoded, '\n'), 0o644); err != nil { + t.Fatal(err) + } +} + +func readManifest(t *testing.T, root string) buildgraph.RepositoryManifest { + t.Helper() + manifest, _, err := buildgraph.ReadManifest(root) + if err != nil { + t.Fatal(err) + } + return manifest +} + +func findFile(t *testing.T, root, directory, suffix string) string { + t.Helper() + entries, err := os.ReadDir(filepath.Join(root, directory)) + if err != nil { + t.Fatal(err) + } + for _, entry := range entries { + if strings.HasSuffix(entry.Name(), suffix) { + return filepath.Join(root, directory, entry.Name()) + } + } + t.Fatalf("no %q file under %q", suffix, directory) + return "" +} + +// editGzip rewrites the contents of a gzipped file, leaving it a valid archive +// so the check has to look at what it says rather than whether it parses. +func editGzip(t *testing.T, name string, edit func(string) string) { + t.Helper() + content, err := os.ReadFile(name) + if err != nil { + t.Fatal(err) + } + // An Alpine index carries its signature in a first member; only the second + // holds the index, and the signature is left exactly as it was. + prefix := []byte(nil) + if body, err := apk.UnsignedIndex(content); err == nil && len(body) != len(content) { + prefix, content = content[:len(content)-len(body)], body + } + reader, err := gzip.NewReader(bytes.NewReader(content)) + if err != nil { + t.Fatal(err) + } + body, err := io.ReadAll(reader) + if err != nil { + t.Fatal(err) + } + edited := edit(string(body)) + if edited == string(body) { + t.Fatalf("the edit to %q changed nothing, so the test would pass without checking anything", name) + } + var compressed bytes.Buffer + writer := gzip.NewWriter(&compressed) + if _, err := writer.Write([]byte(edited)); err != nil { + t.Fatal(err) + } + if err := writer.Close(); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(name, append(prefix, compressed.Bytes()...), 0o644); err != nil { + t.Fatal(err) + } +} + +// The structure check is a check, not an edit. Its caller goes on to snapshot +// the tree against the same manifest, so a check that rewrote what it was given +// would leave the caller comparing files against digests no file has. +// +// A manifest is passed by value and its files are not, which makes this easy to +// get wrong and impossible to see here: the first version of the Alpine check +// rewrote each index entry in place, and it surfaced as a snapshot reporting +// that the index "changed before snapshot" -- a long way from the code that +// changed it. +func TestVerifyStructureDoesNotEditTheManifestItWasGiven(t *testing.T) { + // The signed Alpine repository is the case that matters: it is the only one + // whose check has to rewrite anything at all, because its signature is inside + // the index rather than beside it. + for name, build := range map[string]func(*testing.T) string{ + "rpm": writeRPMRepository, + "apk": writeAPKRepository, + "apk signed": writeSignedAPKRepository, + } { + t.Run(name, func(t *testing.T) { + root := build(t) + manifest := readManifest(t, root) + before := append([]buildgraph.ManifestFile(nil), manifest.Files...) + selected, err := ForID(manifest.Format) + if err != nil { + t.Fatal(err) + } + if _, err := selected.VerifyStructure(root, manifest); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(before, manifest.Files) { + t.Errorf("verifying %s structure rewrote the manifest it was given:\nbefore %+v\nafter %+v", + name, before, manifest.Files) + } + }) + } +} diff --git a/formats/verify.go b/formats/verify.go index 43467b7..818465a 100644 --- a/formats/verify.go +++ b/formats/verify.go @@ -14,12 +14,15 @@ package formats // — so it stays on the side of the boundary ARCHITECTURE §2 draws around // effects, and the client verification that does run containers stays out. // -// rpm and apk have no structure check yet; theirs is the tree digest and the -// real client alone. +// Every format has one. What varies is whether the signature is part of it: +// deb and helm re-apply signing and so reject a broken one here, while rpm and +// apk compare the unsigned structure and leave the signature to the client that +// has to accept it. import ( "crypto/md5" "crypto/sha1" + "crypto/sha256" "encoding/hex" "errors" "fmt" @@ -28,13 +31,16 @@ import ( "path" "path/filepath" "reflect" + "sort" "strings" "time" + "github.com/shellcell/snailmail/formats/apk" "github.com/shellcell/snailmail/formats/deb" "github.com/shellcell/snailmail/formats/helm" "github.com/shellcell/snailmail/formats/pypi" "github.com/shellcell/snailmail/formats/raw" + "github.com/shellcell/snailmail/formats/rpm" "github.com/shellcell/snailmail/internal/buildgraph" "github.com/shellcell/snailmail/internal/domain" "github.com/shellcell/snailmail/internal/factscache" @@ -42,14 +48,227 @@ import ( openpgpsigner "github.com/shellcell/snailmail/signer/openpgp" ) -// A format with no structure check of its own. The tree digest still covers -// every byte, and a real client still installs from it. -func (rpmFormat) VerifyStructure(string, buildgraph.RepositoryManifest) ([]domain.Blob, error) { - return nil, nil +// Both of these rebuild the repository from the package bytes on disk and +// require the published indexes to be what that rebuild produces. +// +// What they deliberately do not check is the signature. deb and helm re-apply +// signing here, which makes their structural check reject a broken signature +// before any client sees it -- and means no test can show that their client +// would have caught it. Leaving it to the client for these two keeps that +// coverage somewhere, and the client is where it matters: a signature is only +// worth writing if dnf and apk accept it. +// +// The consequence is that a signature file and the published key are covered by +// the tree digest and by the client, not by this. signedPaths is what says so. +func (rpmFormat) VerifyStructure(root string, manifest buildgraph.RepositoryManifest) ([]domain.Blob, error) { + var blobs []domain.Blob + signed := signedPaths(manifest) + for _, file := range manifest.Files { + if file.Path == listing.Filename || signed[file.Path] { + continue + } + // repodata is the rebuild's output, so it is compared rather than read. + if strings.HasPrefix(file.Path, "repodata/") { + continue + } + if path.Dir(file.Path) != rpm.PackageDirectory || !rpm.IsArtifactFilename(path.Base(file.Path)) { + return nil, fmt.Errorf("unexpected RPM repository path %q", file.Path) + } + facts, err := inspectPackage(root, file, rpm.FormatID, func(name string, reader io.ReaderAt, size int64) (domain.PackageFacts, error) { + return rpm.Inspect(name, reader, size) + }) + if err != nil { + return nil, err + } + blobs = append(blobs, domain.Blob{ + Filename: path.Base(file.Path), Size: file.Size, SHA256: file.SHA256, Facts: facts, + }) + } + generatedAt, err := time.Parse(time.RFC3339, manifest.GeneratedAt) + if err != nil { + return nil, err + } + expected, err := rpm.Build(blobs, rpm.BuildOptions{GeneratedAt: generatedAt}) + if err != nil { + return nil, fmt.Errorf("rebuild RPM structure: %w", err) + } + if err := compareUnsignedStructure(expected, manifest, generatedAt, "RPM"); err != nil { + return nil, err + } + return blobs, nil +} + +func (apkFormat) VerifyStructure(root string, manifest buildgraph.RepositoryManifest) ([]domain.Blob, error) { + var blobs []domain.Blob + signed := signedPaths(manifest) + for _, file := range manifest.Files { + if file.Path == listing.Filename || signed[file.Path] { + continue + } + if path.Base(file.Path) == apk.IndexFilename { + continue + } + if !apk.IsArtifactFilename(path.Base(file.Path)) || strings.Count(file.Path, "/") != 1 { + return nil, fmt.Errorf("unexpected Alpine repository path %q", file.Path) + } + facts, err := inspectPackage(root, file, apk.FormatID, func(name string, reader io.ReaderAt, size int64) (domain.PackageFacts, error) { + return apk.Inspect(name, reader, size) + }) + if err != nil { + return nil, err + } + blobs = append(blobs, domain.Blob{ + Filename: path.Base(file.Path), Size: file.Size, SHA256: file.SHA256, Facts: facts, + }) + } + generatedAt, err := time.Parse(time.RFC3339, manifest.GeneratedAt) + if err != nil { + return nil, err + } + expected, err := apk.Build(blobs, apk.BuildOptions{ + Architectures: manifest.Install.Architectures, GeneratedAt: generatedAt, + }) + if err != nil { + return nil, fmt.Errorf("rebuild Alpine structure: %w", err) + } + // An Alpine signature lives inside the index rather than beside it, so the + // published index has to have it taken off before it can be compared with a + // rebuild that never had one. + if manifest.Install.SigningKeyPath != "" { + unsigned, err := alpineFilesWithoutSignatures(root, manifest.Files) + if err != nil { + return nil, err + } + manifest.Files = unsigned + } + if err := compareUnsignedStructure(expected, manifest, generatedAt, "Alpine"); err != nil { + return nil, err + } + return blobs, nil +} + +// signedPaths names the files a signature added, which a rebuild from package +// bytes cannot produce. +func signedPaths(manifest buildgraph.RepositoryManifest) map[string]bool { + paths := make(map[string]bool, len(manifest.Signatures)+1) + if manifest.Install.SigningKeyPath != "" { + paths[manifest.Install.SigningKeyPath] = true + } + for _, signature := range manifest.Signatures { + // An Alpine signature names the index it is embedded in, which is a file + // the rebuild does produce and must still be compared. + if path.Base(signature.Path) == apk.IndexFilename { + continue + } + paths[signature.Path] = true + } + return paths +} + +// inspectPackage reads a package's facts, through the shared cache: verification +// runs over the same bytes on every plan and apply, and inspecting an archive is +// the expensive half. +func inspectPackage(root string, file buildgraph.ManifestFile, formatID string, + inspect func(string, io.ReaderAt, int64) (domain.PackageFacts, error)) (domain.PackageFacts, error) { + if facts, cached := factscache.Lookup(formatID, file.SHA256); cached { + return facts, nil + } + packageFile, err := os.Open(filepath.Join(root, filepath.FromSlash(file.Path))) + if err != nil { + return domain.PackageFacts{}, fmt.Errorf("open package %q: %w", file.Path, err) + } + facts, inspectErr := inspect(path.Base(file.Path), packageFile, file.Size) + closeErr := packageFile.Close() + if inspectErr != nil { + return domain.PackageFacts{}, inspectErr + } + if closeErr != nil { + return domain.PackageFacts{}, fmt.Errorf("close package %q: %w", file.Path, closeErr) + } + factscache.Store(formatID, file.SHA256, facts) + return facts, nil +} + +// alpineFilesWithoutSignatures describes each index as it would be without its +// signature member, so the comparison below is between two unsigned rebuilds. +// +// It returns a new slice rather than editing the one it was given. A manifest is +// passed by value but its files are not, and rewriting them in place left the +// caller holding digests that no file on disk had -- which surfaced as a +// snapshot reporting that the index "changed before snapshot", a long way from +// the code that changed it. +func alpineFilesWithoutSignatures(root string, files []buildgraph.ManifestFile) ([]buildgraph.ManifestFile, error) { + unsignedFiles := append([]buildgraph.ManifestFile(nil), files...) + for index, file := range unsignedFiles { + if path.Base(file.Path) != apk.IndexFilename { + continue + } + content, err := os.ReadFile(filepath.Join(root, filepath.FromSlash(file.Path))) + if err != nil { + return nil, fmt.Errorf("read Alpine index %q: %w", file.Path, err) + } + unsigned, err := apk.UnsignedIndex(content) + if err != nil { + return nil, fmt.Errorf("Alpine index %q: %w", file.Path, err) + } + digest := sha256.Sum256(unsigned) + unsignedFiles[index].Size = int64(len(unsigned)) + unsignedFiles[index].SHA256 = hex.EncodeToString(digest[:]) + } + return unsignedFiles, nil +} + +// compareUnsignedStructure requires a published repository to be the rebuild, +// once both are reduced to what a rebuild can produce. +// +// The browsable page is left out for the reason it is everywhere else: it is +// written from the repository name and endpoint, which do not survive into the +// tree. The signing metadata is left out because these two formats leave the +// signature to the client, and the tree digest still covers all of it. +func compareUnsignedStructure(expected domain.RepositoryArtifact, manifest buildgraph.RepositoryManifest, + generatedAt time.Time, label string) error { + _, expectedManifest, err := buildgraph.Finalize(expected, generatedAt) + if err != nil { + return fmt.Errorf("finalize expected %s structure: %w", label, err) + } + expectedManifest.SchemaVersion = manifest.SchemaVersion + expectedManifest.Files = withoutListing(expectedManifest.Files) + manifest.Files = withoutListing(manifest.Files) + for _, path := range sortedKeys(signedPaths(manifest)) { + manifest.Files = withoutPath(manifest.Files, path) + } + // The tree digest covers the page and the signature, so it cannot match a + // rebuild that has neither. It is checked where it belongs: the deployment + // record is matched against the digest of the tree that was published. + expectedManifest.TreeSHA256, manifest.TreeSHA256 = "", "" + expectedManifest.Signatures, manifest.Signatures = nil, nil + expectedManifest.Install.SigningKeyPath, manifest.Install.SigningKeyPath = "", "" + expectedManifest.Install.SigningFingerprint, manifest.Install.SigningFingerprint = "", "" + expectedManifest.Install.TrustedSigningFingerprints, manifest.Install.TrustedSigningFingerprints = nil, nil + if !reflect.DeepEqual(expectedManifest, manifest) { + return fmt.Errorf("%s indexes or verification metadata do not match package bytes", label) + } + return nil } -func (apkFormat) VerifyStructure(string, buildgraph.RepositoryManifest) ([]domain.Blob, error) { - return nil, nil +func withoutPath(files []buildgraph.ManifestFile, path string) []buildgraph.ManifestFile { + kept := make([]buildgraph.ManifestFile, 0, len(files)) + for _, file := range files { + if file.Path == path { + continue + } + kept = append(kept, file) + } + return kept +} + +func sortedKeys(set map[string]bool) []string { + keys := make([]string, 0, len(set)) + for key := range set { + keys = append(keys, key) + } + sort.Strings(keys) + return keys } func (pypiFormat) VerifyStructure(root string, manifest buildgraph.RepositoryManifest) ([]domain.Blob, error) { diff --git a/host/support.go b/host/support.go index b76974d..739471f 100644 --- a/host/support.go +++ b/host/support.go @@ -19,14 +19,6 @@ import "sort" type FormatSupport struct { // Publish reports whether the host can serve this format's tree at all. Publish bool - // RemoteClientVerification reports whether the endpoint-served client probe - // — staged preview and canonical — is implemented for this pair. A local - // host verifies its own directory instead and does not use this. - RemoteClientVerification bool - // InstallDocument reports whether a consumer instruction document is - // generated and digest-bound for this pair. It needs a stable public - // endpoint, so it accompanies remote hosting rather than a local directory. - InstallDocument bool } // Supported reports whether the host can publish the format at all. @@ -44,9 +36,13 @@ var formatSupport = map[string]map[string]FormatSupport{ "rpm": {Publish: true}, "apk": {Publish: true}, }, - // The first remote slice is PyPI. Extending a host to another format means - // its commit paths, install document, and endpoint client probe, so the two - // flags move together per pair rather than per host. + // The first remote slice is PyPI. What is declared per pair is what genuinely + // varies per pair: whether the host can serve the format's tree at all, and + // whether a consumer instruction document is generated for it. Whether an + // ecosystem's client can be run against a served URL does not vary by host — + // the probes take a URL and nothing else — so that is declared once on the + // format, as formats.Format.HasEndpointProbe, and not restated here. + // // An object store commits one object atomically and offers no ordered // multi-object commit, so it can serve a format only if a single path makes a // revision live. PyPI, yum and Helm all qualify on that count — one of @@ -66,18 +62,18 @@ var formatSupport = map[string]map[string]FormatSupport{ // manifest — stay in the release directory in either shape, because they are // rewritten with every revision. "s3": { - "pypi": {Publish: true, RemoteClientVerification: true, InstallDocument: true}, - "helm": {Publish: true, RemoteClientVerification: true, InstallDocument: true}, + "pypi": {Publish: true}, + "helm": {Publish: true}, // yum, but only unsigned. A signed repository reports repomd.xml and // repomd.xml.asc as its commit paths, because a detached signature has to // become live with the document it signs, and the adapter refuses more than // one — so the limitation is enforced by the path count rather than stated // again here. - "rpm": {Publish: true, RemoteClientVerification: true, InstallDocument: true}, + "rpm": {Publish: true}, // Plain artifacts, which is the simplest thing anyone wants from an object // store. One document makes a revision live — SHA256SUMS — and everything it // names sits at a path fixed by the artifact's name and version. - "raw": {Publish: true, RemoteClientVerification: true, InstallDocument: true}, + "raw": {Publish: true}, }, // Pages commits by moving a publish ref to an orphan commit of the whole // tree, which is atomic and format-neutral, so a suite's Release and its @@ -91,25 +87,25 @@ var formatSupport = map[string]map[string]FormatSupport{ // cannot serve at all. The adapter never inspects commit paths, because with a // whole-tree swap their number does not matter. "rsync": { - "pypi": {Publish: true, RemoteClientVerification: true, InstallDocument: true}, - "deb": {Publish: true, RemoteClientVerification: true, InstallDocument: true}, - "helm": {Publish: true, RemoteClientVerification: true, InstallDocument: true}, - "raw": {Publish: true, RemoteClientVerification: true, InstallDocument: true}, - "rpm": {Publish: true, RemoteClientVerification: true, InstallDocument: true}, - "apk": {Publish: true, RemoteClientVerification: true, InstallDocument: true}, + "pypi": {Publish: true}, + "deb": {Publish: true}, + "helm": {Publish: true}, + "raw": {Publish: true}, + "rpm": {Publish: true}, + "apk": {Publish: true}, }, "github-pages": { - "pypi": {Publish: true, RemoteClientVerification: true, InstallDocument: true}, - "deb": {Publish: true, RemoteClientVerification: true, InstallDocument: true}, + "pypi": {Publish: true}, + "deb": {Publish: true}, // A chart repository switches one file, index.yaml; a provenance file // sits at a content-addressed path nothing ever replaces. So the atomic // commit Pages offers is more than Helm needs. - "helm": {Publish: true, RemoteClientVerification: true, InstallDocument: true}, + "helm": {Publish: true}, // Raw has no client to run: a consumer fetches a URL and checks it // against SHA256SUMS, which host-served byte verification already does. - "raw": {Publish: true, RemoteClientVerification: true, InstallDocument: true}, - "rpm": {Publish: true, RemoteClientVerification: true, InstallDocument: true}, - "apk": {Publish: true, RemoteClientVerification: true, InstallDocument: true}, + "raw": {Publish: true}, + "rpm": {Publish: true}, + "apk": {Publish: true}, }, } diff --git a/host/support_test.go b/host/support_test.go index 938d11b..7d72364 100644 --- a/host/support_test.go +++ b/host/support_test.go @@ -15,62 +15,24 @@ func TestSupportsDeniesUnknownPairs(t *testing.T) { {"local", "cargo"}, // An object store commits one object, so a format qualifies only if one // path makes a revision live: Debian needs a Release and its detached - // signature together, Alpine has an index per architecture, raw has a - // architecture. A signed yum repository is in the same - // position, and is refused by the adapter's path count rather than here, - // which is why rpm is declared. + // signature together, and Alpine has one index per architecture. A signed + // yum repository is in the same position, and is refused by the adapter's + // path count rather than here, which is why rpm is declared. {"s3", "deb"}, {"s3", "apk"}, } { support := Supports(pair[0], pair[1]) - if support.Publish || support.RemoteClientVerification || support.InstallDocument { + if support.Publish { t.Errorf("host %q format %q is permitted by default: %+v", pair[0], pair[1], support) } } } -// A capability that implies publication cannot be declared without it, or a -// pair could be verified or documented but never served. -func TestDeclaredCapabilitiesImplyPublication(t *testing.T) { - for _, hostType := range KnownHostTypes() { - for _, format := range []string{"pypi", "deb", "helm"} { - support := Supports(hostType, format) - if (support.RemoteClientVerification || support.InstallDocument) && !support.Publish { - t.Errorf("host %q format %q declares a capability without publication: %+v", hostType, format, support) - } - } - } -} - -// A local directory is verified against the directory itself, never through a -// served endpoint, so claiming the remote probe would route to code that does -// not run for it. -func TestLocalHostDeclaresNoRemoteCapabilities(t *testing.T) { - for _, format := range SupportedFormats("local") { - support := Supports("local", format) - if support.RemoteClientVerification { - t.Errorf("local host claims remote client verification for %q", format) - } - if support.InstallDocument { - t.Errorf("local host claims an install document for %q, which needs a public endpoint", format) - } - } -} - -// Publication without client verification would let a remote host commit bytes -// no official client ever installed, which is the guarantee the tool sells. -func TestRemotePublicationAlwaysCarriesClientVerification(t *testing.T) { - for _, hostType := range KnownHostTypes() { - if hostType == "local" { - continue - } - for _, format := range SupportedFormats(hostType) { - if !Supports(hostType, format).RemoteClientVerification { - t.Errorf("remote host %q publishes %q without client verification", hostType, format) - } - } - } -} +// Both of the tests that stood here checked one declared capability against +// another. The matrix declares one thing now -- whether a host serves a format +// -- so they asserted nothing and were removed rather than left reading like +// coverage. What they were protecting is a repository's own property, and it is +// checked where that property lives, in internal/state. func TestSupportedFormatsIsStableAndPublishOnly(t *testing.T) { for _, hostType := range KnownHostTypes() { diff --git a/internal/app/apk_endpoint.go b/internal/app/apk_endpoint.go new file mode 100644 index 0000000..183ebee --- /dev/null +++ b/internal/app/apk_endpoint.go @@ -0,0 +1,135 @@ +package app + +import ( + "context" + "errors" + "fmt" + "os/exec" + "strings" + "time" + + "github.com/shellcell/snailmail/formats/apk" + "github.com/shellcell/snailmail/host" + "github.com/shellcell/snailmail/internal/buildgraph" + "github.com/shellcell/snailmail/internal/domain" + endpointcheck "github.com/shellcell/snailmail/internal/endpoint" +) + +// VerifyAPKClientEndpointAccess installs from an Alpine repository the host is +// actually serving, running apk against the endpoint rather than a mounted +// directory. +// +// What this adds over the mounted check is specific to how apk fetches: it asks +// for //APKINDEX.tar.gz and then for each package at a path +// the index gives relative to that base, so a host that redirects, rewrites, or +// re-encodes any part of it serves a repository apk cannot resolve. The index is +// also a concatenation of gzip members, and a host that recompresses it breaks +// the signature member without changing anything the mounted check would see. +func VerifyAPKClientEndpointAccess(ctx context.Context, root string, access host.ClientAccess, runner, image string, scope VersionScope) (buildgraph.RepositoryManifest, int, error) { + if access.Credential != nil { + defer access.Credential.Destroy() + } + endpoint, err := validateClientEndpoint(access.Endpoint) + if err != nil { + return buildgraph.RepositoryManifest{}, 0, err + } + // A private Alpine repository would need apk credentials inside the + // container; nothing issues them yet, so refuse rather than silently + // verifying an unauthenticated view. + if access.Credential != nil { + return buildgraph.RepositoryManifest{}, 0, errors.New("Alpine endpoint verification does not support private read credentials") + } + manifest, _, err := verifyRepository(root) + if err != nil { + return buildgraph.RepositoryManifest{}, 0, err + } + if manifest.Format != apk.FormatID { + return buildgraph.RepositoryManifest{}, 0, fmt.Errorf("repository format is %q, not %q", manifest.Format, apk.FormatID) + } + if len(access.Routes) == 0 { + access.Routes, err = defaultClientRoutes(access.Endpoint, manifest.Files) + if err != nil { + return buildgraph.RepositoryManifest{}, 0, err + } + } + if err := awaitHostedBytes(ctx, access, "", "", false); err != nil { + return buildgraph.RepositoryManifest{}, 0, err + } + if runner == "" { + runner = "podman" + } + runner, err = exec.LookPath(runner) + if err != nil { + return buildgraph.RepositoryManifest{}, 0, fmt.Errorf("find container runner: %w", err) + } + if !digestPinnedImage(image) { + return buildgraph.RepositoryManifest{}, 0, errors.New("a digest-pinned Alpine verification image is required") + } + trust, cleanup, err := fetchEndpointTrust(ctx, endpoint, manifest) + if err != nil { + return buildgraph.RepositoryManifest{}, 0, err + } + defer cleanup() + + hostNetwork := endpointcheck.IsLoopback(endpoint.Hostname()) + verificationCases := scope.selection(manifest.VerificationCases, apkCompare) + if err := verifyCases(ctx, verificationCases, func(caseCtx context.Context, verification domain.VerificationCase) error { + return verifyAPKEndpointCase(caseCtx, runner, image, access.Endpoint, trust, hostNetwork, manifest, verification) + }); err != nil { + return buildgraph.RepositoryManifest{}, 0, err + } + return manifest, len(verificationCases), nil +} + +func verifyAPKEndpointCase(ctx context.Context, runner, image, endpoint string, trust endpointTrust, hostNetwork bool, manifest buildgraph.RepositoryManifest, verification domain.VerificationCase) error { + caseCtx, cancel := context.WithTimeout(ctx, 3*time.Minute) + defer cancel() + platform, err := apkOCIPlatform(verification.Architecture) + if err != nil { + return err + } + reference, platformFlag := image, false + if platform != "" { + reference, platformFlag, err = platformImage(caseCtx, runner, image, platform) + if err != nil { + return err + } + } + arguments := []string{"run", "--rm", "--pull=missing"} + if hostNetwork { + arguments = append(arguments, "--network=host") + } + if platformFlag { + arguments = append(arguments, "--platform", platform) + } + arguments = append(arguments, + "--memory=1g", "--cpus=2", "--pids-limit=256", + "--ulimit", "fsize=536870912:536870912", "--ulimit", "nofile=1024:1024", + "--security-opt", "no-new-privileges", + "--env", "SNAILMAIL_PACKAGE="+verification.Package, + "--env", "SNAILMAIL_VERSION="+verification.Version, + "--env", "SNAILMAIL_BASEURL="+strings.TrimSuffix(endpoint, "/"), + "--env", "SNAILMAIL_SIGNING_KEY="+manifest.Install.SigningKeyPath, + "--env", "SNAILMAIL_KEYSOURCE="+endpointKeySource(trust), + "--env", "SNAILMAIL_CA="+endpointCAPath(trust), + "--tmpfs", "/tmp:rw,exec,size=64m,mode=1777", + ) + arguments = append(arguments, endpointTrustMounts(trust)...) + arguments = append(arguments, reference, "sh", "-euc", apkVerificationScript) + + command := exec.CommandContext(caseCtx, runner, arguments...) + command.Env = runnerEnvironment() + output, err := command.CombinedOutput() + if err != nil { + if registryUnavailable(output) { + return fmt.Errorf("%w: %s", ErrVerificationImageUnavailable, strings.TrimSpace(string(output))) + } + if foreignPlatformUnsupported(output) { + return fmt.Errorf("%w: verifying %s needs QEMU and binfmt_misc registered for it", + ErrForeignPlatformUnsupported, verification.Architecture) + } + return fmt.Errorf("Alpine endpoint verification for %s=%s/%s: %w\n%s", + verification.Package, verification.Version, verification.Architecture, err, strings.TrimSpace(string(output))) + } + return nil +} diff --git a/internal/app/apk_verify.go b/internal/app/apk_verify.go index 86b00c2..59a835f 100644 --- a/internal/app/apk_verify.go +++ b/internal/app/apk_verify.go @@ -54,14 +54,14 @@ func VerifyAPKClient(ctx context.Context, repository, runner, image string, scop } verificationCases := scope.selection(manifest.VerificationCases, apkCompare) if err := verifyCases(ctx, verificationCases, func(caseCtx context.Context, verification domain.VerificationCase) error { - return verifyAPKCase(caseCtx, runner, image, snapshot, verification) + return verifyAPKCase(caseCtx, runner, image, snapshot, manifest, verification) }); err != nil { return buildgraph.RepositoryManifest{}, 0, err } return manifest, len(verificationCases), nil } -func verifyAPKCase(ctx context.Context, runner, image, snapshot string, verification domain.VerificationCase) error { +func verifyAPKCase(ctx context.Context, runner, image, snapshot string, manifest buildgraph.RepositoryManifest, verification domain.VerificationCase) error { caseCtx, cancel := context.WithTimeout(ctx, 3*time.Minute) defer cancel() platform, err := apkOCIPlatform(verification.Architecture) @@ -86,6 +86,12 @@ func verifyAPKCase(ctx context.Context, runner, image, snapshot string, verifica "--volume", snapshot+":/target/repo:ro,Z", "--env", "SNAILMAIL_PACKAGE="+verification.Package, "--env", "SNAILMAIL_VERSION="+verification.Version, + "--env", "SNAILMAIL_BASEURL=/target/repo", + "--env", "SNAILMAIL_SIGNING_KEY="+manifest.Install.SigningKeyPath, + "--env", "SNAILMAIL_KEYSOURCE="+mountedKeySource(manifest.Install.SigningKeyPath, "/target/repo"), + // A mounted tree is read from disk, so no CA is involved; the script still + // reads the variable, and an unset one is fatal under set -u. + "--env", "SNAILMAIL_CA=", "--tmpfs", "/tmp:rw,exec,size=64m,mode=1777", reference, "sh", "-euc", apkVerificationScript, @@ -130,16 +136,42 @@ func apkOCIPlatform(architecture string) (string, error) { } } -// apkVerificationScript installs from the built tree alone. --allow-untrusted -// because signing an APKINDEX is not implemented yet, and --repositories-file -// so the distribution's own repositories cannot satisfy the request instead. +// apkVerificationScript installs from the repository under verification alone. +// --repositories-file so the distribution's own repositories cannot satisfy the +// request instead. +// +// SNAILMAIL_BASEURL is what makes this serve both cases: a mounted directory +// path for a built tree and the host's own URL for an endpoint probe. apk takes +// either in the same repositories file, so what is checked — the trust binding, +// the version pin — is the same code in both. +// +// A signed repository is installed with apk's trust checking on, which is the +// whole point of signing one: apk reads the key name out of the index's +// signature member and looks for that filename in /etc/apk/keys, so publishing +// the key under the name the index gives it is the entire binding. Without the +// key in place apk refuses the index as an UNTRUSTED signature. Only an +// unsigned repository falls back to --allow-untrusted, because there is nothing +// to check. const apkVerificationScript = ` -echo /target/repo > /tmp/repositories +echo "$SNAILMAIL_BASEURL" > /tmp/repositories test -n "$SNAILMAIL_PACKAGE" +if test -n "$SNAILMAIL_CA"; then + # The pinned image's own trust store is not necessarily the host's, and an + # endpoint served from a private CA is a supported deployment. + cp "$SNAILMAIL_CA" /etc/ssl/certs/ca-certificates.crt +fi +if test -n "$SNAILMAIL_SIGNING_KEY"; then + # apk finds the key by the filename the index names and nothing else, so the + # basename is the whole binding and has to survive being placed here. + cp "$SNAILMAIL_KEYSOURCE" "/etc/apk/keys/$(basename "$SNAILMAIL_SIGNING_KEY")" + trust="" +else + trust="--allow-untrusted" +fi # The request is pinned to the version under verification. Asking for the bare # name installs whatever is newest, which silently verifies the wrong package as # soon as the repository carries more than one version of it. -apk --repositories-file /tmp/repositories --allow-untrusted --no-cache \ +apk --repositories-file /tmp/repositories $trust --no-cache \ add "${SNAILMAIL_PACKAGE}=${SNAILMAIL_VERSION}" >/dev/null # The installed version must be the one the index advertised, or apk resolved # something other than what was published. "apk info -v" with a package name diff --git a/internal/app/deb_endpoint.go b/internal/app/deb_endpoint.go index 1b7d9a4..e45fb33 100644 --- a/internal/app/deb_endpoint.go +++ b/internal/app/deb_endpoint.go @@ -118,6 +118,14 @@ type endpointTrust struct { // fetchEndpointTrust downloads the published signing keyring and locates a CA // bundle, returning what has to be mounted into the verification container. +// +// Fetching the key rather than reading it out of the tree is deliberate and is +// half of what an endpoint probe buys: it proves the host serves the key, at the +// path the repository tells consumers to fetch it from, with the bytes the +// manifest recorded. A mounted copy proves none of that. +// +// It is format-neutral -- every signable format publishes exactly one key under +// Install.SigningKeyPath -- so all four endpoint probes share it. func fetchEndpointTrust(ctx context.Context, endpoint *url.URL, manifest buildgraph.RepositoryManifest) (endpointTrust, func(), error) { trust := endpointTrust{} noop := func() {} @@ -125,7 +133,7 @@ func fetchEndpointTrust(ctx context.Context, endpoint *url.URL, manifest buildgr trust.caBundle = systemCABundle() if trust.caBundle == "" { return endpointTrust{}, noop, errors.New( - "Debian endpoint verification over HTTPS needs a system CA bundle to mount, because the pinned image ships none") + "endpoint verification over HTTPS needs a system CA bundle to mount, because the pinned images ship none") } } keyPath := manifest.Install.SigningKeyPath @@ -154,7 +162,7 @@ func fetchEndpointTrust(ctx context.Context, endpoint *url.URL, manifest buildgr if hex.EncodeToString(digest[:]) != expected { return endpointTrust{}, noop, errors.New("published signing keyring does not match the verified repository") } - directory, err := os.MkdirTemp("", ".snailmail-deb-trust-*") + directory, err := os.MkdirTemp("", ".snailmail-trust-*") if err != nil { return endpointTrust{}, noop, err } @@ -308,3 +316,74 @@ if test -n "$SNAILMAIL_PACKAGE"; then test "$status" = "install ok installed $SNAILMAIL_VERSION" fi ` + +// The trust material as a verification container sees it. The paths are fixed +// rather than derived, so every format's script reads the same two names. +const ( + trustMountPath = "/snailmail-trust" + caMountPath = "/snailmail-ca.crt" +) + +// endpointKeySource is where the fetched signing key lands inside the container, +// in the same shape mountedKeySource produces for a mounted tree: empty when the +// repository published no key. +func endpointKeySource(trust endpointTrust) string { + if trust.keyringName == "" { + return "" + } + return trustMountPath + "/" + trust.keyringName +} + +// endpointCAPath is the mounted CA bundle, empty when the endpoint is plaintext +// loopback and none was needed. +func endpointCAPath(trust endpointTrust) string { + if trust.caBundle == "" { + return "" + } + return caMountPath +} + +func endpointTrustMounts(trust endpointTrust) []string { + mounts := []string(nil) + if trust.keyringDirectory != "" { + mounts = append(mounts, "--volume", trust.keyringDirectory+":"+trustMountPath+":ro,Z") + } + if trust.caBundle != "" { + mounts = append(mounts, "--volume", trust.caBundle+":"+caMountPath+":ro") + } + return mounts +} + +// localTrust stages the signing key out of a verified tree, in the same shape +// fetchEndpointTrust produces from an endpoint. +// +// It exists so a client is handed the key the manifest recorded in both cases. +// The alternative -- letting the container fetch the key from whatever is +// serving the repository -- verifies a signature against a key from the same +// place as the signature, which checks nothing. +func localTrust(root string, manifest buildgraph.RepositoryManifest) (endpointTrust, func(), error) { + noop := func() {} + keyPath := manifest.Install.SigningKeyPath + if keyPath == "" { + return endpointTrust{}, noop, nil + } + content, err := os.ReadFile(filepath.Join(root, filepath.FromSlash(keyPath))) + if err != nil { + return endpointTrust{}, noop, fmt.Errorf("read published signing key: %w", err) + } + directory, err := os.MkdirTemp("", ".snailmail-trust-*") + if err != nil { + return endpointTrust{}, noop, err + } + // The container reads this directory, so it must be traversable. + if err := os.Chmod(directory, 0o755); err != nil { + _ = os.RemoveAll(directory) + return endpointTrust{}, noop, err + } + trust := endpointTrust{keyringDirectory: directory, keyringName: filepath.Base(keyPath)} + if err := os.WriteFile(filepath.Join(directory, trust.keyringName), content, 0o644); err != nil { + _ = os.RemoveAll(directory) + return endpointTrust{}, noop, err + } + return trust, func() { _ = os.RemoveAll(directory) }, nil +} diff --git a/internal/app/deb_endpoint_test.go b/internal/app/deb_endpoint_test.go index 7814d94..8a53f48 100644 --- a/internal/app/deb_endpoint_test.go +++ b/internal/app/deb_endpoint_test.go @@ -138,14 +138,8 @@ func TestDebEndpointVerificationDetectsServedDrift(t *testing.T) { // host serves, resolves the package, and installs it. This is what separates a // correct tree from an installable one. func TestDebEndpointVerificationInstallsOverHTTP(t *testing.T) { - // A loopback endpoint needs the container to share the host's network - // namespace. On macOS the runtime is a Linux VM, so --network=host joins the - // VM rather than the host and the server here is unreachable. The path this - // proves is Linux-only in practice, which is also where it runs in CI. - if runtime.GOOS != "linux" { - t.Skip("loopback endpoint verification needs a container sharing the host network namespace") - } runner := containerRunner(t) + requireHostNetworkVerification(t, runner) repository := buildDebRepository(t, hostDebianArchitecture(t)) listener, err := net.Listen("tcp", "127.0.0.1:0") diff --git a/internal/app/endpoint_probe_test.go b/internal/app/endpoint_probe_test.go new file mode 100644 index 0000000..1cd7219 --- /dev/null +++ b/internal/app/endpoint_probe_test.go @@ -0,0 +1,266 @@ +package app + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "net" + "net/http" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + + "github.com/shellcell/snailmail/formats/apk" + "github.com/shellcell/snailmail/formats/helm" + "github.com/shellcell/snailmail/formats/rpm" + "github.com/shellcell/snailmail/host" + "github.com/shellcell/snailmail/internal/domain" + "github.com/shellcell/snailmail/internal/release" + "github.com/shellcell/snailmail/internal/testutil" +) + +// An endpoint probe needs two things this machine may not give it: a container +// that can be run with the resource limits verification asks for, and one that +// shares the host's network namespace well enough to reach a listener here. +// +// Rootless Docker gives neither -- it has no CPU cgroup to set, and its "host" +// network is its own namespace rather than the host's -- so a developer running +// the suite on it would see these fail for reasons that have nothing to do with +// the code. Probing for both and skipping is better than a suite whose failures +// have to be explained every time, and CI runs a runtime where it does not skip. +func requireHostNetworkVerification(t *testing.T, runner string) { + t.Helper() + if runtime.GOOS != "linux" { + testutil.MissingVerificationCapability(t, + "a loopback endpoint needs a container sharing the host network namespace", + "this platform runs containers in a virtual machine, whose namespace is not this one") + } + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + server := &http.Server{Handler: http.HandlerFunc(func(writer http.ResponseWriter, _ *http.Request) { + _, _ = writer.Write([]byte("reachable")) + }), ReadHeaderTimeout: 5 * time.Second} + go func() { _ = server.Serve(listener) }() + defer func() { _ = server.Close() }() + + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + command := exec.CommandContext(ctx, runner, "run", "--rm", "--network=host", "--cpus=2", + alpineVerificationImage, "wget", "-q", "-T", "5", "-O", "-", "http://"+listener.Addr().String()) + command.Env = runnerEnvironment() + if output, err := command.CombinedOutput(); err != nil { + testutil.MissingVerificationCapability(t, + "this runtime cannot run a host-networked verification container", strings.TrimSpace(string(output))) + } +} + +const ( + alpineVerificationImage = "docker.io/library/alpine@sha256:48b0309ca019d89d40f670aa1bc06e426dc0931948452e8491e3d65087abc07d" + fedoraVerificationImage = "docker.io/library/fedora@sha256:f1a3fab47bcb3c3ddf3135d5ee7ba8b7b25f2e809a47440936212a3a50957f3d" + helmVerificationImage = "docker.io/alpine/helm@sha256:e7ecbf4a200dea73d64bfb8cb0936829164945f2b4d02a0274093073ee8d264f" +) + +// serveRepository publishes a built tree on loopback and returns its URL. +func serveRepository(t *testing.T, repository string) string { + t.Helper() + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + server := &http.Server{Handler: http.FileServer(http.Dir(repository)), ReadHeaderTimeout: 10 * time.Second} + go func() { _ = server.Serve(listener) }() + t.Cleanup(func() { _ = server.Close() }) + return "http://" + listener.Addr().String() +} + +func blobOf(t *testing.T, source string, facts domain.PackageFacts) domain.Blob { + t.Helper() + content, err := os.ReadFile(source) + if err != nil { + t.Fatal(err) + } + digest := sha256.Sum256(content) + return domain.Blob{ + Filename: filepath.Base(source), Size: int64(len(content)), + SHA256: hex.EncodeToString(digest[:]), Facts: facts, + } +} + +func materialize(t *testing.T, artifact domain.RepositoryArtifact, blob domain.Blob, source string) string { + t.Helper() + output := filepath.Join(t.TempDir(), "repository") + if err := release.Materialize(context.Background(), output, mustFinalize(t, artifact), + map[string]string{blob.SHA256: source}); err != nil { + t.Fatal(err) + } + resolved, err := filepath.EvalSymlinks(output) + if err != nil { + t.Fatal(err) + } + return resolved +} + +func testdataPath(t *testing.T, elements ...string) string { + t.Helper() + return filepath.Join(append([]string{"..", "..", "formats"}, elements...)...) +} + +func buildRPMRepository(t *testing.T) string { + t.Helper() + source := testdataPath(t, "rpm", "testdata", "snail-demo-1.2.3-4.noarch.rpm") + file, err := os.Open(source) + if err != nil { + t.Fatal(err) + } + info, err := file.Stat() + if err != nil { + t.Fatal(err) + } + facts, err := rpm.Inspect(filepath.Base(source), file, info.Size()) + _ = file.Close() + if err != nil { + t.Fatal(err) + } + blob := blobOf(t, source, facts) + artifact, err := rpm.Build([]domain.Blob{blob}, rpm.BuildOptions{GeneratedAt: time.Unix(0, 0).UTC()}) + if err != nil { + t.Fatal(err) + } + return materialize(t, artifact, blob, source) +} + +func buildAPKRepository(t *testing.T) string { + t.Helper() + source := testdataPath(t, "apk", "testdata", "snail-demo-1.2.3-r4.apk") + file, err := os.Open(source) + if err != nil { + t.Fatal(err) + } + info, err := file.Stat() + if err != nil { + t.Fatal(err) + } + facts, err := apk.Inspect(filepath.Base(source), file, info.Size()) + _ = file.Close() + if err != nil { + t.Fatal(err) + } + blob := blobOf(t, source, facts) + artifact, err := apk.Build([]domain.Blob{blob}, apk.BuildOptions{ + Architectures: []string{"x86_64"}, GeneratedAt: time.Unix(0, 0).UTC(), + }) + if err != nil { + t.Fatal(err) + } + return materialize(t, artifact, blob, source) +} + +func buildHelmRepository(t *testing.T) string { + t.Helper() + source, err := testutil.WriteHelmChart(t.TempDir(), "demo", "1.2.3") + if err != nil { + t.Fatal(err) + } + file, err := os.Open(source) + if err != nil { + t.Fatal(err) + } + info, err := file.Stat() + if err != nil { + t.Fatal(err) + } + facts, err := helm.Inspect(filepath.Base(source), file, info.Size()) + _ = file.Close() + if err != nil { + t.Fatal(err) + } + blob := blobOf(t, source, facts) + artifact, err := helm.Build([]domain.Blob{blob}, helm.BuildOptions{GeneratedAt: time.Unix(0, 0).UTC()}) + if err != nil { + t.Fatal(err) + } + return materialize(t, artifact, blob, source) +} + +// dnf reads the repository over HTTP from the address the host serves, resolves +// the package, and installs it. Until this existed, rpm on a remote host was +// only ever verified against the staged directory, so nothing checked that +// repomd.xml's relative locations survived the host's URL handling. +func TestRPMEndpointVerificationInstallsOverHTTP(t *testing.T) { + runner := containerRunner(t) + requireHostNetworkVerification(t, runner) + repository := buildRPMRepository(t) + manifest, installed, err := VerifyRPMClientEndpointAccess(context.Background(), repository, + host.ClientAccess{Endpoint: serveRepository(t, repository)}, runner, fedoraVerificationImage, AllVersions) + if err != nil { + t.Fatalf("dnf could not install from the served repository: %v", err) + } + if manifest.Format != rpm.FormatID { + t.Fatalf("format = %q", manifest.Format) + } + if installed == 0 { + t.Fatal("no verification case ran") + } +} + +// apk asks for //APKINDEX.tar.gz and then for the package at +// the path that index gives, so this is where a host that rewrites either one is +// caught. +func TestAPKEndpointVerificationInstallsOverHTTP(t *testing.T) { + runner := containerRunner(t) + requireHostNetworkVerification(t, runner) + repository := buildAPKRepository(t) + manifest, installed, err := VerifyAPKClientEndpointAccess(context.Background(), repository, + host.ClientAccess{Endpoint: serveRepository(t, repository)}, runner, alpineVerificationImage, AllVersions) + if err != nil { + t.Fatalf("apk could not install from the served repository: %v", err) + } + if manifest.Format != apk.FormatID { + t.Fatalf("format = %q", manifest.Format) + } + if installed == 0 { + t.Fatal("no verification case ran") + } +} + +// helm resolves each chart through the URL index.yaml gives for it, which a host +// is free to serve from somewhere else entirely. +func TestHelmEndpointVerificationPullsOverHTTP(t *testing.T) { + runner := containerRunner(t) + requireHostNetworkVerification(t, runner) + repository := buildHelmRepository(t) + manifest, installed, err := VerifyHelmClientEndpointAccess(context.Background(), repository, + host.ClientAccess{Endpoint: serveRepository(t, repository)}, runner, helmVerificationImage) + if err != nil { + t.Fatalf("helm could not pull from the served repository: %v", err) + } + if manifest.Format != helm.FormatID { + t.Fatalf("format = %q", manifest.Format) + } + if installed == 0 { + t.Fatal("no verification case ran") + } +} + +// A host serving something other than the reviewed tree is refused before any +// client runs, for every format. This is the check that does not need a package +// manager, and it must not be skipped just because one format's client is the +// interesting part. +func TestEveryEndpointProbeRefusesBytesThatAreNotTheReviewedTree(t *testing.T) { + repository := buildRPMRepository(t) + tampered := serveRepository(t, t.TempDir()) // an empty directory serves none of the tree + _, _, err := VerifyRPMClientEndpointAccess(context.Background(), repository, + host.ClientAccess{Endpoint: tampered}, "docker", fedoraVerificationImage, AllVersions) + if err == nil { + t.Fatal("a host serving none of the reviewed tree was accepted") + } + if !strings.Contains(err.Error(), "do not match the reviewed tree") { + t.Fatalf("error = %v, want a served-bytes mismatch", err) + } +} diff --git a/internal/app/helm_endpoint.go b/internal/app/helm_endpoint.go new file mode 100644 index 0000000..ae6650b --- /dev/null +++ b/internal/app/helm_endpoint.go @@ -0,0 +1,86 @@ +package app + +import ( + "context" + "errors" + "fmt" + "os/exec" + + "github.com/shellcell/snailmail/formats/helm" + "github.com/shellcell/snailmail/host" + "github.com/shellcell/snailmail/internal/buildgraph" + "github.com/shellcell/snailmail/internal/domain" + endpointcheck "github.com/shellcell/snailmail/internal/endpoint" +) + +// VerifyHelmClientEndpointAccess pulls a chart from the repository the host is +// actually serving, rather than from a snapshot served here. +// +// Helm is the format where the two differ least in shape and most in what they +// prove. The staged check already runs helm against HTTP, but against a server +// this process controls: it answers every path, sets no cache headers, and +// returns index.yaml byte for byte. A real host may rewrite the URLs index.yaml +// gives for each chart, serve the .tgz with a transfer encoding that changes its +// bytes, or fail to serve the .prov at all -- and a chart that cannot be +// verified is a chart helm refuses to install with --verify. +func VerifyHelmClientEndpointAccess(ctx context.Context, root string, access host.ClientAccess, runner, image string) (buildgraph.RepositoryManifest, int, error) { + if access.Credential != nil { + defer access.Credential.Destroy() + } + endpoint, err := validateClientEndpoint(access.Endpoint) + if err != nil { + return buildgraph.RepositoryManifest{}, 0, err + } + // A private chart repository would need helm credentials inside the + // container; nothing issues them yet, so refuse rather than silently + // verifying an unauthenticated view. + if access.Credential != nil { + return buildgraph.RepositoryManifest{}, 0, errors.New("Helm endpoint verification does not support private read credentials") + } + manifest, _, err := verifyRepository(root) + if err != nil { + return buildgraph.RepositoryManifest{}, 0, err + } + if manifest.Format != helm.FormatID { + return buildgraph.RepositoryManifest{}, 0, fmt.Errorf("repository format is %q, not %q", manifest.Format, helm.FormatID) + } + if len(access.Routes) == 0 { + access.Routes, err = defaultClientRoutes(access.Endpoint, manifest.Files) + if err != nil { + return buildgraph.RepositoryManifest{}, 0, err + } + } + if err := awaitHostedBytes(ctx, access, "", "", false); err != nil { + return buildgraph.RepositoryManifest{}, 0, err + } + if runner == "" { + runner = "podman" + } + runner, err = exec.LookPath(runner) + if err != nil { + return buildgraph.RepositoryManifest{}, 0, fmt.Errorf("find container runner: %w", err) + } + if !digestPinnedImage(image) { + return buildgraph.RepositoryManifest{}, 0, errors.New("a digest-pinned Helm verification image is required") + } + trust, cleanup, err := fetchEndpointTrust(ctx, endpoint, manifest) + if err != nil { + return buildgraph.RepositoryManifest{}, 0, err + } + defer cleanup() + + var network []string + if endpointcheck.IsLoopback(endpoint.Hostname()) { + network = []string{"--network=host"} + } + verificationCases := manifest.VerificationCases + if len(verificationCases) == 0 { + verificationCases = []domain.VerificationCase{{}} + } + for _, verification := range verificationCases { + if err := verifyHelmCase(ctx, runner, image, access.Endpoint, network, trust, manifest, verification); err != nil { + return buildgraph.RepositoryManifest{}, 0, err + } + } + return manifest, len(manifest.VerificationCases), nil +} diff --git a/internal/app/rpm_endpoint.go b/internal/app/rpm_endpoint.go new file mode 100644 index 0000000..e17037b --- /dev/null +++ b/internal/app/rpm_endpoint.go @@ -0,0 +1,143 @@ +package app + +import ( + "context" + "errors" + "fmt" + "os/exec" + "strings" + "time" + + "github.com/shellcell/snailmail/formats/rpm" + "github.com/shellcell/snailmail/host" + "github.com/shellcell/snailmail/internal/buildgraph" + "github.com/shellcell/snailmail/internal/domain" + endpointcheck "github.com/shellcell/snailmail/internal/endpoint" +) + +// VerifyRPMClientEndpointAccess installs from a yum repository the host is +// actually serving, running dnf against the endpoint rather than a mounted +// directory. +// +// The mounted check proves the tree snailmail built is installable. It cannot +// prove the tree the host serves is, and for a signed repository those come +// apart in ways that matter: repomd.xml names each index by a relative location +// the host has to resolve the same way, repomd.xml.asc has to be served as the +// bytes that were signed rather than re-encoded, and the key has to be reachable +// at the path the install instructions send a consumer to. A host that gets any +// of those wrong publishes a repository nobody can install from, and until this +// existed nothing noticed until a consumer did. +func VerifyRPMClientEndpointAccess(ctx context.Context, root string, access host.ClientAccess, runner, image string, scope VersionScope) (buildgraph.RepositoryManifest, int, error) { + if access.Credential != nil { + defer access.Credential.Destroy() + } + endpoint, err := validateClientEndpoint(access.Endpoint) + if err != nil { + return buildgraph.RepositoryManifest{}, 0, err + } + // A private yum repository would need dnf credentials inside the container; + // nothing issues them yet, so refuse rather than silently verifying an + // unauthenticated view. + if access.Credential != nil { + return buildgraph.RepositoryManifest{}, 0, errors.New("RPM endpoint verification does not support private read credentials") + } + manifest, _, err := verifyRepository(root) + if err != nil { + return buildgraph.RepositoryManifest{}, 0, err + } + if manifest.Format != rpm.FormatID { + return buildgraph.RepositoryManifest{}, 0, fmt.Errorf("repository format is %q, not %q", manifest.Format, rpm.FormatID) + } + if len(access.Routes) == 0 { + access.Routes, err = defaultClientRoutes(access.Endpoint, manifest.Files) + if err != nil { + return buildgraph.RepositoryManifest{}, 0, err + } + } + if err := awaitHostedBytes(ctx, access, "", "", false); err != nil { + return buildgraph.RepositoryManifest{}, 0, err + } + if runner == "" { + runner = "podman" + } + runner, err = exec.LookPath(runner) + if err != nil { + return buildgraph.RepositoryManifest{}, 0, fmt.Errorf("find container runner: %w", err) + } + if !digestPinnedImage(image) { + return buildgraph.RepositoryManifest{}, 0, errors.New("a digest-pinned RPM verification image is required") + } + trust, cleanup, err := fetchEndpointTrust(ctx, endpoint, manifest) + if err != nil { + return buildgraph.RepositoryManifest{}, 0, err + } + defer cleanup() + + // A loopback endpoint is the host's loopback, not the container's, so it is + // only reachable with host networking. A public endpoint uses the runner's + // default network and stays isolated from the host. + hostNetwork := endpointcheck.IsLoopback(endpoint.Hostname()) + verificationCases := scope.selection(manifest.VerificationCases, rpmCompare) + if err := verifyCases(ctx, verificationCases, func(caseCtx context.Context, verification domain.VerificationCase) error { + return verifyRPMEndpointCase(caseCtx, runner, image, access.Endpoint, trust, hostNetwork, manifest, verification) + }); err != nil { + return buildgraph.RepositoryManifest{}, 0, err + } + return manifest, len(verificationCases), nil +} + +func verifyRPMEndpointCase(ctx context.Context, runner, image, endpoint string, trust endpointTrust, hostNetwork bool, manifest buildgraph.RepositoryManifest, verification domain.VerificationCase) error { + caseCtx, cancel := context.WithTimeout(ctx, 5*time.Minute) + defer cancel() + platform, err := rpmOCIPlatform(verification.Architecture) + if err != nil { + return err + } + reference, platformFlag := image, false + if platform != "" { + reference, platformFlag, err = platformImage(caseCtx, runner, image, platform) + if err != nil { + return err + } + } + arguments := []string{"run", "--rm", "--pull=missing"} + if hostNetwork { + arguments = append(arguments, "--network=host") + } + if platformFlag { + arguments = append(arguments, "--platform", platform) + } + arguments = append(arguments, + "--memory=2g", "--cpus=2", "--pids-limit=256", + "--ulimit", "fsize=536870912:536870912", "--ulimit", "nofile=4096:4096", + "--security-opt", "no-new-privileges", + "--env", "SNAILMAIL_PACKAGE="+verification.Package, + "--env", "SNAILMAIL_VERSION="+verification.Version, + "--env", "SNAILMAIL_BASEURL="+strings.TrimSuffix(endpoint, "/"), + "--env", "SNAILMAIL_SIGNING_KEY="+manifest.Install.SigningKeyPath, + "--env", "SNAILMAIL_KEYSOURCE="+endpointKeySource(trust), + "--env", "SNAILMAIL_CA="+endpointCAPath(trust), + // exec is explicit because runtimes mount tmpfs noexec by default and + // dnf runs scriptlets out of its working directories. + "--tmpfs", "/tmp:rw,exec,size=256m,mode=1777", + "--tmpfs", "/var/cache/dnf:rw,exec,size=512m,mode=0755", + ) + arguments = append(arguments, endpointTrustMounts(trust)...) + arguments = append(arguments, reference, "sh", "-euc", rpmVerificationScript) + + command := exec.CommandContext(caseCtx, runner, arguments...) + command.Env = runnerEnvironment() + output, err := command.CombinedOutput() + if err != nil { + if registryUnavailable(output) { + return fmt.Errorf("%w: %s", ErrVerificationImageUnavailable, strings.TrimSpace(string(output))) + } + if foreignPlatformUnsupported(output) { + return fmt.Errorf("%w: verifying %s needs QEMU and binfmt_misc registered for it", + ErrForeignPlatformUnsupported, verification.Architecture) + } + return fmt.Errorf("RPM endpoint verification for %s=%s/%s: %w\n%s", + verification.Package, verification.Version, verification.Architecture, err, strings.TrimSpace(string(output))) + } + return nil +} diff --git a/internal/app/rpm_verify.go b/internal/app/rpm_verify.go index e14b3b8..8e75986 100644 --- a/internal/app/rpm_verify.go +++ b/internal/app/rpm_verify.go @@ -57,14 +57,14 @@ func VerifyRPMClient(ctx context.Context, repository, runner, image string, scop } verificationCases := scope.selection(manifest.VerificationCases, rpmCompare) if err := verifyCases(ctx, verificationCases, func(caseCtx context.Context, verification domain.VerificationCase) error { - return verifyRPMCase(caseCtx, runner, image, snapshot, verification) + return verifyRPMCase(caseCtx, runner, image, snapshot, manifest, verification) }); err != nil { return buildgraph.RepositoryManifest{}, 0, err } return manifest, len(verificationCases), nil } -func verifyRPMCase(ctx context.Context, runner, image, snapshot string, verification domain.VerificationCase) error { +func verifyRPMCase(ctx context.Context, runner, image, snapshot string, manifest buildgraph.RepositoryManifest, verification domain.VerificationCase) error { caseCtx, cancel := context.WithTimeout(ctx, 5*time.Minute) defer cancel() platform, err := rpmOCIPlatform(verification.Architecture) @@ -89,6 +89,12 @@ func verifyRPMCase(ctx context.Context, runner, image, snapshot string, verifica "--volume", snapshot+":/target/repo:ro,Z", "--env", "SNAILMAIL_PACKAGE="+verification.Package, "--env", "SNAILMAIL_VERSION="+verification.Version, + "--env", "SNAILMAIL_BASEURL=file:///target/repo", + "--env", "SNAILMAIL_SIGNING_KEY="+manifest.Install.SigningKeyPath, + "--env", "SNAILMAIL_KEYSOURCE="+mountedKeySource(manifest.Install.SigningKeyPath, "/target/repo"), + // A mounted tree is read from disk, so no CA is involved; the script still + // reads the variable, and an unset one is fatal under set -u. + "--env", "SNAILMAIL_CA=", // exec is explicit because runtimes mount tmpfs noexec by default and // dnf runs scriptlets out of its working directories. "--tmpfs", "/tmp:rw,exec,size=256m,mode=1777", @@ -134,22 +140,69 @@ func rpmOCIPlatform(architecture string) (string, error) { } } -// rpmVerificationScript configures the built tree as the only repository and -// installs from it. Every other repository is disabled so the package can only -// come from the tree under test, and gpgcheck is off because signing a yum -// repository is not implemented yet — which is exactly what the knowledge -// bundle records. +// mountedKeySource is where the published signing key sits inside the container +// when the repository is a mounted directory. Empty stays empty, because that is +// how a script is told the repository is unsigned. +func mountedKeySource(keyPath, root string) string { + if keyPath == "" { + return "" + } + return root + "/" + keyPath +} + +// rpmVerificationScript configures the repository under verification as the only +// one dnf may use and installs from it. Every other repository is disabled so the +// package can only come from the repository under test. +// +// SNAILMAIL_BASEURL is what makes this serve both cases: a file:// URL for a +// mounted tree and the host's own URL for an endpoint probe. Everything the +// check depends on — how the signature is verified, how the version is pinned — +// is then the same code in both, so the endpoint case cannot quietly check less +// than the mounted one. +// +// gpgcheck stays off because snailmail signs a yum repository's metadata and +// not the packages inside it: repomd.xml.asc covers the index chain, and the +// index chain covers each package by checksum. repo_gpgcheck is what checks +// that signature, and it is on exactly when the repository published one. const rpmVerificationScript = ` -cat > /etc/yum.repos.d/snailmail.repo <<'REPO' +cat > /etc/yum.repos.d/snailmail.repo </dev/null +if test -n "$SNAILMAIL_CA"; then + # The pinned image's own trust store is not necessarily the host's, and an + # endpoint served from a private CA is a supported deployment. + printf 'sslcacert=%s\n' "$SNAILMAIL_CA" >>/etc/yum.repos.d/snailmail.repo +fi +if test -n "$SNAILMAIL_SIGNING_KEY"; then + printf 'repo_gpgcheck=1\ngpgkey=file://%s\n' "$SNAILMAIL_KEYSOURCE" >>/etc/yum.repos.d/snailmail.repo +else + printf 'repo_gpgcheck=0\n' >>/etc/yum.repos.d/snailmail.repo +fi +# dnf imports the signing key on first contact and reports "signing key not +# found" while doing so, which makes a correctly signed repository and a broken +# one look alike on the first pass. So the first pass is the import and the +# second is the check. +dnf -y --disablerepo='*' --enablerepo=snailmail makecache >/dev/null 2>&1 || true +cache=$(dnf -y --disablerepo='*' --enablerepo=snailmail makecache --refresh 2>&1) || { + echo "$cache" >&2 + exit 1 +} +# repo_gpgcheck is the enforcement: dnf will not install from metadata that does +# not verify. But it reports the refusal and exits 0, so the install below fails +# as "no match for argument" — indistinguishable from a package that was never +# published. Reading the report is what names the real cause. If dnf ever changes +# the wording, the install still fails; only this message is lost. +case "$cache" in + *"GPG signature verification error"*) + echo "$cache" >&2 + echo "the repository signature did not verify against $SNAILMAIL_SIGNING_KEY" >&2 + exit 1 ;; +esac test -n "$SNAILMAIL_PACKAGE" # Pinned to the version under verification: the bare name resolves to whatever # is newest, which verifies the wrong package once the repository carries two. diff --git a/internal/app/signed_verification_test.go b/internal/app/signed_verification_test.go new file mode 100644 index 0000000..e95735e --- /dev/null +++ b/internal/app/signed_verification_test.go @@ -0,0 +1,100 @@ +package app + +import ( + "strings" + "testing" + + "github.com/shellcell/snailmail/formats" +) + +// signingKeyVariable is how a verification script learns that the repository it +// is about to install from published a signing key, and where. +const signingKeyVariable = `$SNAILMAIL_SIGNING_KEY` + +// clientCheckWaivers are the ways each client is told not to check a signature. +// Every one of them is legitimate for an unsigned repository and none of them is +// legitimate for a signed one. +var clientCheckWaivers = map[string][]string{ + "deb": {"trusted=yes"}, + "rpm": {"repo_gpgcheck=0"}, + "apk": {"--allow-untrusted"}, + "helm": nil, +} + +// verificationScripts is what each ecosystem's client actually runs. A format +// that signs but has no entry here is the failure this file exists to catch: +// snailmail produced a signature and then verified the repository without ever +// asking the client whether it holds. +// +// That is not hypothetical. rpm and apk signing shipped with their verification +// scripts still passing --allow-untrusted and gpgcheck=0, and helm published a +// .prov per chart that `helm pull` never read, so a repository whose signature +// no client would accept verified green. +var verificationScripts = map[string]string{ + "deb": debVerificationScript, + "rpm": rpmVerificationScript, + "apk": apkVerificationScript, + "helm": helmVerificationScript, +} + +func TestEverySignableFormatHasItsSignatureCheckedByItsClient(t *testing.T) { + signable := 0 + for _, format := range formats.All() { + if !format.ImplementsSigning() { + continue + } + signable++ + script, present := verificationScripts[format.Name()] + if !present { + t.Errorf("format %q signs repositories but has no verification script here", format.Name()) + continue + } + if !strings.Contains(script, signingKeyVariable) { + t.Errorf("format %q signs repositories, but its client verification never reads %s, "+ + "so a signature no client would accept verifies green", format.Name(), signingKeyVariable) + } + } + if signable == 0 { + t.Fatal("no format reported that it signs repositories, so this test checked nothing") + } +} + +// A waiver ahead of the branch that decides whether the repository is signed is +// a waiver that applies to signed repositories too. +func TestNoClientIsToldToSkipTheCheckBeforeAskingWhetherThereIsOne(t *testing.T) { + for format, script := range verificationScripts { + decision := strings.Index(script, signingKeyVariable) + if decision < 0 { + continue // Reported by the test above. + } + for _, waiver := range clientCheckWaivers[format] { + at := strings.Index(script, waiver) + if at < 0 { + continue + } + if at < decision { + t.Errorf("%s verification passes %q before it has looked at %s, "+ + "so a signed repository is installed without its signature being checked", + format, waiver, signingKeyVariable) + } + } + } +} + +// The waiver list is only meaningful while it names strings the scripts could +// actually contain, so it is held to the scripts rather than left to rot. +func TestEveryWaiverNamesSomethingAScriptStillDoes(t *testing.T) { + for format, waivers := range clientCheckWaivers { + script, present := verificationScripts[format] + if !present { + t.Errorf("waivers are declared for %q, which has no verification script", format) + continue + } + for _, waiver := range waivers { + if !strings.Contains(script, waiver) { + t.Errorf("%s verification no longer contains %q; the waiver list is stale", + format, waiver) + } + } + } +} diff --git a/internal/app/verify.go b/internal/app/verify.go index 86bada4..d121df3 100644 --- a/internal/app/verify.go +++ b/internal/app/verify.go @@ -518,7 +518,28 @@ func verifyDebCase(ctx context.Context, runner, image, snapshot string, workspac // chroot below runs apt-get and dpkg-query out of this mount. "--tmpfs", "/target:rw,exec,size="+strconv.FormatInt(workspaceBytes, 10)+",mode=0755", reference, - "sh", "-euc", ` + "sh", "-euc", debVerificationScript) + command := exec.CommandContext(caseCtx, runner, arguments...) + command.Env = runnerEnvironment() + output, err := command.CombinedOutput() + if err != nil { + if registryUnavailable(output) { + return fmt.Errorf("%w: %s", ErrVerificationImageUnavailable, strings.TrimSpace(string(output))) + } + if foreignPlatformUnsupported(output) { + return fmt.Errorf("%w: verifying %s needs QEMU and binfmt_misc registered for it", + ErrForeignPlatformUnsupported, verification.Architecture) + } + return fmt.Errorf("Debian client verification for %s=%s/%s: %w\n%s", verification.Package, verification.Version, verification.Architecture, err, strings.TrimSpace(string(output))) + } + return nil +} + +// debVerificationScript builds a chroot from the image and installs out of the +// mounted tree with apt. A signed repository is pinned to its own key with +// signed-by, so apt checks the Release signature the publication made; only an +// unsigned one falls back to trusted=yes, because there is nothing to check. +const debVerificationScript = ` for directory in bin etc lib lib64 sbin usr var; do test -e "/$directory" && cp -a "/$directory" /target/ done @@ -538,22 +559,7 @@ if test -n "$SNAILMAIL_PACKAGE"; then status=$(chroot /target dpkg-query -W -f='${Status} ${Version}' "$SNAILMAIL_PACKAGE") test "$status" = "install ok installed $SNAILMAIL_VERSION" fi -`) - command := exec.CommandContext(caseCtx, runner, arguments...) - command.Env = runnerEnvironment() - output, err := command.CombinedOutput() - if err != nil { - if registryUnavailable(output) { - return fmt.Errorf("%w: %s", ErrVerificationImageUnavailable, strings.TrimSpace(string(output))) - } - if foreignPlatformUnsupported(output) { - return fmt.Errorf("%w: verifying %s needs QEMU and binfmt_misc registered for it", - ErrForeignPlatformUnsupported, verification.Architecture) - } - return fmt.Errorf("Debian client verification for %s=%s/%s: %w\n%s", verification.Package, verification.Version, verification.Architecture, err, strings.TrimSpace(string(output))) - } - return nil -} +` func debWorkspaceSize(blobs []domain.Blob, maximum int64) (int64, error) { const ( @@ -660,8 +666,17 @@ func VerifyHelmClient(ctx context.Context, root, runner, image string) (buildgra if len(verificationCases) == 0 { verificationCases = []domain.VerificationCase{{}} } + // The key is mounted rather than fetched by the script, so the container + // verifies against the key the manifest recorded instead of whatever the + // server happens to answer with. + trust, cleanup, err := localTrust(snapshot, manifest) + if err != nil { + return buildgraph.RepositoryManifest{}, 0, err + } + defer cleanup() for _, verification := range verificationCases { - if err := verifyHelmCase(ctx, runner, image, snapshotHTTP, verification); err != nil { + if err := verifyHelmCase(ctx, runner, image, snapshotHTTP.endpoint, snapshotHTTP.runnerArguments, + trust, manifest, verification); err != nil { return buildgraph.RepositoryManifest{}, 0, err } } @@ -720,11 +735,11 @@ func serveSnapshot(snapshot string) (snapshotServer, error) { return result, nil } -func verifyHelmCase(ctx context.Context, runner, image string, snapshotHTTP snapshotServer, verification domain.VerificationCase) error { +func verifyHelmCase(ctx context.Context, runner, image, endpoint string, network []string, trust endpointTrust, manifest buildgraph.RepositoryManifest, verification domain.VerificationCase) error { caseCtx, cancel := context.WithTimeout(ctx, 3*time.Minute) defer cancel() arguments := []string{"run", "--rm", "--pull=missing"} - arguments = append(arguments, snapshotHTTP.runnerArguments...) + arguments = append(arguments, network...) arguments = append(arguments, "--read-only", "--memory=768m", "--cpus=2", "--pids-limit=256", "--ulimit", "fsize=536870912:536870912", "--ulimit", "nofile=1024:1024", @@ -735,22 +750,18 @@ func verifyHelmCase(ctx context.Context, runner, image string, snapshotHTTP snap "--env", "HELM_CACHE_HOME=/home/helm/cache", "--env", "HELM_CONFIG_HOME=/home/helm/config", "--env", "HELM_DATA_HOME=/home/helm/data", - "--env", "SNAILMAIL_ENDPOINT="+snapshotHTTP.endpoint, + "--env", "SNAILMAIL_ENDPOINT="+strings.TrimSuffix(endpoint, "/"), "--env", "SNAILMAIL_CHART="+verification.Project, "--env", "SNAILMAIL_VERSION="+verification.Version, + "--env", "SNAILMAIL_SIGNING_KEY="+manifest.Install.SigningKeyPath, + "--env", "SNAILMAIL_KEYSOURCE="+endpointKeySource(trust), + "--env", "SNAILMAIL_CA="+endpointCAPath(trust), + ) + arguments = append(arguments, endpointTrustMounts(trust)...) + arguments = append(arguments, "--entrypoint", "/bin/sh", image, - "-euc", ` -helm repo add staged "$SNAILMAIL_ENDPOINT" -helm repo update staged -if test -n "$SNAILMAIL_CHART"; then - mkdir -p /tmp/chart - helm pull "staged/$SNAILMAIL_CHART" --version "$SNAILMAIL_VERSION" --untar --untardir /tmp/chart - helm show chart "staged/$SNAILMAIL_CHART" --version "$SNAILMAIL_VERSION" >/tmp/chart-metadata.yaml - helm lint "/tmp/chart/$SNAILMAIL_CHART" - helm template snailmail "/tmp/chart/$SNAILMAIL_CHART" >/tmp/rendered.yaml -fi -`) + "-euc", helmVerificationScript) command := exec.CommandContext(caseCtx, runner, arguments...) command.Env = runnerEnvironment() output, err := command.CombinedOutput() @@ -760,6 +771,32 @@ fi return nil } +// helmVerificationScript pulls the chart from a served repository and renders +// it. A signed repository is pulled with --verify against the keyring it +// publishes, so the .prov beside each chart is checked by helm rather than only +// written by snailmail; an unsigned one has no provenance to check. +const helmVerificationScript = ` +helm repo add staged "$SNAILMAIL_ENDPOINT" +helm repo update staged +if test -n "$SNAILMAIL_CA"; then + cp "$SNAILMAIL_CA" /etc/ssl/certs/ca-certificates.crt +fi +verify="" +if test -n "$SNAILMAIL_SIGNING_KEY"; then + # The keyring is the one the repository publishes, checked against the + # manifest before it was mounted, so verification uses the key a consumer + # would install rather than whatever the server answers with. + verify="--verify --keyring $SNAILMAIL_KEYSOURCE" +fi +if test -n "$SNAILMAIL_CHART"; then + mkdir -p /tmp/chart + helm pull "staged/$SNAILMAIL_CHART" --version "$SNAILMAIL_VERSION" $verify --untar --untardir /tmp/chart + helm show chart "staged/$SNAILMAIL_CHART" --version "$SNAILMAIL_VERSION" >/tmp/chart-metadata.yaml + helm lint "/tmp/chart/$SNAILMAIL_CHART" + helm template snailmail "/tmp/chart/$SNAILMAIL_CHART" >/tmp/rendered.yaml +fi +` + func snapshotRepository(ctx context.Context, source string, manifest buildgraph.RepositoryManifest) (string, error) { // Beside the repository rather than in the system temp directory, so the // hard links below always have a filesystem to be made on. A snapshot on diff --git a/internal/state/store.go b/internal/state/store.go index 18ad1aa..0e21867 100644 --- a/internal/state/store.go +++ b/internal/state/store.go @@ -209,7 +209,7 @@ func Setup(root string, options SetupOptions) error { } func writeInstallDocument(root, name string, repository Repository, keys map[string]SigningKey) error { - if !host.Supports(repository.Host.Type, repository.Format).InstallDocument { + if !PublishesInstallDocument(repository) { return nil } content := installDocumentContent(name, repository, keys) @@ -221,7 +221,7 @@ func writeInstallDocument(root, name string, repository Repository, keys map[str } func ValidateInstallDocument(root, name string, repository Repository, keys map[string]SigningKey) error { - if !host.Supports(repository.Host.Type, repository.Format).InstallDocument { + if !PublishesInstallDocument(repository) { return nil } filename, err := WorkspacePath(root, filepath.ToSlash(filepath.Join("docs", "install-"+name+".md"))) @@ -238,6 +238,24 @@ func ValidateInstallDocument(root, name string, repository Repository, keys map[ return nil } +// PublishesInstallDocument reports whether a repository has consumer +// instructions to write. +// +// The condition is an address a consumer can fetch from, and nothing else. It +// used to be declared per host-and-format pair in the support matrix, which was +// a restatement: every remote pair said yes and every local one said no, so what +// it really encoded was "this host type has a public URL". +// +// Encoding it that way was also wrong in one direction. A local repository is a +// directory something else serves, and the operator can record that URL -- +// ValidateRepository accepts one precisely so the instructions have an address +// to name -- but the matrix refused to write them anything. Asking for the +// endpoint answers both cases with the fact that actually matters, and a +// repository without one gets no document rather than one saying nothing. +func PublishesInstallDocument(repository Repository) bool { + return repository.Host.CanonicalEndpoint != "" +} + // installDocumentContent renders the consumer instructions for a repository. // ARCHITECTURE §6.5 requires these to be generated rather than hand-written, // so they cannot advertise a layout the repository does not serve. @@ -735,6 +753,19 @@ func ValidateGateConfiguration(policy string, approvalKeys []string, forgeReposi return nil } +// DefaultKeyBackend is what holds a signing key unless something else is named. +// It is the one backend that can create a key, so it is what `keys new` uses +// when an operator says nothing. +const DefaultKeyBackend = "file" + +// KnownKeyBackend reports whether a manifest names a backend snailmail can +// resolve a key through. A workspace written by a newer snailmail can name one +// this build has never heard of, and that has to be an error rather than a key +// silently resolved somewhere else. +func KnownKeyBackend(backend string) bool { + return backend == DefaultKeyBackend || backend == "env" +} + func validateSigningKeys(manifest Manifest) error { fingerprints := make(map[string]string) for name, key := range manifest.Keys { @@ -750,7 +781,7 @@ func validateSigningKeys(manifest Manifest) error { if !identifierPattern.MatchString(name) || (key.Algorithm != "openpgp-rsa4096" && key.Algorithm != "apk-rsa4096") || key.Usage != "sign" || !fingerprintPattern.MatchString(key.Fingerprint) || !hexdigest.ValidSHA256(key.PublicKeySHA256) || !hexdigest.ValidSHA256(key.PublicArmorSHA256) || key.PublicKeyPath != binaryPath || - key.PublicArmorPath != armorPath || key.Ref.Backend != "file" || key.Ref.ID != manifest.Workspace.ID+"/"+name { + key.PublicArmorPath != armorPath || !KnownKeyBackend(key.Ref.Backend) || key.Ref.ID != manifest.Workspace.ID+"/"+name { return fmt.Errorf("signing key %q has invalid identity or public forms", name) } createdAt, createdErr := time.Parse(time.RFC3339, key.CreatedAt) diff --git a/internal/state/store_test.go b/internal/state/store_test.go index 4fdc90f..49520ef 100644 --- a/internal/state/store_test.go +++ b/internal/state/store_test.go @@ -176,6 +176,81 @@ func TestValidateInstallDocumentRejectsEndpointDrift(t *testing.T) { } } +// Whether consumer instructions are written is decided by whether there is an +// address to put in them, not by which host type serves the tree. +// +// The two cases below are the ones the support matrix used to get wrong in +// opposite directions. A local repository is a directory something else serves, +// and ValidateRepository accepts the URL of whatever serves it -- for exactly +// this reason -- but the matrix said local hosts publish no instructions, so an +// operator who supplied the address got nothing that used it. An rsync +// repository without one got the reverse: a document that had no address to +// name and so said only that there was nothing to say. +func TestInstallInstructionsFollowTheAddressRatherThanTheHostType(t *testing.T) { + for _, repository := range []struct { + what string + wanted bool + subject Repository + }{ + { + what: "a local repository whose serving URL the operator recorded", wanted: true, + subject: Repository{Format: "pypi", Visibility: "public", Host: HostConfig{ + Type: "local", Path: "public/python", CanonicalEndpoint: "https://packages.example/python", + }}, + }, + { + what: "a local repository nothing is known to serve", wanted: false, + subject: Repository{Format: "pypi", Visibility: "public", Host: HostConfig{ + Type: "local", Path: "public/python", + }}, + }, + { + what: "an rsync repository with no recorded URL", wanted: false, + subject: Repository{Format: "deb", Visibility: "public", Host: HostConfig{ + Type: "rsync", Target: "deploy@packages.example", Path: "/srv/apt", + }}, + }, + } { + if PublishesInstallDocument(repository.subject) != repository.wanted { + t.Errorf("%s: publishes instructions = %v, want %v", + repository.what, !repository.wanted, repository.wanted) + } + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "docs"), 0o755); err != nil { + t.Fatal(err) + } + if err := writeInstallDocument(root, "python", repository.subject, nil); err != nil { + t.Fatal(err) + } + _, err := os.Stat(filepath.Join(root, "docs", "install-python.md")) + if written := err == nil; written != repository.wanted { + t.Errorf("%s: wrote a document = %v, want %v", repository.what, written, repository.wanted) + } + } +} + +// And a document that was written names the address it was written for, or the +// instructions send a consumer somewhere else. +func TestALocalRepositoryDocumentsTheURLItWasGiven(t *testing.T) { + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "docs"), 0o755); err != nil { + t.Fatal(err) + } + repository := Repository{Format: "pypi", Visibility: "public", Host: HostConfig{ + Type: "local", Path: "public/python", CanonicalEndpoint: "https://packages.example/python", + }} + if err := writeInstallDocument(root, "python", repository, nil); err != nil { + t.Fatal(err) + } + content, err := os.ReadFile(filepath.Join(root, "docs", "install-python.md")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(content), "https://packages.example/python") { + t.Errorf("the document does not name the address it was written for:\n%s", content) + } +} + func TestPrivateS3HostRequiresNonSecretBasicBrokerReference(t *testing.T) { repository := Repository{ Format: "pypi", Visibility: "private", @@ -274,3 +349,24 @@ func TestRepositoryGateValidation(t *testing.T) { } } } + +// A manifest names the backend that holds each private key, and this build has +// to refuse one it cannot resolve rather than looking somewhere else. +// +// The failure that matters is forward-looking: a workspace written by a newer +// snailmail can name a backend this build has never heard of. Treating that as +// the file backend would search a directory the key was never in and report it +// missing, which sends whoever reads it looking in the wrong place. +func TestOnlyAResolvableKeyBackendIsAccepted(t *testing.T) { + for backend, wanted := range map[string]bool{ + DefaultKeyBackend: true, + "env": true, + "kms": false, + "FILE": false, + "": false, + } { + if KnownKeyBackend(backend) != wanted { + t.Errorf("KnownKeyBackend(%q) = %v, want %v", backend, !wanted, wanted) + } + } +} diff --git a/internal/testutil/verification.go b/internal/testutil/verification.go new file mode 100644 index 0000000..e000cf7 --- /dev/null +++ b/internal/testutil/verification.go @@ -0,0 +1,41 @@ +package testutil + +import ( + "os" + "testing" +) + +// RequiredClientVerificationEnvironment names the variable that turns a missing +// verification capability from a skip into a failure. +// +// Client verification is the coverage this project sells: it runs a real apt, +// dnf, apk, pip or helm over the bytes a publication would serve. Those tests +// skip where the machine cannot run the container they need -- rootless Docker +// has no delegated CPU cgroup and refuses --cpus, and its "host" network is its +// own namespace rather than the host's -- because a developer should see what +// was checked rather than a daemon error that says nothing about snailmail. +// +// A skip in CI is indistinguishable from a pass. If a runner image changes and +// the capability goes away, every one of those tests would stop running and the +// build would stay green, which is the failure this variable exists to prevent. +// CI sets it, so a machine that is supposed to have the capability reports its +// absence as a failure naming what was missing. +// +// This is the same argument the workflow already makes for binfmt_misc, where +// registering the handlers is asserted rather than assumed "so the coverage +// silently disappears rather than failing". +const RequiredClientVerificationEnvironment = "SNAILMAIL_REQUIRE_CLIENT_VERIFICATION" + +// MissingVerificationCapability reports something client verification needs and +// this machine does not have -- a container runtime, the limits one is run +// with, a network path into it, or the interpreter pip runs under. +// +// what describes the capability in a sentence; detail is what was reported. +func MissingVerificationCapability(t *testing.T, what, detail string) { + t.Helper() + if os.Getenv(RequiredClientVerificationEnvironment) != "" { + t.Fatalf("%s: %s\n%s is set, so this has to work here rather than be skipped", + what, detail, RequiredClientVerificationEnvironment) + } + t.Skipf("%s: %s", what, detail) +} diff --git a/internal/wire/signers.go b/internal/wire/signers.go index 0c282f6..93cd23a 100644 --- a/internal/wire/signers.go +++ b/internal/wire/signers.go @@ -2,13 +2,17 @@ package wire import ( "bytes" + "context" "errors" "fmt" "io" "os" "path/filepath" + "time" + envsigner "github.com/shellcell/snailmail/adapters/signer/env" filesigner "github.com/shellcell/snailmail/adapters/signer/file" + "github.com/shellcell/snailmail/signer" ) const SigningPassphraseEnvironment = "SNAILMAIL_KEY_PASSPHRASE" @@ -30,7 +34,14 @@ const MinimumSigningPassphraseBytes = 24 // at an arbitrary path fails cleanly instead of reading it into memory. const maxSigningPassphraseBytes = 4 << 10 -func NewSignerStore() (*filesigner.Store, error) { +// NewSignerStore selects the backend a key reference names. +// +// This is the composition root's job and nowhere else's: ARCHITECTURE §2 puts +// driver selection here, and the engine asks for a key by reference without +// knowing what holds it. Until there was a second backend there was nothing to +// select, and the file store was handed straight to the engine -- which is why +// the reference's backend field existed and was never read. +func NewSignerStore() (*Signers, error) { dataHome := os.Getenv("XDG_DATA_HOME") if dataHome == "" { home, err := os.UserHomeDir() @@ -39,7 +50,72 @@ func NewSignerStore() (*filesigner.Store, error) { } dataHome = filepath.Join(home, ".local", "share") } - return filesigner.New(filepath.Join(dataHome, "snailmail", "private-keys"), signingPassphrase) + file, err := filesigner.New(filepath.Join(dataHome, "snailmail", "private-keys"), signingPassphrase) + if err != nil { + return nil, err + } + environment, err := envsigner.New(signingPassphrase) + if err != nil { + return nil, err + } + return &Signers{file: file, environment: environment}, nil +} + +// Signers routes a key reference to the backend that holds it. +type Signers struct { + file *filesigner.Store + environment *envsigner.Store +} + +// backend is what both of them can do. Keeping it to one interface means a +// backend that cannot generate says so when asked rather than by being absent +// from a table, which is the difference between an error naming what to do and +// one saying the key does not exist. +type backend interface { + signer.Generator + signer.Resolver +} + +func (signers *Signers) backendFor(ref signer.Ref) (backend, error) { + switch ref.Backend { + case "", "file": + return signers.file, nil + case envsigner.Backend: + return signers.environment, nil + } + return nil, fmt.Errorf("unknown signing key backend %q; known backends are file and %s", ref.Backend, envsigner.Backend) +} + +func (signers *Signers) Resolve(ctx context.Context, ref signer.Ref) (signer.Signer, error) { + selected, err := signers.backendFor(ref) + if err != nil { + return nil, err + } + return selected.Resolve(ctx, ref) +} + +func (signers *Signers) Generate(ctx context.Context, ref signer.Ref, algorithm signer.Algorithm, name string, createdAt time.Time, expiresIn time.Duration) (signer.Generated, error) { + selected, err := signers.backendFor(ref) + if err != nil { + return signer.Generated{}, err + } + return selected.Generate(ctx, ref, algorithm, name, createdAt, expiresIn) +} + +func (signers *Signers) Public(ctx context.Context, ref signer.Ref) (signer.Generated, error) { + selected, err := signers.backendFor(ref) + if err != nil { + return signer.Generated{}, err + } + return selected.Public(ctx, ref) +} + +func (signers *Signers) Delete(ctx context.Context, ref signer.Ref) error { + selected, err := signers.backendFor(ref) + if err != nil { + return err + } + return selected.Delete(ctx, ref) } func signingPassphrase() ([]byte, error) { diff --git a/internal/wire/signers_test.go b/internal/wire/signers_test.go index 35be12e..3799605 100644 --- a/internal/wire/signers_test.go +++ b/internal/wire/signers_test.go @@ -5,6 +5,9 @@ import ( "path/filepath" "strings" "testing" + + envsigner "github.com/shellcell/snailmail/adapters/signer/env" + "github.com/shellcell/snailmail/signer" ) const validPassphrase = "a-secret-manager-value-long-enough" @@ -95,3 +98,52 @@ func TestSigningPassphraseReportsAMissingFile(t *testing.T) { t.Fatal("expected a missing passphrase file to be reported") } } + +// A key reference names the backend that holds it, and this is the only place +// that reads it. Until there was a second backend the field was carried through +// every layer and never consulted, so the dispatch is worth holding to its +// contract: each backend reachable, and an unknown one refused rather than +// quietly resolved by whichever is the default. +func TestAKeyReferenceReachesTheBackendItNames(t *testing.T) { + t.Setenv(SigningPassphraseEnvironment, "wire-dispatch-test-passphrase-0001") + t.Setenv("XDG_DATA_HOME", t.TempDir()) + signers, err := NewSignerStore() + if err != nil { + t.Fatal(err) + } + const workspace = "1111111111111111111111111111111111111111111111111111111111111111" + for _, backend := range []string{"", "file", envsigner.Backend} { + if _, err := signers.backendFor(signer.Ref{Backend: backend, ID: workspace + "/archive"}); err != nil { + t.Errorf("backend %q is not reachable: %v", backend, err) + } + } + // A workspace written by a newer snailmail can name a backend this build has + // never heard of. Falling back to the file store would look for a key that + // was never there and report it missing, which sends an operator looking in + // the wrong place entirely. + _, err = signers.backendFor(signer.Ref{Backend: "kms", ID: workspace + "/archive"}) + if err == nil || !strings.Contains(err.Error(), "kms") { + t.Errorf("error = %v, want the unknown backend named", err) + } + if err != nil && !strings.Contains(err.Error(), "file") { + t.Errorf("error = %v, want it to say which backends exist", err) + } +} + +// The empty backend is what every workspace written before backends existed +// carries, and it has to keep resolving to the file store. +func TestAKeyWrittenBeforeBackendsExistedStillResolves(t *testing.T) { + t.Setenv(SigningPassphraseEnvironment, "wire-dispatch-test-passphrase-0001") + t.Setenv("XDG_DATA_HOME", t.TempDir()) + signers, err := NewSignerStore() + if err != nil { + t.Fatal(err) + } + selected, err := signers.backendFor(signer.Ref{ID: "unused"}) + if err != nil { + t.Fatal(err) + } + if selected != backend(signers.file) { + t.Error("a reference naming no backend did not resolve to the file store") + } +}