From 84e12c48850ce8134ee6d0d9b46fdc0edaa71f22 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 20 May 2026 18:34:36 +0800 Subject: [PATCH] feat(types): validate snapshot name length and shape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cocoon snapshot save/import previously accepted any string for --name. A snapshot named with 100 chars or shell-unsafe chars would write straight into the DB / OCI annotation / cidata propagation chain, breaking downstream consumers (Linux HOST_NAME_MAX=64, DNS-1123 labels, etc.). Mirror VMConfig.Validate's `^[a-zA-Z0-9][a-zA-Z0-9._-]{0,62}$` regex on SnapshotConfig: - types.SnapshotConfig.Validate() enforces ≤63 chars + safe charset (empty Name still allowed — name is optional for snapshots). - cmd/snapshot/handler.go Save+Import validate the --name flag early to fail before the expensive snapshot/import operation. - snapshot/localfile Create+Import call Validate as defense-in-depth so programmatic callers (vk-cocoon, future API) can't bypass. Empty name remains valid (existing behavior — auto-generated ID is the fallback identifier). --- cmd/snapshot/handler.go | 7 +++++++ snapshot/localfile/import.go | 4 ++++ snapshot/localfile/localfile.go | 3 +++ types/snapshot.go | 13 ++++++++++++- types/snapshot_test.go | 32 ++++++++++++++++++++++++++++++++ 5 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 types/snapshot_test.go diff --git a/cmd/snapshot/handler.go b/cmd/snapshot/handler.go index 703044bf..da428104 100644 --- a/cmd/snapshot/handler.go +++ b/cmd/snapshot/handler.go @@ -41,6 +41,9 @@ func (h Handler) Save(cmd *cobra.Command, args []string) error { name, _ := cmd.Flags().GetString("name") description, _ := cmd.Flags().GetString("description") + if err = (&types.SnapshotConfig{Name: name}).Validate(); err != nil { + return err + } if name != "" { if _, inspectErr := snapBackend.Inspect(ctx, name); inspectErr == nil { return fmt.Errorf("snapshot name %q already exists", name) @@ -251,6 +254,10 @@ func (h Handler) Import(cmd *cobra.Command, args []string) error { name, _ := cmd.Flags().GetString("name") description, _ := cmd.Flags().GetString("description") + if err = (&types.SnapshotConfig{Name: name}).Validate(); err != nil { + return err + } + var r io.Reader if len(args) > 0 { f, openErr := os.Open(args[0]) //nolint:gosec diff --git a/snapshot/localfile/import.go b/snapshot/localfile/import.go index 01bdbbde..2fb6f7e0 100644 --- a/snapshot/localfile/import.go +++ b/snapshot/localfile/import.go @@ -54,6 +54,10 @@ func (lf *LocalFile) Import(ctx context.Context, r io.Reader, name, description cfg.Name = cmp.Or(name, cfg.Name) cfg.Description = cmp.Or(description, cfg.Description) + if err = cfg.Validate(); err != nil { + return "", err + } + size, sizeErr := utils.DirSize(dataDir) if sizeErr != nil { return "", fmt.Errorf("compute data dir size: %w", sizeErr) diff --git a/snapshot/localfile/localfile.go b/snapshot/localfile/localfile.go index e6165a26..72c9713d 100644 --- a/snapshot/localfile/localfile.go +++ b/snapshot/localfile/localfile.go @@ -86,6 +86,9 @@ func (lf *LocalFile) Create(ctx context.Context, cfg *types.SnapshotConfig, stre if id == "" { return "", fmt.Errorf("snapshot ID is required (must be set by caller)") } + if err = cfg.Validate(); err != nil { + return "", err + } dataDir := lf.conf.SnapshotDataDir(id) now := time.Now() diff --git a/types/snapshot.go b/types/snapshot.go index 63a31eec..0186a53d 100644 --- a/types/snapshot.go +++ b/types/snapshot.go @@ -1,6 +1,9 @@ package types -import "time" +import ( + "fmt" + "time" +) // SnapshotConfig carries the parameters for creating a snapshot. // The hypervisor fills ID, Image, ImageBlobIDs, Hypervisor, and resource fields; the CLI adds Name and Description. @@ -15,6 +18,14 @@ type SnapshotConfig struct { NICs int `json:"nics,omitempty"` } +// Validate checks SnapshotConfig caller-controlled fields. Empty Name is allowed (name is optional). +func (cfg *SnapshotConfig) Validate() error { + if cfg.Name != "" && !validName.MatchString(cfg.Name) { + return fmt.Errorf("snapshot name %q is invalid: must match %s (max 63 chars)", cfg.Name, validName.String()) + } + return nil +} + // Snapshot is the public record for a snapshot. type Snapshot struct { SnapshotConfig diff --git a/types/snapshot_test.go b/types/snapshot_test.go new file mode 100644 index 00000000..ce7502da --- /dev/null +++ b/types/snapshot_test.go @@ -0,0 +1,32 @@ +package types + +import ( + "strings" + "testing" +) + +func TestSnapshotConfig_Validate(t *testing.T) { + cases := []struct { + name string + cfgName string + wantErr bool + }{ + {"empty allowed", "", false}, + {"simple", "my-snap", false}, + {"with dot underscore", "my.snap_v1", false}, + {"max 63", strings.Repeat("a", 63), false}, + {"over 63", strings.Repeat("a", 64), true}, + {"leading hyphen", "-bad", true}, + {"space", "bad name", true}, + {"slash", "bad/name", true}, + {"control char", "bad\x00name", true}, + } + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + err := (&SnapshotConfig{Name: tt.cfgName}).Validate() + if (err != nil) != tt.wantErr { + t.Errorf("Validate(%q): err=%v, wantErr=%v", tt.cfgName, err, tt.wantErr) + } + }) + } +}