Skip to content

Commit 7d9b3dc

Browse files
authored
feat(types): validate snapshot name length and shape (#61)
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).
1 parent 58741ad commit 7d9b3dc

5 files changed

Lines changed: 58 additions & 1 deletion

File tree

cmd/snapshot/handler.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@ func (h Handler) Save(cmd *cobra.Command, args []string) error {
4141
name, _ := cmd.Flags().GetString("name")
4242
description, _ := cmd.Flags().GetString("description")
4343

44+
if err = (&types.SnapshotConfig{Name: name}).Validate(); err != nil {
45+
return err
46+
}
4447
if name != "" {
4548
if _, inspectErr := snapBackend.Inspect(ctx, name); inspectErr == nil {
4649
return fmt.Errorf("snapshot name %q already exists", name)
@@ -251,6 +254,10 @@ func (h Handler) Import(cmd *cobra.Command, args []string) error {
251254
name, _ := cmd.Flags().GetString("name")
252255
description, _ := cmd.Flags().GetString("description")
253256

257+
if err = (&types.SnapshotConfig{Name: name}).Validate(); err != nil {
258+
return err
259+
}
260+
254261
var r io.Reader
255262
if len(args) > 0 {
256263
f, openErr := os.Open(args[0]) //nolint:gosec

snapshot/localfile/import.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ func (lf *LocalFile) Import(ctx context.Context, r io.Reader, name, description
5454
cfg.Name = cmp.Or(name, cfg.Name)
5555
cfg.Description = cmp.Or(description, cfg.Description)
5656

57+
if err = cfg.Validate(); err != nil {
58+
return "", err
59+
}
60+
5761
size, sizeErr := utils.DirSize(dataDir)
5862
if sizeErr != nil {
5963
return "", fmt.Errorf("compute data dir size: %w", sizeErr)

snapshot/localfile/localfile.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ func (lf *LocalFile) Create(ctx context.Context, cfg *types.SnapshotConfig, stre
8686
if id == "" {
8787
return "", fmt.Errorf("snapshot ID is required (must be set by caller)")
8888
}
89+
if err = cfg.Validate(); err != nil {
90+
return "", err
91+
}
8992

9093
dataDir := lf.conf.SnapshotDataDir(id)
9194
now := time.Now()

types/snapshot.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package types
22

3-
import "time"
3+
import (
4+
"fmt"
5+
"time"
6+
)
47

58
// SnapshotConfig carries the parameters for creating a snapshot.
69
// The hypervisor fills ID, Image, ImageBlobIDs, Hypervisor, and resource fields; the CLI adds Name and Description.
@@ -15,6 +18,14 @@ type SnapshotConfig struct {
1518
NICs int `json:"nics,omitempty"`
1619
}
1720

21+
// Validate checks SnapshotConfig caller-controlled fields. Empty Name is allowed (name is optional).
22+
func (cfg *SnapshotConfig) Validate() error {
23+
if cfg.Name != "" && !validName.MatchString(cfg.Name) {
24+
return fmt.Errorf("snapshot name %q is invalid: must match %s (max 63 chars)", cfg.Name, validName.String())
25+
}
26+
return nil
27+
}
28+
1829
// Snapshot is the public record for a snapshot.
1930
type Snapshot struct {
2031
SnapshotConfig

types/snapshot_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
package types
2+
3+
import (
4+
"strings"
5+
"testing"
6+
)
7+
8+
func TestSnapshotConfig_Validate(t *testing.T) {
9+
cases := []struct {
10+
name string
11+
cfgName string
12+
wantErr bool
13+
}{
14+
{"empty allowed", "", false},
15+
{"simple", "my-snap", false},
16+
{"with dot underscore", "my.snap_v1", false},
17+
{"max 63", strings.Repeat("a", 63), false},
18+
{"over 63", strings.Repeat("a", 64), true},
19+
{"leading hyphen", "-bad", true},
20+
{"space", "bad name", true},
21+
{"slash", "bad/name", true},
22+
{"control char", "bad\x00name", true},
23+
}
24+
for _, tt := range cases {
25+
t.Run(tt.name, func(t *testing.T) {
26+
err := (&SnapshotConfig{Name: tt.cfgName}).Validate()
27+
if (err != nil) != tt.wantErr {
28+
t.Errorf("Validate(%q): err=%v, wantErr=%v", tt.cfgName, err, tt.wantErr)
29+
}
30+
})
31+
}
32+
}

0 commit comments

Comments
 (0)