Snapshot Identifier Validation¶

Authors

Matt Cockayne, Claude (AI drafting assistant)

Date

17 April 2026

Status

IN PROGRESS

Overview¶

The FileStore in pkg/chat/filestore.go constructs snapshot file paths via filepath.Join(s.dir, id+".json"). Today id is the Snapshot.ID field — populated from google/uuid.New() when a snapshot is created, but arriving as a caller-supplied string on the Load and Delete paths and as a JSON-unmarshalled field on Save. A caller or a malicious JSON snapshot that supplies id = "../../../etc/passwd" (or any path-traversal variant) would cause reads, writes, or deletes outside s.dir.

This is identified as H-1 in docs/development/reports/security-audit-2026-04-17.md.

Threat Model¶

The filesystem-scoped nature of the risk limits impact to the calling process's privileges, but a user running GTB can read/write anywhere that user can. A compromised snapshot store is a credible local-privilege-escalation primitive when combined with other vectors (e.g. a tool that later executes a file found in a "trusted" location).

Design Decisions¶

Two layers of defence — input validation AND resolved-path containment. Validating the ID against a character class catches the most common attacks cleanly. Additionally checking that the cleaned absolute path remains a descendant of s.dir protects against edge cases (Windows \\..\\, Unicode normalisation tricks, absolute paths starting with / or a drive letter). Either defence alone is insufficient; both together are robust.

Strict UUID character class. The snapshot ID is generated by uuid.New() which emits canonical 8-4-4-4-12 hex form. The validator rejects anything that does not match ^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$. This is stricter than "no path separators" and forecloses the entire class of traversal attacks at the shape level.

Validation is centralised in a single helper. validateSnapshotID(id string) error is called from every public path (Save, Load, Delete) so that adding a new path cannot accidentally omit the check.

Validation errors are ErrInvalidSnapshotID (sentinel). Callers that want to distinguish validation errors from I/O errors can errors.Is against it. The error message includes the offending ID truncated to its first 32 characters and quoted, to aid debugging without allowing log injection.

Path containment check is a filepath.Rel / strings.HasPrefix combo after cleaning both paths. Using Abs + Clean ensures symlink-free comparison; filepath.Rel(s.dir, target) returning a path that begins with .. indicates escape. This approach handles Windows and POSIX uniformly.

Save uses its own Snapshot.ID directly — but we re-validate on the Save path too. A caller that constructs a Snapshot struct with a manually-set ID must also pass validation. This defends against the "JSON round-trip plus re-save" attack where a malicious snapshot is loaded (and stored under a validated ID) then re-saved under an attacker-chosen ID.

Public API Changes¶

New sentinel error¶

package chat

// ErrInvalidSnapshotID is returned when a snapshot identifier fails
// validation (not a canonical UUID, contains path separators, or
// produces a filesystem path outside the store directory).
var ErrInvalidSnapshotID = errors.New("invalid snapshot identifier")

Existing API — no signature changes¶

FileStore.Save, FileStore.Load, FileStore.Delete retain their current signatures. The behaviour change is that they now return ErrInvalidSnapshotID (wrapped) for inputs that previously would have proceeded to a filesystem operation.

Callers that currently rely on the "anything goes" behaviour will break — but that behaviour is the bug being fixed.

Optional: exported validator¶

For downstream tool authors who construct snapshot IDs themselves:

// ValidateSnapshotID returns nil if id is a canonical UUID that will
// be accepted by the FileStore methods, or an error wrapping
// ErrInvalidSnapshotID otherwise.
//
// Use this when accepting snapshot IDs from user input so that
// validation happens at the boundary of the system rather than being
// deferred to Save/Load/Delete.
func ValidateSnapshotID(id string) error

Internal Implementation¶

pkg/chat/filestore.go changes¶

import "regexp"

// uuidCanonicalPattern matches the canonical 8-4-4-4-12 hex form
// produced by google/uuid.New().
var uuidCanonicalPattern = regexp.MustCompile(
    `^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`,
)

// ValidateSnapshotID — see exported doc above.
func ValidateSnapshotID(id string) error {
    if id == "" {
        return errors.WithHint(ErrInvalidSnapshotID, "snapshot ID is empty")
    }
    if !uuidCanonicalPattern.MatchString(id) {
        return errors.WithHintf(ErrInvalidSnapshotID,
            "snapshot ID %q is not a canonical UUID", truncate(id, 32))
    }
    return nil
}

// resolveStorePath joins id to the store directory and verifies the
// resolved absolute path does not escape s.dir. Used as a second line
// of defence after ValidateSnapshotID.
func (s *fileStore) resolveStorePath(id string) (string, error) {
    if err := ValidateSnapshotID(id); err != nil {
        return "", err
    }

    baseAbs, err := filepath.Abs(s.dir)
    if err != nil {
        return "", errors.Wrap(err, "resolving store directory")
    }
    baseAbs = filepath.Clean(baseAbs)

    target := filepath.Clean(filepath.Join(baseAbs, id+".json"))

    rel, err := filepath.Rel(baseAbs, target)
    if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
        return "", errors.WithHint(ErrInvalidSnapshotID,
            "snapshot path escapes the store directory")
    }

    return target, nil
}

func (s *fileStore) Save(_ context.Context, snapshot *Snapshot) error {
    path, err := s.resolveStorePath(snapshot.ID)
    if err != nil {
        return err
    }
    // ... existing encryption + write logic using path ...
}

func (s *fileStore) Load(_ context.Context, id string) (*Snapshot, error) {
    path, err := s.resolveStorePath(id)
    if err != nil {
        return nil, err
    }
    // ... existing read + decrypt logic using path ...
}

func (s *fileStore) Delete(_ context.Context, id string) error {
    path, err := s.resolveStorePath(id)
    if err != nil {
        return err
    }
    return errors.Wrap(s.fs.Remove(path), "removing snapshot file")
}

// truncate is a tiny helper to avoid log floods when logging invalid IDs.
func truncate(s string, n int) string {
    if len(s) <= n {
        return s
    }
    return s[:n] + "…"
}

List method¶

fileStore.List iterates the store directory and returns each file's ID by stripping the .json extension. A file that has been manually placed in the store with a non-canonical name (e.g. ../evil.json via a directory traversal outside this package) would fail validation and be skipped with a DEBUG log — NOT an error — so the List operation remains robust in the face of corrupt-directory state.

Project Structure¶

Error Handling¶

All errors use cockroachdb/errors and are wrapped with WithHint for user-facing clarity.

Non-Functional Requirements¶

Testing & Quality Gates¶

Documentation Deliverables¶

Observability¶

Performance Bounds¶

Security Invariants¶

1. Every exported FileStore method calls resolveStorePath before touching the filesystem.
2. resolveStorePath always runs both layers: regex validation AND path containment.
3. List never returns a filename that would fail validation — so downstream callers who iterate List output and pass IDs back to Load cannot re-enter a traversal state.
4. Snapshot.ID is always generated via uuid.New() by GTB-provided constructors.

Migration & Compatibility¶

Behaviour change: methods that previously silently traversed on non-canonical IDs now return ErrInvalidSnapshotID. No known downstream consumer relies on the traversal behaviour (it was a bug). The breaking case is exclusively the security fix itself.

No public API signature change. FileStore.Save, Load, Delete, List all retain identical signatures.

API stability: pkg/chat is Beta-tier. The new ValidateSnapshotID function and ErrInvalidSnapshotID sentinel are additive.

Implementation Phases¶

Single phase — this is a focused fix.

Estimated effort: half a day.

Resolved Decisions¶

1. UUID-only, not "no-separators". A strict shape-match is preferred over a permissive allow-list because it forecloses the entire traversal class (no .., no /, no \, no NUL, no Unicode tricks) and also documents the contract — every FileStore identifier is a UUID.
2. Second-layer path containment is retained despite the first-layer regex making it unreachable through normal code paths. Defence-in-depth against Unicode normalisation quirks, future regex relaxation, and platform-specific path weirdness.
3. List is robust, not strict. A malformed file in the store directory is logged and skipped rather than causing List to error — the opposite would let one bad file permanently break snapshot enumeration for the user.
4. No migration for existing stored snapshots. Every snapshot that GTB has ever written has a canonical UUID ID (per uuid.New()). Hand-crafted non-canonical files are not in scope.