feat: spx admin images remove (#239)

cmd/spinifex/cmd/admin.go

@@ -17,6 +17,7 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
 package cmd
 
 import (
+	"bufio"
 	"bytes"
 	"context"
 	"crypto/tls"
@@ -48,6 +49,7 @@ import (
 	"github.com/mulgadc/spinifex/spinifex/gpu"
 	handlers_ec2_vpc "github.com/mulgadc/spinifex/spinifex/handlers/ec2/vpc"
 	handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam"
+	"github.com/mulgadc/spinifex/spinifex/objectstore"
 	"github.com/mulgadc/spinifex/spinifex/utils"
 	"github.com/mulgadc/viperblock/viperblock"
 	"github.com/mulgadc/viperblock/viperblock/backends/s3"
@@ -151,6 +153,19 @@ var imagesListCmd = &cobra.Command{
 	Run:   runimagesListCmd,
 }
 
+var imagesRemoveCmd = &cobra.Command{
+	Use:   "remove",
+	Short: "Remove an admin-imported system AMI",
+	Long: `Delete an AMI imported via 'spx admin images import', including its
+backing block storage and snapshot artefacts. Only operates on AMIs with a
+non-account owner (e.g. "system"); account-owned AMIs must be removed via
+'aws ec2 deregister-image' followed by 'aws ec2 delete-snapshot'.
+
+Refuses to delete an AMI that has dependent volumes or copied snapshots/AMIs
+unless --force is passed. Prompts for confirmation unless --yes is passed.`,
+	Run: runimagesRemoveCmd,
+}
+
 var accountCmd = &cobra.Command{
 	Use:   "account",
 	Short: "Manage Spinifex accounts",
@@ -233,6 +248,7 @@ func init() {
 	adminCmd.AddCommand(imagesCmd)
 	imagesCmd.AddCommand(imagesImportCmd)
 	imagesCmd.AddCommand(imagesListCmd)
+	imagesCmd.AddCommand(imagesRemoveCmd)
 
 	adminCmd.AddCommand(accountCmd)
 	accountCmd.AddCommand(accountCreateCmd)
@@ -316,6 +332,13 @@ func init() {
 	imagesImportCmd.Flags().StringSlice("tag", nil, "Tag to apply to the imported AMI as key=value (repeatable; e.g. --tag spinifex:managed-by=elbv2)")
 	imagesImportCmd.Flags().Bool("force", false, "Force command execution (overwrites existing files)")
 	imagesImportCmd.Flags().Bool("skip-verify", false, "Skip catalog-image checksum verification (INSECURE; operator assumes integrity responsibility)")
+
+	imagesRemoveCmd.Flags().String("image-id", "", "AMI ID to remove (required)")
+	imagesRemoveCmd.Flags().Bool("force", false, "Bypass dependency, ownership and config-corrupt checks (salvage mode)")
+	imagesRemoveCmd.Flags().Bool("yes", false, "Skip interactive confirmation prompt")
+	if err := imagesRemoveCmd.MarkFlagRequired("image-id"); err != nil {
+		panic(err)
+	}
 }
 
 func runimagesImportCmd(cmd *cobra.Command, args []string) {
@@ -602,6 +625,136 @@ func runimagesImportCmd(cmd *cobra.Command, args []string) {
 	fmt.Printf("✅ Image import complete. Image-ID (AMI): %s\n", volumeId)
 }
 
+func runimagesRemoveCmd(cmd *cobra.Command, args []string) {
+	imageID, _ := cmd.Flags().GetString("image-id")
+	force, _ := cmd.Flags().GetBool("force")
+	yes, _ := cmd.Flags().GetBool("yes")
+
+	cfgFile, _ := cmd.Flags().GetString("config")
+	if cfgFile == "" {
+		cfgFile = DefaultConfigFile()
+	}
+
+	appConfig, err := config.LoadConfig(cfgFile)
+	if err != nil {
+		fmt.Fprintln(os.Stderr, "Error loading config file:", err)
+		os.Exit(1)
+	}
+
+	node := appConfig.Nodes[appConfig.Node]
+	store := objectstore.NewS3ObjectStoreFromConfig(
+		node.Predastore.Host,
+		node.Predastore.Region,
+		node.Predastore.AccessKey,
+		node.Predastore.SecretKey,
+	)
+	bucket := node.Predastore.Bucket
+
+	preview, err := admin.PreviewRemoveSystemImage(store, bucket, imageID)
+	if err != nil {
+		fmt.Fprintln(os.Stderr, "Failed to inspect AMI:", err)
+		os.Exit(1)
+	}
+
+	// Print metadata block.
+	fmt.Println("About to remove system AMI:")
+	fmt.Println()
+	fmt.Printf("  Image ID:        %s\n", preview.ImageID)
+	switch {
+	case !preview.ConfigPresent && !preview.ConfigCorrupt:
+		fmt.Println("  Name:            <unknown — config.json missing>")
+		fmt.Println("  Owner:           <unknown>")
+	case preview.ConfigCorrupt:
+		fmt.Println("  Name:            <unknown — config.json corrupt>")
+		fmt.Println("  Owner:           <unknown>")
+	default:
+		fmt.Printf("  Name:            %s\n", preview.Name)
+		fmt.Printf("  Owner:           %s\n", preview.Owner)
+		if !preview.Created.IsZero() {
+			fmt.Printf("  Created:         %s\n", preview.Created.UTC().Format("2006-01-02T15:04:05Z"))
+		}
+	}
+	fmt.Printf("  Backing storage: %s/      (%d objects, %s)\n",
+		preview.ImageID, preview.AMIObjectCount, utils.HumanBytes(utils.SafeInt64ToUint64(preview.AMIBytesTotal)))
+	fmt.Printf("                   %s/ (%d objects, %s)\n",
+		admin.SnapPrefix(preview.ImageID), preview.SnapObjectCount, utils.HumanBytes(utils.SafeInt64ToUint64(preview.SnapBytesTotal)))
+	fmt.Println()
+
+	// Account-owned guard before salvage / dependents — the AWS-flow hint is
+	// the most useful thing to surface for this kind of mistake.
+	if preview.ConfigPresent && !preview.IsSystemOwned && !force {
+		fmt.Fprintf(os.Stderr,
+			"Refusing to remove: %s is account-owned (%s).\n"+
+				"Use 'aws ec2 deregister-image --image-id %s' followed by 'aws ec2 delete-snapshot ...'.\n",
+			preview.ImageID, preview.Owner, preview.ImageID)
+		os.Exit(1)
+	}
+
+	if !preview.ConfigPresent && !force {
+		fmt.Fprintln(os.Stderr, "AMI config.json missing or corrupt; re-run with --force to salvage backing blocks.")
+		os.Exit(1)
+	}
+
+	if !preview.Dependents.Empty() && !force {
+		fmt.Fprintln(os.Stderr, "Refusing to remove: dependent resources reference this image.")
+		printDependents(os.Stderr, preview.Dependents)
+		fmt.Fprintln(os.Stderr, "Remove them first (e.g. 'aws ec2 terminate-instances', 'aws ec2 delete-snapshot', 'aws ec2 deregister-image') or re-run with --force.")
+		os.Exit(1)
+	}
+
+	if force && (!preview.ConfigPresent || !preview.Dependents.Empty()) {
+		fmt.Println("⚠️  --force: skipping dependency check and ownership check.")
+		if !preview.Dependents.Empty() {
+			printDependents(os.Stdout, preview.Dependents)
+		}
+		fmt.Println()
+	}
+
+	fmt.Println("This is permanent and cannot be undone.")
+	if !yes {
+		fmt.Print("Type 'yes' to proceed: ")
+		reader := bufio.NewReader(os.Stdin)
+		answer, _ := reader.ReadString('\n')
+		if strings.TrimSpace(answer) != "yes" {
+			fmt.Println("Aborted.")
+			return
+		}
+	}
+
+	res, err := admin.RemoveSystemImage(store, bucket, admin.RemoveImageOpts{
+		ImageID: imageID,
+		Force:   force,
+	})
+	if err != nil {
+		fmt.Fprintln(os.Stderr, "Remove failed:", err)
+		os.Exit(1)
+	}
+
+	fmt.Printf("✅ Removed AMI %s (freed %s across %d objects).\n",
+		imageID, utils.HumanBytes(utils.SafeInt64ToUint64(res.BytesFreed)), res.ObjectsDeleted)
+}
+
+func printDependents(w io.Writer, d admin.Dependents) {
+	if len(d.Volumes) > 0 {
+		fmt.Fprintf(w, "  Volumes (%d):\n", len(d.Volumes))
+		for _, v := range d.Volumes {
+			fmt.Fprintf(w, "    - %s\n", v)
+		}
+	}
+	if len(d.Snapshots) > 0 {
+		fmt.Fprintf(w, "  Snapshots (%d):\n", len(d.Snapshots))
+		for _, s := range d.Snapshots {
+			fmt.Fprintf(w, "    - %s\n", s)
+		}
+	}
+	if len(d.AMIs) > 0 {
+		fmt.Fprintf(w, "  AMIs (%d):\n", len(d.AMIs))
+		for _, a := range d.AMIs {
+			fmt.Fprintf(w, "    - %s\n", a)
+		}
+	}
+}
+
 // List remote images available
 func runimagesListCmd(cmd *cobra.Command, args []string) {
 	//fmt.Println(availableImages)

cmd/spinifex/cmd/admin_test.go

@@ -310,3 +310,24 @@ func TestWritePredastoreEncryptionKey_CreatesMissingDir(t *testing.T) {
 	require.NoError(t, err)
 	assert.True(t, dirInfo.IsDir())
 }
+
+// TestImagesRemoveCmd_FlagSchema guards the public flag surface for
+// `spx admin images remove`. --image-id is required so cobra rejects a bare
+// invocation; --force/--yes default to false.
+func TestImagesRemoveCmd_FlagSchema(t *testing.T) {
+	imageIDFlag := imagesRemoveCmd.Flags().Lookup("image-id")
+	require.NotNil(t, imageIDFlag, "--image-id must be defined")
+	assert.Equal(t, []string{"true"}, imageIDFlag.Annotations[cobraRequiredAnnotation],
+		"--image-id must be marked required")
+
+	forceFlag := imagesRemoveCmd.Flags().Lookup("force")
+	require.NotNil(t, forceFlag)
+	assert.Equal(t, "false", forceFlag.DefValue)
+
+	yesFlag := imagesRemoveCmd.Flags().Lookup("yes")
+	require.NotNil(t, yesFlag)
+	assert.Equal(t, "false", yesFlag.DefValue)
+}
+
+// cobraRequiredAnnotation is the annotation key cobra uses to mark required flags.
+const cobraRequiredAnnotation = "cobra_annotation_bash_completion_one_required_flag"

docs/COMMANDS.md

@@ -91,6 +91,7 @@ Operational commands for inspecting cluster state. These fan out NATS requests t
 |---------|-------|---------------|-------------|------------|--------|
 | `spx admin images import` | `--name`, `--file`, `--distro`, `--version`, `--arch`, `--platform`, `--boot-mode` (bios/uefi/uefi-preferred), `--tag`, `--force`, `--skip-verify` | Cluster must be running; either `--name` (catalog download) or `--file` (operator-supplied media). `--file` mode additionally requires `--distro`, `--version`, `--arch`, `--boot-mode`. | Catalog imports (`--name`) download the image, fetch the catalog `Checksum` URL, verify the SHA-256/SHA-512 digest, and inherit `BootMode` from the catalog entry. `--boot-mode` overrides the catalog value when set. Mismatch fails closed; the cached file is left on disk and `--force` re-downloads. `--file` imports skip checksum verification (operator-supplied media is outside Spinifex's trust boundary, the skip is logged at INFO for audit) and require an explicit `--boot-mode` because there is no catalog metadata to inherit from. `--skip-verify` bypasses verification for catalog imports and emits a WARN slog + stderr notice; use only for debugging or when upstream mirrors are confirmed-broken. | 1. Import valid catalog image (verifies checksum)<br>2. Tampered cache hit fails with `ErrChecksumMismatch`<br>3. `--force` recovers after a mismatch<br>4. `--file` import skips verification<br>5. `--skip-verify` bypasses checksum with WARN<br>6. `--file` without `--boot-mode` rejected<br>7. `--boot-mode=invalid` rejected | **DONE** |
 | `spx admin images list` | — | None | Lists available OS images that can be imported or downloaded | 1. List available images | **DONE** |
+| `spx admin images remove` | `--image-id` (required), `--force`, `--yes` | Cluster must be running; predastore reachable. Targets AMIs imported via `spx admin images import` (non-account `ImageOwnerAlias`, e.g. `"system"`). | Loads `ami-<id>/config.json`, walks transitive dependents — copied snapshots whose `VolumeID == imageID`, volumes whose `SnapshotID` references the internal `snap-ami-<id>` or any derived snap, and account AMIs created via `CopyImage` whose `SnapshotID` is a derived snap — then prompts (skipped with `--yes`) before deleting `ami-<id>/config.json` (the DescribeImages barrier) followed by the rest of `ami-<id>/` and `snap-ami-<id>/`. Account-owned AMIs are refused with a hint pointing at `aws ec2 deregister-image` + `aws ec2 delete-snapshot`. `--force` bypasses the dependency, ownership and config-corrupt checks for salvage of orphaned blocks. | 1. Happy path: system AMI, no deps → deleted<br>2. Account-owned → refused with AWS-flow hint<br>3. Missing/corrupt config → `InvalidAMIID.NotFound` (salvageable with `--force`)<br>4. Dependent volume (direct or via copied snap) → refused<br>5. Dependent account AMI → refused<br>6. `--force` overrides dependents<br>7. Idempotent re-run after partial delete | **DONE** |
 
 #### Image integrity verification (CMMC SI.L1-3.14.2)
 
@@ -123,6 +124,35 @@ served. A mirror compromise that swaps both image and sums file is not
 detected; closing that gap requires GPG signature verification of the sums
 file, deferred to a later phase.
 
+#### `spx admin images remove` caveats
+
+Admin-imported AMIs (`ImageOwnerAlias = "system"`) live
+under the `ami-<id>/` S3 prefix and use a viperblock-internal snap checkpoint
+at `snap-ami-<id>/` — there is no `snap-<id>/metadata.json`. The AWS handlers
+(`DeregisterImage`, `DeleteSnapshot`) reject system owners with
+`UnauthorizedOperation`, which is the right behaviour for tenant API callers
+but leaves no AWS-flow path to reclaim space. `spx admin images remove` is
+the admin-trust-boundary counterpart that performs the dependency walk and
+hard-deletes the blocks directly against predastore.
+
+`CopyImage` of a system AMI is metadata-only: it writes a fresh
+`snap-<acct>/metadata.json` whose `VolumeID` points at `ami-<sys>` and a new
+`ami-<acct>/config.json` referencing that snap. Volumes launched from the
+copied AMI read transitively from `ami-<sys>/chunks/...`. The remove command
+walks this transitive set and refuses if anything references the target.
+
+**TOCTOU window:** between the safety scan and the `config.json` delete a
+concurrent `RunInstances` against the AMI could create a new dependent
+volume. The window is sub-second on a healthy cluster. The admin running
+this command is expected to know the fleet's operational state; if the race
+fires the result is a `vol-<id>` with deleted backing blocks, recovered by
+terminating the orphaned instance.
+
+**`--force` bypasses every safety check** (dependents, ownership, missing /
+corrupt `config.json`). Use only for salvage of orphaned blocks. Running it
+against a live system AMI corrupts every dependent volume on the next disk
+read.
+
 ### GPU Management
 
 | Command | Flags | Prerequisites | Basic Logic | Test Cases | Status |

scripts/dev-install.sh

@@ -79,10 +79,6 @@ SPINIFEX_AZ="${SPINIFEX_AZ:-${SPINIFEX_REGION}a}"
 echo "=== Initializing (region=$SPINIFEX_REGION az=$SPINIFEX_AZ) ==="
 sudo spx admin init --force --region "$SPINIFEX_REGION" --az "$SPINIFEX_AZ" --node node1 --nodes 1
 
-echo "=== Installing CA certificate ==="
-sudo cp /etc/spinifex/ca.pem /usr/local/share/ca-certificates/spinifex-ca.crt
-sudo update-ca-certificates
-
 echo "=== Starting services ==="
 sudo systemctl start spinifex.target
 

spinifex/admin/images_remove.go

@@ -0,0 +1,479 @@
+package admin
+
+import (
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io"
+	"log/slog"
+	"strings"
+	"time"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/s3"
+	"github.com/mulgadc/spinifex/spinifex/awserrors"
+	handlers_ec2_image "github.com/mulgadc/spinifex/spinifex/handlers/ec2/image"
+	handlers_ec2_snapshot "github.com/mulgadc/spinifex/spinifex/handlers/ec2/snapshot"
+	"github.com/mulgadc/spinifex/spinifex/objectstore"
+	"github.com/mulgadc/spinifex/spinifex/utils"
+	"github.com/mulgadc/viperblock/viperblock"
+)
+
+// RemoveImageOpts configures RemoveSystemImage.
+type RemoveImageOpts struct {
+	ImageID string
+	// Force bypasses the dependency check, the ownership-shape check, and
+	// the "config.json missing/corrupt" check. Salvage-mode lever.
+	Force bool
+}
+
+// RemoveImageResult summarises what was deleted from object storage.
+type RemoveImageResult struct {
+	ObjectsDeleted int
+	BytesFreed     int64
+}
+
+// Dependents lists every resource that transitively backs an admin-imported
+// AMI. Removing the AMI while any of these exist would corrupt the dependent.
+type Dependents struct {
+	Snapshots []string // EC2 snapshots whose VolumeID == the AMI ID (i.e. CopyImage-derived snaps)
+	Volumes   []string // Volumes whose SnapshotID is snap-ami-<id> or a derived snap
+	AMIs      []string // AMIs whose SnapshotID is a derived snap (CopyImage of a system AMI then re-copy)
+}
+
+// Empty is true when nothing depends on the AMI.
+func (d Dependents) Empty() bool {
+	return len(d.Snapshots) == 0 && len(d.Volumes) == 0 && len(d.AMIs) == 0
+}
+
+// RemovePreview captures everything the CLI needs to render the confirmation
+// prompt and decide whether to proceed: AMI metadata (or zero values when the
+// config is missing/corrupt), backing-storage byte counts, and the dependent
+// resource set. PreviewRemoveSystemImage performs no deletions.
+type RemovePreview struct {
+	ImageID       string
+	Name          string
+	Owner         string
+	Created       time.Time
+	ConfigPresent bool // false when ami-<id>/config.json is missing
+	ConfigCorrupt bool // true when config.json exists but is undecodable
+	IsSystemOwned bool // ImageOwnerAlias is set and not an account ID
+
+	AMIObjectCount  int
+	AMIBytesTotal   int64
+	SnapObjectCount int
+	SnapBytesTotal  int64
+
+	Dependents Dependents
+}
+
+// SnapPrefix returns the viperblock-internal snapshot prefix backing an
+// admin-imported AMI. v_utils.ImportDiskImage writes the snapshot as
+// "snap-<volumeName>" where the volume name IS the AMI ID.
+func SnapPrefix(imageID string) string {
+	return "snap-" + imageID
+}
+
+// PreviewRemoveSystemImage walks the object store and gathers everything the
+// caller needs to decide whether to remove an admin-imported AMI. It performs
+// no mutations. Returned errors indicate unexpected I/O failures only —
+// AMI-not-found, corrupt config, and account-owned AMIs are reflected in the
+// preview fields, not as errors.
+func PreviewRemoveSystemImage(store objectstore.ObjectStore, bucket, imageID string) (*RemovePreview, error) {
+	if !strings.HasPrefix(imageID, "ami-") {
+		return nil, errors.New(awserrors.ErrorInvalidAMIIDMalformed)
+	}
+
+	preview := &RemovePreview{ImageID: imageID}
+
+	meta, configErr := readAMIConfig(store, bucket, imageID)
+	switch {
+	case configErr == nil:
+		preview.ConfigPresent = true
+		preview.Name = meta.Name
+		preview.Owner = meta.ImageOwnerAlias
+		preview.Created = meta.CreationDate
+		preview.IsSystemOwned = meta.ImageOwnerAlias != "" && !utils.IsAccountID(meta.ImageOwnerAlias)
+	case objectstore.IsNoSuchKeyError(configErr):
+		// Config absent — salvage candidate. Leave ConfigPresent=false.
+	case errors.Is(configErr, handlers_ec2_image.ErrCorruptAMIConfig):
+		preview.ConfigCorrupt = true
+	default:
+		return nil, fmt.Errorf("preview: read AMI config: %w", configErr)
+	}
+
+	amiCount, amiBytes, err := sumPrefix(store, bucket, imageID+"/")
+	if err != nil {
+		return nil, fmt.Errorf("preview: sum ami prefix: %w", err)
+	}
+	preview.AMIObjectCount = amiCount
+	preview.AMIBytesTotal = amiBytes
+
+	snapCount, snapBytes, err := sumPrefix(store, bucket, SnapPrefix(imageID)+"/")
+	if err != nil {
+		return nil, fmt.Errorf("preview: sum snap prefix: %w", err)
+	}
+	preview.SnapObjectCount = snapCount
+	preview.SnapBytesTotal = snapBytes
+
+	deps, err := FindAMIDependents(store, bucket, imageID)
+	if err != nil {
+		return nil, fmt.Errorf("preview: find dependents: %w", err)
+	}
+	preview.Dependents = deps
+
+	return preview, nil
+}
+
+// FindAMIDependents walks the bucket to find every resource that transitively
+// depends on the given admin-imported AMI:
+//
+//  1. snap-*/metadata.json whose VolumeID == imageID  → derivedSnaps
+//     (these are CopyImage-created snapshots that point back at the system AMI)
+//  2. vol-*/config.json whose VolumeMetadata.SnapshotID ∈ {snap-ami-<id>} ∪ derivedSnaps
+//  3. ami-*/config.json whose AMIMetadata.SnapshotID ∈ derivedSnaps
+//
+// The walk terminates one hop deep. CopyImage of an account AMI uses its own
+// VolumeID, not the system AMI's, so further recursion is not required today.
+func FindAMIDependents(store objectstore.ObjectStore, bucket, imageID string) (Dependents, error) {
+	var deps Dependents
+
+	prefixes, err := listCommonPrefixes(store, bucket)
+	if err != nil {
+		return Dependents{}, fmt.Errorf("list bucket prefixes: %w", err)
+	}
+
+	// Pass 1: collect derived snaps (CopyImage of this AMI writes a snap whose
+	// VolumeID points back at the source AMI ID).
+	derived := map[string]bool{}
+	for _, p := range prefixes {
+		if !strings.HasPrefix(p, "snap-") {
+			continue
+		}
+		snapID := strings.TrimSuffix(p, "/")
+		// Skip the viperblock-internal snap prefix for this AMI — it has no
+		// metadata.json (it was written by viperblock.CreateSnapshot, not by
+		// the EC2 snapshot service).
+		if snapID == SnapPrefix(imageID) {
+			continue
+		}
+		cfg, err := handlers_ec2_snapshot.ReadSnapshotConfig(store, bucket, snapID)
+		if err != nil {
+			if objectstore.IsNoSuchKeyError(err) || errors.Is(err, handlers_ec2_snapshot.ErrCorruptSnapshotMetadata) {
+				continue
+			}
+			return Dependents{}, fmt.Errorf("read snapshot %s: %w", snapID, err)
+		}
+		if cfg.VolumeID == imageID {
+			derived[snapID] = true
+			deps.Snapshots = append(deps.Snapshots, snapID)
+		}
+	}
+
+	// The set of snap IDs that a dependent volume might reference: the
+	// admin-import's internal snap plus every CopyImage-derived snap.
+	volSnapRefs := map[string]bool{SnapPrefix(imageID): true}
+	for s := range derived {
+		volSnapRefs[s] = true
+	}
+
+	// Pass 2: volumes (vol-*/config.json) whose SnapshotID matches.
+	for _, p := range prefixes {
+		if !strings.HasPrefix(p, "vol-") {
+			continue
+		}
+		volID := strings.TrimSuffix(p, "/")
+		cfg, err := readVolumeConfig(store, bucket, volID)
+		if err != nil {
+			if objectstore.IsNoSuchKeyError(err) || errors.Is(err, errCorruptVolumeConfig) {
+				continue
+			}
+			return Dependents{}, fmt.Errorf("read volume %s: %w", volID, err)
+		}
+		if volSnapRefs[cfg.VolumeMetadata.SnapshotID] {
+			deps.Volumes = append(deps.Volumes, volID)
+		}
+	}
+
+	// Pass 3: AMIs (ami-*/config.json) whose SnapshotID is a derived snap.
+	// Skip the target AMI itself.
+	for _, p := range prefixes {
+		if !strings.HasPrefix(p, "ami-") {
+			continue
+		}
+		otherAMI := strings.TrimSuffix(p, "/")
+		if otherAMI == imageID {
+			continue
+		}
+		meta, err := readAMIConfig(store, bucket, otherAMI)
+		if err != nil {
+			if objectstore.IsNoSuchKeyError(err) || errors.Is(err, handlers_ec2_image.ErrCorruptAMIConfig) {
+				continue
+			}
+			return Dependents{}, fmt.Errorf("read AMI %s: %w", otherAMI, err)
+		}
+		if derived[meta.SnapshotID] {
+			deps.AMIs = append(deps.AMIs, otherAMI)
+		}
+	}
+
+	return deps, nil
+}
+
+// RemoveSystemImage deletes an admin-imported AMI and its backing storage
+// after re-validating the preconditions:
+//
+//  1. ami-<id>/config.json exists and parses (skipped under --force = salvage)
+//  2. AMI is system-owned, not account-owned (--force bypasses)
+//  3. No volumes / snapshots / AMIs depend on it (--force bypasses)
+//
+// Deletion order is intentional: the config.json is deleted FIRST so the AMI
+// disappears from DescribeImages immediately, preventing new RunInstances
+// from racing the block cleanup.
+func RemoveSystemImage(store objectstore.ObjectStore, bucket string, opts RemoveImageOpts) (*RemoveImageResult, error) {
+	if !strings.HasPrefix(opts.ImageID, "ami-") {
+		return nil, errors.New(awserrors.ErrorInvalidAMIIDMalformed)
+	}
+
+	meta, configErr := readAMIConfig(store, bucket, opts.ImageID)
+	configMissing := objectstore.IsNoSuchKeyError(configErr)
+	configCorrupt := errors.Is(configErr, handlers_ec2_image.ErrCorruptAMIConfig)
+	switch {
+	case configErr == nil:
+		// fine
+	case configMissing, configCorrupt:
+		if !opts.Force {
+			return nil, errors.New(awserrors.ErrorInvalidAMIIDNotFound)
+		}
+	default:
+		slog.Error("RemoveSystemImage: read AMI config", "imageId", opts.ImageID, "err", configErr)
+		return nil, errors.New(awserrors.ErrorServerInternal)
+	}
+
+	if configErr == nil && !opts.Force {
+		if meta.ImageOwnerAlias != "" && utils.IsAccountID(meta.ImageOwnerAlias) {
+			return nil, fmt.Errorf("%s is account-owned (%s); use `aws ec2 deregister-image` followed by `aws ec2 delete-snapshot`",
+				opts.ImageID, meta.ImageOwnerAlias)
+		}
+	}
+
+	if !opts.Force {
+		deps, err := FindAMIDependents(store, bucket, opts.ImageID)
+		if err != nil {
+			slog.Error("RemoveSystemImage: dependency walk", "imageId", opts.ImageID, "err", err)
+			return nil, errors.New(awserrors.ErrorServerInternal)
+		}
+		if !deps.Empty() {
+			return nil, &DependentError{ImageID: opts.ImageID, Dependents: deps}
+		}
+	}
+
+	result := &RemoveImageResult{}
+
+	// Step 1: drop config.json first — the barrier that hides the AMI from
+	// DescribeImages so no new launches can land on the blocks we're deleting.
+	if configErr == nil || (opts.Force && !configMissing) {
+		n, b, err := deletePrefix(store, bucket, opts.ImageID+"/config.json")
+		if err != nil {
+			return nil, fmt.Errorf("delete config: %w", err)
+		}
+		result.ObjectsDeleted += n
+		result.BytesFreed += b
+	}
+
+	// Step 2: drop the rest of ami-<id>/ (chunks, WAL, checkpoints).
+	n, b, err := deletePrefix(store, bucket, opts.ImageID+"/")
+	if err != nil {
+		return nil, fmt.Errorf("delete ami prefix: %w", err)
+	}
+	result.ObjectsDeleted += n
+	result.BytesFreed += b
+
+	// Step 3: drop snap-<amiID>/ (the viperblock-internal snap checkpoint).
+	n, b, err = deletePrefix(store, bucket, SnapPrefix(opts.ImageID)+"/")
+	if err != nil {
+		return nil, fmt.Errorf("delete snap prefix: %w", err)
+	}
+	result.ObjectsDeleted += n
+	result.BytesFreed += b
+
+	slog.Info("RemoveSystemImage completed",
+		"imageId", opts.ImageID,
+		"objectsDeleted", result.ObjectsDeleted,
+		"bytesFreed", result.BytesFreed,
+		"force", opts.Force,
+	)
+	return result, nil
+}
+
+// DependentError is returned by RemoveSystemImage when dependent resources
+// block deletion. The CLI prints the dependents list and exits non-zero.
+type DependentError struct {
+	ImageID    string
+	Dependents Dependents
+}
+
+func (e *DependentError) Error() string {
+	return fmt.Sprintf("refusing to remove %s: %d volumes, %d snapshots, %d AMIs depend on this image",
+		e.ImageID, len(e.Dependents.Volumes), len(e.Dependents.Snapshots), len(e.Dependents.AMIs))
+}
+
+// readAMIConfig reads ami-<id>/config.json and returns the AMIMetadata.
+// Mirrors ImageServiceImpl.GetAMIConfig but operates package-locally so the
+// admin tooling doesn't require an ImageServiceImpl (which carries NATS).
+func readAMIConfig(store objectstore.ObjectStore, bucket, imageID string) (viperblock.AMIMetadata, error) {
+	key := imageID + "/config.json"
+	res, err := store.GetObject(&s3.GetObjectInput{
+		Bucket: aws.String(bucket),
+		Key:    aws.String(key),
+	})
+	if err != nil {
+		return viperblock.AMIMetadata{}, err
+	}
+	defer res.Body.Close()
+
+	body, err := io.ReadAll(res.Body)
+	if err != nil {
+		return viperblock.AMIMetadata{}, err
+	}
+
+	var state viperblock.VBState
+	if err := json.Unmarshal(body, &state); err != nil {
+		return viperblock.AMIMetadata{}, fmt.Errorf("%w: %s: %v", handlers_ec2_image.ErrCorruptAMIConfig, key, err)
+	}
+	return state.VolumeConfig.AMIMetadata, nil
+}
+
+// errCorruptVolumeConfig lets the dependency walk distinguish a vol-*/config.json
+// that can't be parsed (recovery is operator-owned, walk continues) from a
+// transient transport error (walk must fail closed — silently dropping a
+// dependent could lead to deleting blocks underneath a live volume).
+var errCorruptVolumeConfig = errors.New("corrupt volume config")
+
+// readVolumeConfig reads vol-<id>/config.json into VolumeConfig.
+type volumeConfigWrapper struct {
+	VolumeConfig viperblock.VolumeConfig `json:"VolumeConfig"`
+}
+
+func readVolumeConfig(store objectstore.ObjectStore, bucket, volumeID string) (*viperblock.VolumeConfig, error) {
+	key := volumeID + "/config.json"
+	res, err := store.GetObject(&s3.GetObjectInput{
+		Bucket: aws.String(bucket),
+		Key:    aws.String(key),
+	})
+	if err != nil {
+		return nil, err
+	}
+	defer res.Body.Close()
+
+	body, err := io.ReadAll(res.Body)
+	if err != nil {
+		return nil, err
+	}
+
+	var w volumeConfigWrapper
+	if err := json.Unmarshal(body, &w); err != nil {
+		return nil, fmt.Errorf("%w: %s: %v", errCorruptVolumeConfig, key, err)
+	}
+	return &w.VolumeConfig, nil
+}
+
+// listCommonPrefixes returns the top-level "directory" prefixes in the bucket
+// (e.g. "ami-xxx/", "vol-yyy/", "snap-zzz/"), exhausting any pagination.
+func listCommonPrefixes(store objectstore.ObjectStore, bucket string) ([]string, error) {
+	seen := map[string]bool{}
+	var token *string
+	for {
+		out, err := store.ListObjectsV2(&s3.ListObjectsV2Input{
+			Bucket:            aws.String(bucket),
+			Delimiter:         aws.String("/"),
+			ContinuationToken: token,
+		})
+		if err != nil {
+			return nil, err
+		}
+		for _, cp := range out.CommonPrefixes {
+			if cp.Prefix == nil {
+				continue
+			}
+			seen[*cp.Prefix] = true
+		}
+		if !aws.BoolValue(out.IsTruncated) {
+			break
+		}
+		token = out.NextContinuationToken
+	}
+	prefixes := make([]string, 0, len(seen))
+	for p := range seen {
+		prefixes = append(prefixes, p)
+	}
+	return prefixes, nil
+}
+
+// sumPrefix returns (object count, total bytes) for every object under prefix.
+// Used by the preview to surface what an operator is about to delete.
+func sumPrefix(store objectstore.ObjectStore, bucket, prefix string) (int, int64, error) {
+	var count int
+	var bytes int64
+	var token *string
+	for {
+		out, err := store.ListObjectsV2(&s3.ListObjectsV2Input{
+			Bucket:            aws.String(bucket),
+			Prefix:            aws.String(prefix),
+			ContinuationToken: token,
+		})
+		if err != nil {
+			return 0, 0, err
+		}
+		for _, obj := range out.Contents {
+			count++
+			if obj.Size != nil {
+				bytes += *obj.Size
+			}
+		}
+		if !aws.BoolValue(out.IsTruncated) {
+			break
+		}
+		token = out.NextContinuationToken
+	}
+	return count, bytes, nil
+}
+
+// deletePrefix removes every object under prefix and returns count+bytes
+// deleted. Used by RemoveSystemImage for both the single-key config.json
+// barrier and the bulk ami-<id>/ and snap-<id>/ teardown.
+func deletePrefix(store objectstore.ObjectStore, bucket, prefix string) (int, int64, error) {
+	var count int
+	var bytes int64
+	var token *string
+	for {
+		out, err := store.ListObjectsV2(&s3.ListObjectsV2Input{
+			Bucket:            aws.String(bucket),
+			Prefix:            aws.String(prefix),
+			ContinuationToken: token,
+		})
+		if err != nil {
+			return count, bytes, err
+		}
+		for _, obj := range out.Contents {
+			if obj.Key == nil {
+				continue
+			}
+			if _, err := store.DeleteObject(&s3.DeleteObjectInput{
+				Bucket: aws.String(bucket),
+				Key:    obj.Key,
+			}); err != nil {
+				return count, bytes, fmt.Errorf("delete %s: %w", *obj.Key, err)
+			}
+			count++
+			if obj.Size != nil {
+				bytes += *obj.Size
+			}
+		}
+		if !aws.BoolValue(out.IsTruncated) {
+			break
+		}
+		token = out.NextContinuationToken
+	}
+	return count, bytes, nil
+}

spinifex/admin/images_remove_test.go

@@ -0,0 +1,369 @@
+package admin
+
+import (
+	"bytes"
+	"encoding/json"
+	"errors"
+	"testing"
+
+	"github.com/aws/aws-sdk-go/aws"
+	awss3 "github.com/aws/aws-sdk-go/service/s3"
+	"github.com/mulgadc/spinifex/spinifex/awserrors"
+	handlers_ec2_snapshot "github.com/mulgadc/spinifex/spinifex/handlers/ec2/snapshot"
+	"github.com/mulgadc/spinifex/spinifex/objectstore"
+	"github.com/mulgadc/viperblock/viperblock"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	testRemoveBucket    = "test-bucket"
+	testRemoveAccountID = "000000000001"
+)
+
+// putAMI writes an ami-<id>/config.json with the given owner. Owner "" produces
+// a corrupt config (empty ImageOwnerAlias is treated as corrupt by readers).
+func putAMI(t *testing.T, store *objectstore.MemoryObjectStore, imageID, name, owner, snapshotID string) {
+	t.Helper()
+	state := viperblock.VBState{
+		VolumeConfig: viperblock.VolumeConfig{
+			AMIMetadata: viperblock.AMIMetadata{
+				ImageID:         imageID,
+				Name:            name,
+				ImageOwnerAlias: owner,
+				SnapshotID:      snapshotID,
+				VolumeSizeGiB:   8,
+			},
+		},
+	}
+	data, err := json.Marshal(state)
+	require.NoError(t, err)
+	_, err = store.PutObject(&awss3.PutObjectInput{
+		Bucket: aws.String(testRemoveBucket),
+		Key:    aws.String(imageID + "/config.json"),
+		Body:   bytes.NewReader(data),
+	})
+	require.NoError(t, err)
+}
+
+// putAMIBlocks writes a few dummy chunk objects under ami-<id>/ so deletion
+// has bulk work and byte counts to verify.
+func putAMIBlocks(t *testing.T, store *objectstore.MemoryObjectStore, imageID string, n int, size int) {
+	t.Helper()
+	body := bytes.Repeat([]byte{0xab}, size)
+	for i := range n {
+		_, err := store.PutObject(&awss3.PutObjectInput{
+			Bucket: aws.String(testRemoveBucket),
+			Key:    aws.String(imageID + "/chunks/" + string(rune('a'+i)) + ".dat"),
+			Body:   bytes.NewReader(body),
+		})
+		require.NoError(t, err)
+	}
+}
+
+func putSnapBlocks(t *testing.T, store *objectstore.MemoryObjectStore, snapID string, n int, size int) {
+	t.Helper()
+	body := bytes.Repeat([]byte{0xcd}, size)
+	for i := range n {
+		_, err := store.PutObject(&awss3.PutObjectInput{
+			Bucket: aws.String(testRemoveBucket),
+			Key:    aws.String(snapID + "/cp/" + string(rune('a'+i)) + ".bin"),
+			Body:   bytes.NewReader(body),
+		})
+		require.NoError(t, err)
+	}
+}
+
+// putSnapMetadata writes a CopyImage-derived EC2 snapshot metadata that points
+// VolumeID back at an admin-imported AMI's ID.
+func putSnapMetadata(t *testing.T, store *objectstore.MemoryObjectStore, snapID, volumeID string) {
+	t.Helper()
+	cfg := &handlers_ec2_snapshot.SnapshotConfig{
+		SnapshotID: snapID,
+		VolumeID:   volumeID,
+		VolumeSize: 8,
+		State:      "completed",
+	}
+	require.NoError(t, handlers_ec2_snapshot.WriteSnapshotConfig(store, testRemoveBucket, snapID, cfg))
+}
+
+// putVolume writes vol-<id>/config.json with the given SnapshotID reference.
+func putVolume(t *testing.T, store *objectstore.MemoryObjectStore, volID, snapshotID string) {
+	t.Helper()
+	wrapper := volumeConfigWrapper{
+		VolumeConfig: viperblock.VolumeConfig{
+			VolumeMetadata: viperblock.VolumeMetadata{
+				VolumeID:   volID,
+				SnapshotID: snapshotID,
+				SizeGiB:    8,
+			},
+		},
+	}
+	data, err := json.Marshal(wrapper)
+	require.NoError(t, err)
+	_, err = store.PutObject(&awss3.PutObjectInput{
+		Bucket: aws.String(testRemoveBucket),
+		Key:    aws.String(volID + "/config.json"),
+		Body:   bytes.NewReader(data),
+	})
+	require.NoError(t, err)
+}
+
+func TestRemoveSystemImage_HappyPath(t *testing.T) {
+	store := objectstore.NewMemoryObjectStore()
+	const id = "ami-sys-deb13"
+	putAMI(t, store, id, "debian-13", "system", SnapPrefix(id))
+	putAMIBlocks(t, store, id, 3, 128)
+	putSnapBlocks(t, store, SnapPrefix(id), 2, 16)
+
+	preview, err := PreviewRemoveSystemImage(store, testRemoveBucket, id)
+	require.NoError(t, err)
+	assert.True(t, preview.ConfigPresent)
+	assert.True(t, preview.IsSystemOwned)
+	assert.Equal(t, "system", preview.Owner)
+	// config.json + 3 chunks = 4 ami-prefix objects.
+	assert.Equal(t, 4, preview.AMIObjectCount)
+	assert.Equal(t, 2, preview.SnapObjectCount)
+	assert.True(t, preview.Dependents.Empty())
+
+	res, err := RemoveSystemImage(store, testRemoveBucket, RemoveImageOpts{ImageID: id})
+	require.NoError(t, err)
+	assert.Equal(t, preview.AMIObjectCount+preview.SnapObjectCount, res.ObjectsDeleted)
+	assert.Equal(t, preview.AMIBytesTotal+preview.SnapBytesTotal, res.BytesFreed)
+	assert.Equal(t, 0, store.Count())
+}
+
+func TestRemoveSystemImage_AccountOwned_Refused(t *testing.T) {
+	store := objectstore.NewMemoryObjectStore()
+	const id = "ami-acct-001"
+	putAMI(t, store, id, "user-ami", testRemoveAccountID, "snap-acct-001")
+	putAMIBlocks(t, store, id, 1, 16)
+
+	_, err := RemoveSystemImage(store, testRemoveBucket, RemoveImageOpts{ImageID: id})
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), id)
+	assert.Contains(t, err.Error(), testRemoveAccountID)
+	assert.Contains(t, err.Error(), "account-owned")
+	assert.Contains(t, err.Error(), "aws ec2 deregister-image")
+	// Objects untouched.
+	assert.Equal(t, 2, store.Count())
+}
+
+func TestRemoveSystemImage_AccountOwned_ForceBypasses(t *testing.T) {
+	store := objectstore.NewMemoryObjectStore()
+	const id = "ami-acct-002"
+	putAMI(t, store, id, "user-ami", testRemoveAccountID, "snap-acct-002")
+	putAMIBlocks(t, store, id, 2, 16)
+
+	res, err := RemoveSystemImage(store, testRemoveBucket, RemoveImageOpts{ImageID: id, Force: true})
+	require.NoError(t, err)
+	assert.Greater(t, res.ObjectsDeleted, 0)
+	assert.Equal(t, 0, store.Count())
+}
+
+func TestRemoveSystemImage_MissingConfig_NotFound(t *testing.T) {
+	store := objectstore.NewMemoryObjectStore()
+	const id = "ami-missing"
+	putAMIBlocks(t, store, id, 1, 16)
+
+	_, err := RemoveSystemImage(store, testRemoveBucket, RemoveImageOpts{ImageID: id})
+	require.Error(t, err)
+	assert.Equal(t, awserrors.ErrorInvalidAMIIDNotFound, err.Error())
+}
+
+func TestRemoveSystemImage_CorruptConfig_NotFound(t *testing.T) {
+	store := objectstore.NewMemoryObjectStore()
+	const id = "ami-corrupt"
+	_, err := store.PutObject(&awss3.PutObjectInput{
+		Bucket: aws.String(testRemoveBucket),
+		Key:    aws.String(id + "/config.json"),
+		Body:   bytes.NewReader([]byte("{not valid json")),
+	})
+	require.NoError(t, err)
+
+	_, err = RemoveSystemImage(store, testRemoveBucket, RemoveImageOpts{ImageID: id})
+	require.Error(t, err)
+	assert.Equal(t, awserrors.ErrorInvalidAMIIDNotFound, err.Error())
+}
+
+func TestRemoveSystemImage_DependentVolume_Direct_Refused(t *testing.T) {
+	store := objectstore.NewMemoryObjectStore()
+	const id = "ami-sys-deb13"
+	putAMI(t, store, id, "debian-13", "system", SnapPrefix(id))
+	// Direct dependent: a volume launched from the admin import.
+	putVolume(t, store, "vol-aaa", SnapPrefix(id))
+
+	_, err := RemoveSystemImage(store, testRemoveBucket, RemoveImageOpts{ImageID: id})
+	require.Error(t, err)
+	var depErr *DependentError
+	require.True(t, errors.As(err, &depErr))
+	assert.Contains(t, depErr.Dependents.Volumes, "vol-aaa")
+}
+
+func TestRemoveSystemImage_DependentVolume_Transitive_Refused(t *testing.T) {
+	store := objectstore.NewMemoryObjectStore()
+	const id = "ami-sys-deb13"
+	putAMI(t, store, id, "debian-13", "system", SnapPrefix(id))
+
+	// CopyImage of the system AMI wrote a snap whose VolumeID points back at the AMI ID.
+	const derivedSnap = "snap-derived-001"
+	putSnapMetadata(t, store, derivedSnap, id)
+	// A volume launched from the copied AMI references the derived snap.
+	putVolume(t, store, "vol-bbb", derivedSnap)
+
+	_, err := RemoveSystemImage(store, testRemoveBucket, RemoveImageOpts{ImageID: id})
+	require.Error(t, err)
+	var depErr *DependentError
+	require.True(t, errors.As(err, &depErr))
+	assert.Contains(t, depErr.Dependents.Volumes, "vol-bbb")
+	assert.Contains(t, depErr.Dependents.Snapshots, derivedSnap)
+}
+
+func TestRemoveSystemImage_DependentAMI_Refused(t *testing.T) {
+	store := objectstore.NewMemoryObjectStore()
+	const id = "ami-sys-deb13"
+	putAMI(t, store, id, "debian-13", "system", SnapPrefix(id))
+
+	const derivedSnap = "snap-derived-002"
+	putSnapMetadata(t, store, derivedSnap, id)
+	// Account AMI created via CopyImage; its SnapshotID is the derived snap.
+	putAMI(t, store, "ami-acct-002", "copied", testRemoveAccountID, derivedSnap)
+
+	_, err := RemoveSystemImage(store, testRemoveBucket, RemoveImageOpts{ImageID: id})
+	require.Error(t, err)
+	var depErr *DependentError
+	require.True(t, errors.As(err, &depErr))
+	assert.Contains(t, depErr.Dependents.AMIs, "ami-acct-002")
+}
+
+func TestRemoveSystemImage_Force_OverridesDependents(t *testing.T) {
+	store := objectstore.NewMemoryObjectStore()
+	const id = "ami-sys-deb13"
+	putAMI(t, store, id, "debian-13", "system", SnapPrefix(id))
+	putAMIBlocks(t, store, id, 2, 64)
+	putVolume(t, store, "vol-orphan", SnapPrefix(id))
+
+	res, err := RemoveSystemImage(store, testRemoveBucket, RemoveImageOpts{ImageID: id, Force: true})
+	require.NoError(t, err)
+	assert.Greater(t, res.ObjectsDeleted, 0)
+	// vol-orphan/config.json remains; the AMI is gone.
+	_, err = store.GetObject(&awss3.GetObjectInput{
+		Bucket: aws.String(testRemoveBucket),
+		Key:    aws.String(id + "/config.json"),
+	})
+	require.Error(t, err)
+	_, err = store.GetObject(&awss3.GetObjectInput{
+		Bucket: aws.String(testRemoveBucket),
+		Key:    aws.String("vol-orphan/config.json"),
+	})
+	require.NoError(t, err)
+}
+
+func TestRemoveSystemImage_Salvage_MissingConfig_ForceCleans(t *testing.T) {
+	store := objectstore.NewMemoryObjectStore()
+	const id = "ami-salvage-1"
+	// No config.json — just orphaned blocks.
+	putAMIBlocks(t, store, id, 3, 32)
+	putSnapBlocks(t, store, SnapPrefix(id), 1, 8)
+
+	// Without --force: NotFound.
+	_, err := RemoveSystemImage(store, testRemoveBucket, RemoveImageOpts{ImageID: id})
+	require.Error(t, err)
+	assert.Equal(t, awserrors.ErrorInvalidAMIIDNotFound, err.Error())
+
+	// With --force: salvage proceeds.
+	res, err := RemoveSystemImage(store, testRemoveBucket, RemoveImageOpts{ImageID: id, Force: true})
+	require.NoError(t, err)
+	assert.Equal(t, 4, res.ObjectsDeleted)
+	assert.Equal(t, 0, store.Count())
+}
+
+func TestRemoveSystemImage_Salvage_CorruptConfig_ForceCleans(t *testing.T) {
+	store := objectstore.NewMemoryObjectStore()
+	const id = "ami-salvage-2"
+	_, err := store.PutObject(&awss3.PutObjectInput{
+		Bucket: aws.String(testRemoveBucket),
+		Key:    aws.String(id + "/config.json"),
+		Body:   bytes.NewReader([]byte("garbage")),
+	})
+	require.NoError(t, err)
+	putAMIBlocks(t, store, id, 1, 8)
+
+	// Without --force.
+	_, err = RemoveSystemImage(store, testRemoveBucket, RemoveImageOpts{ImageID: id})
+	require.Error(t, err)
+	assert.Equal(t, awserrors.ErrorInvalidAMIIDNotFound, err.Error())
+
+	// With --force: corrupt config + blocks all go.
+	res, err := RemoveSystemImage(store, testRemoveBucket, RemoveImageOpts{ImageID: id, Force: true})
+	require.NoError(t, err)
+	assert.Equal(t, 2, res.ObjectsDeleted)
+	assert.Equal(t, 0, store.Count())
+}
+
+func TestRemoveSystemImage_IdempotentRerun(t *testing.T) {
+	store := objectstore.NewMemoryObjectStore()
+	const id = "ami-rerun"
+	putAMI(t, store, id, "debian-13", "system", SnapPrefix(id))
+	putAMIBlocks(t, store, id, 2, 16)
+
+	_, err := RemoveSystemImage(store, testRemoveBucket, RemoveImageOpts{ImageID: id})
+	require.NoError(t, err)
+	require.Equal(t, 0, store.Count())
+
+	// Second call without --force should report NotFound.
+	_, err = RemoveSystemImage(store, testRemoveBucket, RemoveImageOpts{ImageID: id})
+	require.Error(t, err)
+	assert.Equal(t, awserrors.ErrorInvalidAMIIDNotFound, err.Error())
+
+	// Salvage re-run is a no-op.
+	res, err := RemoveSystemImage(store, testRemoveBucket, RemoveImageOpts{ImageID: id, Force: true})
+	require.NoError(t, err)
+	assert.Equal(t, 0, res.ObjectsDeleted)
+}
+
+func TestRemoveSystemImage_MalformedID(t *testing.T) {
+	store := objectstore.NewMemoryObjectStore()
+	_, err := RemoveSystemImage(store, testRemoveBucket, RemoveImageOpts{ImageID: "vol-wrong"})
+	require.Error(t, err)
+	assert.Equal(t, awserrors.ErrorInvalidAMIIDMalformed, err.Error())
+}
+
+func TestPreviewRemoveSystemImage_Salvage(t *testing.T) {
+	store := objectstore.NewMemoryObjectStore()
+	const id = "ami-salvage-preview"
+	putAMIBlocks(t, store, id, 2, 4)
+
+	preview, err := PreviewRemoveSystemImage(store, testRemoveBucket, id)
+	require.NoError(t, err)
+	assert.False(t, preview.ConfigPresent)
+	assert.False(t, preview.ConfigCorrupt)
+	assert.Equal(t, 2, preview.AMIObjectCount)
+	assert.Equal(t, int64(8), preview.AMIBytesTotal)
+}
+
+func TestPreviewRemoveSystemImage_CorruptConfig(t *testing.T) {
+	store := objectstore.NewMemoryObjectStore()
+	const id = "ami-corrupt-preview"
+	_, err := store.PutObject(&awss3.PutObjectInput{
+		Bucket: aws.String(testRemoveBucket),
+		Key:    aws.String(id + "/config.json"),
+		Body:   bytes.NewReader([]byte("{nope")),
+	})
+	require.NoError(t, err)
+
+	preview, err := PreviewRemoveSystemImage(store, testRemoveBucket, id)
+	require.NoError(t, err)
+	assert.False(t, preview.ConfigPresent)
+	assert.True(t, preview.ConfigCorrupt)
+}
+
+func TestFindAMIDependents_SkipsTargetAMI(t *testing.T) {
+	store := objectstore.NewMemoryObjectStore()
+	const id = "ami-sys-only"
+	putAMI(t, store, id, "debian-13", "system", SnapPrefix(id))
+
+	deps, err := FindAMIDependents(store, testRemoveBucket, id)
+	require.NoError(t, err)
+	assert.True(t, deps.Empty())
+}

spinifex/utils/utils.go

@@ -270,7 +270,7 @@ func DownloadFileWithProgress(url string, name string, filename string, timeout
 		var written int64
 		reader := io.TeeReader(resp.Body, progressWriter(func(n int) {
 			written += int64(n)
-			spin.UpdateText(fmt.Sprintf("Downloading %s (%s) ...", name, humanBytes(SafeInt64ToUint64(written))))
+			spin.UpdateText(fmt.Sprintf("Downloading %s (%s) ...", name, HumanBytes(SafeInt64ToUint64(written))))
 		}))
 		_, err = io.Copy(f, reader)
 		_ = spin.Stop()
@@ -291,7 +291,9 @@ func (pw progressWriter) Write(p []byte) (int, error) {
 	return len(p), nil
 }
 
-func humanBytes(b uint64) string {
+// HumanBytes formats a byte count using IEC binary suffixes (KiB, MiB, ...).
+// Values below 1024 render as exact bytes.
+func HumanBytes(b uint64) string {
 	const unit = 1024
 	if b < unit {
 		return fmt.Sprintf("%d B", b)

spinifex/utils/utils_test.go

@@ -831,7 +831,7 @@ func TestHumanBytes(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			assert.Equal(t, tt.want, humanBytes(tt.b))
+			assert.Equal(t, tt.want, HumanBytes(tt.b))
 		})
 	}
 }