Validate portable workspace identifiers
This commit is contained in:
@@ -391,7 +391,11 @@ func previousSessionCacheCandidatePaths(paths SessionPaths, canonicalRelPath str
|
||||
out := make([]string, 0, len(relCandidates))
|
||||
seen := map[string]struct{}{}
|
||||
for _, rel := range relCandidates {
|
||||
abs := filepath.Clean(SessionPreviousArtifactPath(paths, rel))
|
||||
abs, err := SessionPreviousArtifactPath(paths, rel)
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
abs = filepath.Clean(abs)
|
||||
if _, ok := seen[abs]; ok {
|
||||
continue
|
||||
}
|
||||
|
||||
@@ -436,8 +436,8 @@ func TestResolvePreviousSessionArtifactWithCatalogPrefersManifestInputRecord(t *
|
||||
workspace := t.TempDir()
|
||||
paths := buildSessionPaths(workspace, "campaign", "session")
|
||||
|
||||
manifestBackedPath := SessionPreviousArtifactPath(paths, "artifacts/session_recap.md")
|
||||
fallbackPath := SessionPreviousArtifactPath(paths, "session_recap.md")
|
||||
manifestBackedPath := mustSessionPreviousArtifactPath(t, paths, "artifacts/session_recap.md")
|
||||
fallbackPath := mustSessionPreviousArtifactPath(t, paths, "session_recap.md")
|
||||
if err := os.MkdirAll(filepath.Dir(manifestBackedPath), 0o755); err != nil {
|
||||
t.Fatalf("MkdirAll() error = %v", err)
|
||||
}
|
||||
@@ -487,7 +487,7 @@ func TestResolvePreviousSessionArtifactWithCatalogFallsBackToPreparedCachePath(t
|
||||
workspace := t.TempDir()
|
||||
paths := buildSessionPaths(workspace, "campaign", "session")
|
||||
|
||||
fallbackPath := SessionPreviousArtifactPath(paths, "session_recap.md")
|
||||
fallbackPath := mustSessionPreviousArtifactPath(t, paths, "session_recap.md")
|
||||
if err := os.MkdirAll(filepath.Dir(fallbackPath), 0o755); err != nil {
|
||||
t.Fatalf("MkdirAll() error = %v", err)
|
||||
}
|
||||
|
||||
@@ -92,8 +92,8 @@ func (c *ArtifactCatalog) RegisterExtractionArtifacts(configured map[string]Extr
|
||||
|
||||
for _, key := range keys {
|
||||
trimmed := strings.TrimSpace(key)
|
||||
if trimmed == "" {
|
||||
return fmt.Errorf("extraction artifact keys must be non-empty")
|
||||
if !artifactpolicy.IsConfiguredKey(trimmed) {
|
||||
return fmt.Errorf("extraction artifact key %q must match ^[a-z][a-z0-9_]*$", key)
|
||||
}
|
||||
if _, exists := c.extractionIndex[trimmed]; exists {
|
||||
return fmt.Errorf("duplicate extraction artifact key %q", trimmed)
|
||||
@@ -152,16 +152,16 @@ func (c *ArtifactCatalog) RegisterConfiguredArtifacts(
|
||||
selectedSet := map[string]struct{}{}
|
||||
for _, key := range selected {
|
||||
trimmed := strings.TrimSpace(key)
|
||||
if trimmed == "" {
|
||||
return fmt.Errorf("selected artifact keys must be non-empty")
|
||||
if !artifactpolicy.IsConfiguredKey(trimmed) {
|
||||
return fmt.Errorf("selected artifact key %q must match ^[a-z][a-z0-9_]*$", key)
|
||||
}
|
||||
selectedSet[trimmed] = struct{}{}
|
||||
}
|
||||
|
||||
for _, key := range keys {
|
||||
trimmed := strings.TrimSpace(key)
|
||||
if trimmed == "" {
|
||||
return fmt.Errorf("configured artifact keys must be non-empty")
|
||||
if !artifactpolicy.IsConfiguredKey(trimmed) {
|
||||
return fmt.Errorf("configured artifact key %q must match ^[a-z][a-z0-9_]*$", key)
|
||||
}
|
||||
def := configured[key]
|
||||
sourceID := ConfiguredArtifactSourceID(trimmed)
|
||||
|
||||
@@ -114,6 +114,26 @@ func TestArtifactCatalogRejectsSelectedUnknownArtifact(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestArtifactCatalogRejectsUnsafeArtifactKeys(t *testing.T) {
|
||||
unsafe := []string{"", "artifact/name", `artifact\name`, "artifact-name", "café"}
|
||||
for _, key := range unsafe {
|
||||
t.Run("configured", func(t *testing.T) {
|
||||
catalog := NewArtifactCatalog()
|
||||
err := catalog.RegisterConfiguredArtifacts(map[string]ConfiguredArtifactDefinition{key: {OutputPath: "artifacts/output.md"}}, nil)
|
||||
if err == nil {
|
||||
t.Fatalf("RegisterConfiguredArtifacts(%q) error = nil, want rejection", key)
|
||||
}
|
||||
})
|
||||
t.Run("extraction", func(t *testing.T) {
|
||||
catalog := NewArtifactCatalog()
|
||||
err := catalog.RegisterExtractionArtifacts(map[string]ExtractionArtifactDefinition{key: {LaneID: "lane"}})
|
||||
if err == nil {
|
||||
t.Fatalf("RegisterExtractionArtifacts(%q) error = nil, want rejection", key)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestArtifactCatalogRejectsConfiguredSourceConflictAcrossRegistrations(t *testing.T) {
|
||||
catalog := NewArtifactCatalog()
|
||||
if err := catalog.RegisterConfiguredArtifacts(
|
||||
|
||||
@@ -86,6 +86,9 @@ func LoadCurrentRunPointer(ctx context.Context, store storage.ObjectStore, curre
|
||||
if runID == "" {
|
||||
return "", fmt.Errorf("current run pointer %q is empty", key)
|
||||
}
|
||||
if err := ValidateRunIdentity(runID); err != nil {
|
||||
return "", fmt.Errorf("current run pointer %q contains an unsafe legacy run id; migrate remote state before use: %w", key, err)
|
||||
}
|
||||
return runID, nil
|
||||
}
|
||||
|
||||
@@ -163,6 +166,16 @@ func ValidateCurrentStateIdentity(state *CurrentState, validation CurrentStateVa
|
||||
manifestSessionID := strings.TrimSpace(state.Manifest.SessionID)
|
||||
manifestCampaign := strings.TrimSpace(state.Manifest.Campaign)
|
||||
manifestRunID := strings.TrimSpace(state.Manifest.RunID)
|
||||
if manifestCampaign != "" {
|
||||
if err := ValidateSessionIdentity(manifestCampaign, manifestSessionID); err != nil {
|
||||
return fmt.Errorf("current manifest contains unsafe legacy identities; migrate remote state before use: %w", err)
|
||||
}
|
||||
}
|
||||
if manifestRunID != "" {
|
||||
if err := ValidateRunIdentity(manifestRunID); err != nil {
|
||||
return fmt.Errorf("current manifest contains an unsafe legacy run id; migrate remote state before use: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
if expectedSessionID != "" && manifestSessionID != expectedSessionID {
|
||||
return fmt.Errorf(
|
||||
|
||||
40
internal/artifacts/identity_paths_test.go
Normal file
40
internal/artifacts/identity_paths_test.go
Normal file
@@ -0,0 +1,40 @@
|
||||
package artifacts
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestEnsureLayoutForRejectsUnsafeIdentityBeforeFilesystemAccess(t *testing.T) {
|
||||
workspace := t.TempDir()
|
||||
store := NewLocalStore(workspace)
|
||||
for _, identity := range []struct {
|
||||
name string
|
||||
campaign string
|
||||
session string
|
||||
}{
|
||||
{name: "campaign traversal", campaign: "../outside", session: "session"},
|
||||
{name: "session separator", campaign: "campaign", session: `session\other`},
|
||||
{name: "session drive", campaign: "campaign", session: `C:\outside`},
|
||||
{name: "unicode", campaign: "café", session: "session"},
|
||||
} {
|
||||
t.Run(identity.name, func(t *testing.T) {
|
||||
if _, err := store.EnsureLayoutFor(identity.campaign, identity.session); err == nil {
|
||||
t.Fatal("EnsureLayoutFor() error = nil, want unsafe identity rejection")
|
||||
}
|
||||
if _, err := os.Stat(filepath.Join(workspace, "work")); !os.IsNotExist(err) {
|
||||
t.Fatalf("workspace path was created for rejected identity, stat err = %v", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestSessionPreviousArtifactPathRejectsEscapingRelativePath(t *testing.T) {
|
||||
paths := buildSessionPaths(t.TempDir(), "campaign", "session")
|
||||
for _, relative := range []string{"../escape.json", "/absolute.json", `C:\escape.json`} {
|
||||
if _, err := SessionPreviousArtifactPath(paths, relative); err == nil {
|
||||
t.Fatalf("SessionPreviousArtifactPath(%q) error = nil, want confinement failure", relative)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -51,6 +51,9 @@ func (s *LocalStore) EnsureLayoutFor(campaign, sessionID string) (SessionPaths,
|
||||
if campaign == "" {
|
||||
return SessionPaths{}, fmt.Errorf("campaign is required")
|
||||
}
|
||||
if err := ValidateSessionIdentity(campaign, sessionID); err != nil {
|
||||
return SessionPaths{}, err
|
||||
}
|
||||
|
||||
return s.ensureLayout(s.SessionPathsFor(campaign, sessionID))
|
||||
}
|
||||
@@ -62,6 +65,9 @@ func (s *LocalStore) ensureLayout(paths SessionPaths) (SessionPaths, error) {
|
||||
if strings.TrimSpace(paths.SessionID) == "" {
|
||||
return SessionPaths{}, fmt.Errorf("sessionID is required")
|
||||
}
|
||||
if err := ValidateSessionIdentity(paths.CampaignID, paths.SessionID); err != nil {
|
||||
return SessionPaths{}, err
|
||||
}
|
||||
|
||||
dirs := []string{
|
||||
paths.Root,
|
||||
|
||||
@@ -7,6 +7,7 @@ import (
|
||||
"strings"
|
||||
|
||||
"gitea.maximumdirect.net/eric/narratio/internal/config"
|
||||
"gitea.maximumdirect.net/eric/narratio/internal/pathsafe"
|
||||
)
|
||||
|
||||
// SessionPaths contains canonical local paths for one session work directory.
|
||||
@@ -63,11 +64,15 @@ func SessionPreviousArtifactsDirForCampaign(rootDir, campaign, sessionID string)
|
||||
return filepath.Join(SessionPreviousDirForCampaign(rootDir, campaign, sessionID), config.PathArtifactsDirSegment)
|
||||
}
|
||||
|
||||
// SessionPreviousArtifactPathForCampaign returns a path under previous/artifacts for one artifact.
|
||||
func SessionPreviousArtifactPathForCampaign(rootDir, campaign, sessionID, artifactRelativePath string) string {
|
||||
return filepath.Join(
|
||||
// SessionPreviousArtifactPathForCampaign returns a confined path under
|
||||
// previous/artifacts for one artifact.
|
||||
func SessionPreviousArtifactPathForCampaign(rootDir, campaign, sessionID, artifactRelativePath string) (string, error) {
|
||||
if err := ValidateSessionIdentity(campaign, sessionID); err != nil {
|
||||
return "", err
|
||||
}
|
||||
return pathsafe.JoinSlashRelativeUnderRoot(
|
||||
SessionPreviousArtifactsDirForCampaign(rootDir, campaign, sessionID),
|
||||
filepath.FromSlash(previousArtifactCacheRelativePath(artifactRelativePath)),
|
||||
previousArtifactCacheRelativePath(artifactRelativePath),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -190,9 +195,34 @@ func SessionPreviousArtifactsDir(paths SessionPaths) string {
|
||||
return paths.PreviousArtifactsDir
|
||||
}
|
||||
|
||||
// SessionPreviousArtifactPath returns a path under previous/artifacts for already-resolved session paths.
|
||||
func SessionPreviousArtifactPath(paths SessionPaths, artifactRelativePath string) string {
|
||||
return filepath.Join(paths.PreviousArtifactsDir, filepath.FromSlash(previousArtifactCacheRelativePath(artifactRelativePath)))
|
||||
// SessionPreviousArtifactPath returns a confined path under previous/artifacts
|
||||
// for already-resolved session paths.
|
||||
func SessionPreviousArtifactPath(paths SessionPaths, artifactRelativePath string) (string, error) {
|
||||
if err := ValidateSessionIdentity(paths.CampaignID, paths.SessionID); err != nil {
|
||||
return "", err
|
||||
}
|
||||
return pathsafe.JoinSlashRelativeUnderRoot(paths.PreviousArtifactsDir, previousArtifactCacheRelativePath(artifactRelativePath))
|
||||
}
|
||||
|
||||
// ValidateSessionIdentity validates the opaque components used in workspace
|
||||
// and object-store session paths.
|
||||
func ValidateSessionIdentity(campaign, sessionID string) error {
|
||||
if err := pathsafe.ValidateOpaqueSegment(campaign); err != nil {
|
||||
return fmt.Errorf("campaign identity: %w", err)
|
||||
}
|
||||
if err := pathsafe.ValidateOpaqueSegment(sessionID); err != nil {
|
||||
return fmt.Errorf("session identity: %w", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// ValidateRunIdentity validates the opaque component used below a session's
|
||||
// run directory and object-store prefix.
|
||||
func ValidateRunIdentity(runID string) error {
|
||||
if err := pathsafe.ValidateOpaqueSegment(runID); err != nil {
|
||||
return fmt.Errorf("run identity: %w", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func previousArtifactCacheRelativePath(artifactRelativePath string) string {
|
||||
|
||||
@@ -96,15 +96,15 @@ func TestSessionPreviousPathsForCampaign(t *testing.T) {
|
||||
t.Fatalf("SessionPreviousArtifactsDirForCampaign() = %q, want %q", artifactsDir, wantArtifactsDir)
|
||||
}
|
||||
|
||||
artifactPath := SessionPreviousArtifactPathForCampaign(root, "forsaken", "2026-04-19", "session_recap.md")
|
||||
artifactPath := mustSessionPreviousArtifactPathForCampaign(t, root, "forsaken", "2026-04-19", "session_recap.md")
|
||||
wantArtifactPath := filepath.Join(root, "work", "forsaken", "2026-04-19", "previous", "artifacts", "session_recap.md")
|
||||
if artifactPath != wantArtifactPath {
|
||||
t.Fatalf("SessionPreviousArtifactPathForCampaign() = %q, want %q", artifactPath, wantArtifactPath)
|
||||
t.Fatalf("mustSessionPreviousArtifactPathForCampaign(t, ) = %q, want %q", artifactPath, wantArtifactPath)
|
||||
}
|
||||
|
||||
previousRelativeArtifactPath := SessionPreviousArtifactPathForCampaign(root, "forsaken", "2026-04-19", "artifacts/session_recap.md")
|
||||
previousRelativeArtifactPath := mustSessionPreviousArtifactPathForCampaign(t, root, "forsaken", "2026-04-19", "artifacts/session_recap.md")
|
||||
if previousRelativeArtifactPath != wantArtifactPath {
|
||||
t.Fatalf("SessionPreviousArtifactPathForCampaign(previous-relative) = %q, want %q", previousRelativeArtifactPath, wantArtifactPath)
|
||||
t.Fatalf("mustSessionPreviousArtifactPathForCampaign(t, previous-relative) = %q, want %q", previousRelativeArtifactPath, wantArtifactPath)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -120,13 +120,13 @@ func TestSessionPreviousPathsFromSessionPaths(t *testing.T) {
|
||||
t.Fatalf("SessionPreviousArtifactsDir() = %q, want %q", got, paths.PreviousArtifactsDir)
|
||||
}
|
||||
|
||||
got := SessionPreviousArtifactPath(paths, "quest_log.json")
|
||||
got := mustSessionPreviousArtifactPath(t, paths, "quest_log.json")
|
||||
want := filepath.Join(paths.PreviousArtifactsDir, "quest_log.json")
|
||||
if got != want {
|
||||
t.Fatalf("SessionPreviousArtifactPath() = %q, want %q", got, want)
|
||||
}
|
||||
|
||||
got = SessionPreviousArtifactPath(paths, "artifacts/session_recap.md")
|
||||
got = mustSessionPreviousArtifactPath(t, paths, "artifacts/session_recap.md")
|
||||
want = filepath.Join(paths.PreviousArtifactsDir, "session_recap.md")
|
||||
if got != want {
|
||||
t.Fatalf("SessionPreviousArtifactPath(previous-relative) = %q, want %q", got, want)
|
||||
|
||||
21
internal/artifacts/paths_test_helpers_test.go
Normal file
21
internal/artifacts/paths_test_helpers_test.go
Normal file
@@ -0,0 +1,21 @@
|
||||
package artifacts
|
||||
|
||||
import "testing"
|
||||
|
||||
func mustSessionPreviousArtifactPath(t *testing.T, paths SessionPaths, relative string) string {
|
||||
t.Helper()
|
||||
path, err := SessionPreviousArtifactPath(paths, relative)
|
||||
if err != nil {
|
||||
t.Fatalf("SessionPreviousArtifactPath() error = %v", err)
|
||||
}
|
||||
return path
|
||||
}
|
||||
|
||||
func mustSessionPreviousArtifactPathForCampaign(t *testing.T, root, campaign, sessionID, relative string) string {
|
||||
t.Helper()
|
||||
path, err := SessionPreviousArtifactPathForCampaign(root, campaign, sessionID, relative)
|
||||
if err != nil {
|
||||
t.Fatalf("SessionPreviousArtifactPathForCampaign() error = %v", err)
|
||||
}
|
||||
return path
|
||||
}
|
||||
@@ -39,6 +39,9 @@ func ResolvePublishSessionPrefix(cfg *config.Config, m *manifest.Manifest) (stri
|
||||
if cfg.Pipeline.Storage.S3 == nil {
|
||||
return "", fmt.Errorf("pipeline.storage.s3 configuration is required")
|
||||
}
|
||||
if err := ValidateSessionIdentity(campaign, sessionID); err != nil {
|
||||
return "", err
|
||||
}
|
||||
|
||||
sessionPrefix := S3SessionPrefix(cfg.Pipeline.Storage.S3.RootPrefix, campaign, sessionID)
|
||||
if strings.TrimSpace(sessionPrefix) == "" {
|
||||
@@ -68,6 +71,9 @@ func ResolvePublishRunPrefix(cfg *config.Config, m *manifest.Manifest) (string,
|
||||
if runID == "" {
|
||||
return "", fmt.Errorf("run id is required")
|
||||
}
|
||||
if err := ValidateRunIdentity(runID); err != nil {
|
||||
return "", err
|
||||
}
|
||||
return S3RunPrefix(sessionPrefix, runID), nil
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user