diff --git a/docs/internal/sources.md b/docs/internal/sources.md index 411a3cb..37c5031 100644 --- a/docs/internal/sources.md +++ b/docs/internal/sources.md @@ -52,8 +52,12 @@ failures are operational errors. `internal/artifact` composes inline and file readers. The ordinary composite reader used by CLI and the public engine reads file references from the process -filesystem. The restricted composite reader used by the HTTP adapter combines -inline reading with a rooted file reader and optional byte limit. +filesystem. `internal/adapter/http` provides the restricted public artifact +reader for HTTP containment: it combines inline reading with a rooted file +reader and optional byte limit. The existing internal restricted composite +reader remains a temporary bridge for the current handler and serve wiring; it +does not define the HTTP reader's long-term boundary or carry a compatibility +promise. The rooted reader cleans paths and applies lexical containment without resolving symlinks. It checks relative references against the configured root and accepts @@ -80,6 +84,7 @@ Inspect: - `internal/profile/repository_test.go` - `internal/profile/builtin/repository_test.go` - `internal/artifact/reader_test.go` +- `internal/adapter/http/artifact_reader_test.go` - `internal/validate/standard_validator_test.go` - `internal/usecase/integration_test.go` - `engine_test.go` diff --git a/internal/adapter/http/artifact_reader.go b/internal/adapter/http/artifact_reader.go new file mode 100644 index 0000000..7490b8b --- /dev/null +++ b/internal/adapter/http/artifact_reader.go @@ -0,0 +1,165 @@ +package httpadapter + +import ( + "context" + "crypto/sha256" + "errors" + "fmt" + "io" + "mime" + "os" + "path/filepath" + "strings" + + "gitea.maximumdirect.net/eric/scriptorium" +) + +var ( + ErrFileNotAllowed = errors.New("file artifact references are not allowed") + ErrFileOutsideRoot = errors.New("file artifact path is outside artifact root") + ErrFileTooLarge = errors.New("file artifact exceeds size limit") +) + +const fallbackArtifactContentType = "text/plain" + +// NewRestrictedArtifactReader creates the HTTP artifact reader for a rooted +// filesystem and optional byte limit. An empty root permits inline artifacts +// but denies file references; a zero limit permits artifacts of any size. +func NewRestrictedArtifactReader(root string, maxBytes int64) (scriptorium.ArtifactReader, error) { + if maxBytes < 0 { + return nil, fmt.Errorf("artifact size limit must be greater than or equal to 0") + } + + cleanRoot := strings.TrimSpace(root) + if cleanRoot == "" { + return &restrictedArtifactReader{maxBytes: maxBytes}, nil + } + absRoot, err := filepath.Abs(filepath.Clean(cleanRoot)) + if err != nil { + return nil, fmt.Errorf("resolve artifact root: %w", err) + } + return &restrictedArtifactReader{root: absRoot, maxBytes: maxBytes}, nil +} + +type restrictedArtifactReader struct { + root string + maxBytes int64 +} + +var _ scriptorium.ArtifactReader = (*restrictedArtifactReader)(nil) + +func (r *restrictedArtifactReader) Read(ctx context.Context, ref scriptorium.ArtifactRef) (*scriptorium.Artifact, error) { + select { + case <-ctx.Done(): + return nil, ctx.Err() + default: + } + + switch ref.Type { + case scriptorium.ArtifactRefInline: + return readInlineArtifact(ref) + case scriptorium.ArtifactRefFile: + return r.readFileArtifact(ref) + default: + return nil, fmt.Errorf("unsupported artifact reference type %q", ref.Type) + } +} + +func readInlineArtifact(ref scriptorium.ArtifactRef) (*scriptorium.Artifact, error) { + if ref.Body == "" { + return nil, errors.New("inline artifact body is required") + } + + body := []byte(ref.Body) + return &scriptorium.Artifact{ + ContentType: fallbackArtifactContentType, + Body: body, + Size: int64(len(body)), + Hash: artifactHash(body), + URI: ref.URI, + }, nil +} + +func (r *restrictedArtifactReader) readFileArtifact(ref scriptorium.ArtifactRef) (*scriptorium.Artifact, error) { + if ref.URI == "" { + return nil, errors.New("file artifact path is required") + } + if r.root == "" { + return nil, ErrFileNotAllowed + } + + path, err := r.resolveLexicalPath(ref.URI) + if err != nil { + return nil, err + } + return readArtifactFile(path, r.maxBytes) +} + +// resolveLexicalPath checks cleaned path containment without resolving symlinks. +func (r *restrictedArtifactReader) resolveLexicalPath(rawPath string) (string, error) { + cleanPath := filepath.Clean(strings.TrimSpace(rawPath)) + candidate := cleanPath + if !filepath.IsAbs(cleanPath) { + candidate = filepath.Join(r.root, cleanPath) + } + + absCandidate, err := filepath.Abs(candidate) + if err != nil { + return "", fmt.Errorf("resolve artifact path: %w", err) + } + absCandidate = filepath.Clean(absCandidate) + + rel, err := filepath.Rel(r.root, absCandidate) + if err != nil { + return "", fmt.Errorf("compare artifact path to root: %w", err) + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) || filepath.IsAbs(rel) { + return "", ErrFileOutsideRoot + } + return absCandidate, nil +} + +func readArtifactFile(path string, maxBytes int64) (*scriptorium.Artifact, error) { + file, err := os.Open(path) + if err != nil { + return nil, fmt.Errorf("failed to read file %s: %w", path, err) + } + defer file.Close() + + info, err := file.Stat() + if err != nil { + return nil, fmt.Errorf("failed to stat file %s: %w", path, err) + } + if maxBytes > 0 && info.Size() > maxBytes { + return nil, ErrFileTooLarge + } + + var reader io.Reader = file + if maxBytes > 0 { + reader = io.LimitReader(file, maxBytes+1) + } + body, err := io.ReadAll(reader) + if err != nil { + return nil, fmt.Errorf("failed to read file %s: %w", path, err) + } + if maxBytes > 0 && int64(len(body)) > maxBytes { + return nil, ErrFileTooLarge + } + + contentType := mime.TypeByExtension(filepath.Ext(path)) + if contentType == "" { + contentType = fallbackArtifactContentType + } + return &scriptorium.Artifact{ + Name: filepath.Base(path), + ContentType: contentType, + Body: body, + URI: path, + Size: int64(len(body)), + Hash: artifactHash(body), + }, nil +} + +func artifactHash(body []byte) string { + return fmt.Sprintf("%x", sha256.Sum256(body)) +} diff --git a/internal/adapter/http/artifact_reader_test.go b/internal/adapter/http/artifact_reader_test.go new file mode 100644 index 0000000..8ab653b --- /dev/null +++ b/internal/adapter/http/artifact_reader_test.go @@ -0,0 +1,181 @@ +package httpadapter + +import ( + "context" + "errors" + "mime" + "os" + "path/filepath" + "testing" + + "gitea.maximumdirect.net/eric/scriptorium" +) + +func TestRestrictedArtifactReaderReadsContainedFiles(t *testing.T) { + root := t.TempDir() + outside := t.TempDir() + inputPath := filepath.Join(root, "input.md") + if err := os.WriteFile(inputPath, []byte("allowed"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "input.unknown"), []byte("unknown type"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(filepath.Join(root, "nested"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(outside, "secret.txt"), []byte("denied"), 0o644); err != nil { + t.Fatal(err) + } + + reader, err := NewRestrictedArtifactReader(root, 0) + if err != nil { + t.Fatalf("construct restricted reader: %v", err) + } + + for _, ref := range []scriptorium.ArtifactRef{ + {Type: scriptorium.ArtifactRefFile, URI: "nested/../input.md"}, + {Type: scriptorium.ArtifactRefFile, URI: inputPath}, + } { + artifact, err := reader.Read(context.Background(), ref) + if err != nil { + t.Fatalf("read contained path %q: %v", ref.URI, err) + } + if artifact.Name != "input.md" || artifact.URI != inputPath || artifact.Size != int64(len("allowed")) || string(artifact.Body) != "allowed" { + t.Fatalf("unexpected artifact metadata: %#v", artifact) + } + if artifact.ContentType != mime.TypeByExtension(".md") { + t.Fatalf("unexpected artifact content type: %q", artifact.ContentType) + } + if artifact.Hash != artifactHash([]byte("allowed")) { + t.Fatalf("unexpected artifact hash: %q", artifact.Hash) + } + } + + artifact, err := reader.Read(context.Background(), scriptorium.File("input.unknown")) + if err != nil { + t.Fatalf("read unknown-extension path: %v", err) + } + if artifact.ContentType != fallbackArtifactContentType { + t.Fatalf("unexpected fallback content type: %q", artifact.ContentType) + } + + for _, ref := range []scriptorium.ArtifactRef{ + {Type: scriptorium.ArtifactRefFile, URI: filepath.Join("..", filepath.Base(outside), "secret.txt")}, + {Type: scriptorium.ArtifactRefFile, URI: filepath.Join(outside, "secret.txt")}, + } { + _, err := reader.Read(context.Background(), ref) + if !errors.Is(err, ErrFileOutsideRoot) { + t.Fatalf("expected ErrFileOutsideRoot for %q, got %v", ref.URI, err) + } + } +} + +func TestRestrictedArtifactReaderFollowsSymlinkAfterLexicalCheck(t *testing.T) { + root := t.TempDir() + outside := t.TempDir() + target := filepath.Join(outside, "linked.txt") + if err := os.WriteFile(target, []byte("linked outside root"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, filepath.Join(root, "linked.txt")); err != nil { + t.Skipf("symlink creation unavailable: %v", err) + } + + reader, err := NewRestrictedArtifactReader(root, 0) + if err != nil { + t.Fatalf("construct restricted reader: %v", err) + } + artifact, err := reader.Read(context.Background(), scriptorium.File("linked.txt")) + if err != nil { + t.Fatalf("read symlink inside root: %v", err) + } + if string(artifact.Body) != "linked outside root" { + t.Fatalf("unexpected symlink artifact body: %q", artifact.Body) + } +} + +func TestRestrictedArtifactReaderWithoutRootDeniesFiles(t *testing.T) { + reader, err := NewRestrictedArtifactReader("", 0) + if err != nil { + t.Fatalf("construct rootless reader: %v", err) + } + + artifact, err := reader.Read(context.Background(), scriptorium.Inline("inline")) + if err != nil { + t.Fatalf("read inline artifact: %v", err) + } + if artifact.ContentType != fallbackArtifactContentType || string(artifact.Body) != "inline" || artifact.Hash != artifactHash([]byte("inline")) { + t.Fatalf("unexpected inline artifact: %#v", artifact) + } + + _, err = reader.Read(context.Background(), scriptorium.File("input.txt")) + if !errors.Is(err, ErrFileNotAllowed) { + t.Fatalf("expected ErrFileNotAllowed, got %v", err) + } +} + +func TestRestrictedArtifactReaderEnforcesLimits(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "exact.txt"), []byte("12345"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "large.txt"), []byte("123456"), 0o644); err != nil { + t.Fatal(err) + } + + reader, err := NewRestrictedArtifactReader(root, 5) + if err != nil { + t.Fatalf("construct limited reader: %v", err) + } + artifact, err := reader.Read(context.Background(), scriptorium.File("exact.txt")) + if err != nil || string(artifact.Body) != "12345" { + t.Fatalf("expected exact-limit artifact, got %#v and %v", artifact, err) + } + _, err = reader.Read(context.Background(), scriptorium.File("large.txt")) + if !errors.Is(err, ErrFileTooLarge) { + t.Fatalf("expected ErrFileTooLarge, got %v", err) + } + + unlimited, err := NewRestrictedArtifactReader(root, 0) + if err != nil { + t.Fatalf("construct unlimited reader: %v", err) + } + artifact, err = unlimited.Read(context.Background(), scriptorium.File("large.txt")) + if err != nil || string(artifact.Body) != "123456" { + t.Fatalf("expected unlimited artifact, got %#v and %v", artifact, err) + } + + if _, err := NewRestrictedArtifactReader(root, -1); err == nil { + t.Fatal("expected negative limit to fail") + } +} + +func TestRestrictedArtifactReaderRejectsCanceledAndMalformedReferences(t *testing.T) { + reader, err := NewRestrictedArtifactReader(t.TempDir(), 0) + if err != nil { + t.Fatalf("construct reader: %v", err) + } + + canceledCtx, cancel := context.WithCancel(context.Background()) + cancel() + for _, ref := range []scriptorium.ArtifactRef{ + scriptorium.Inline("input"), + scriptorium.File("input.txt"), + } { + _, err := reader.Read(canceledCtx, ref) + if !errors.Is(err, context.Canceled) { + t.Fatalf("expected cancellation for %#v, got %v", ref, err) + } + } + + for _, ref := range []scriptorium.ArtifactRef{ + {Type: scriptorium.ArtifactRefType("unsupported")}, + {Type: scriptorium.ArtifactRefInline}, + {Type: scriptorium.ArtifactRefFile}, + } { + if _, err := reader.Read(context.Background(), ref); err == nil { + t.Fatalf("expected malformed reference %#v to fail", ref) + } + } +} diff --git a/internal/artifact/reader.go b/internal/artifact/reader.go index 721eeb5..3d7f2a9 100644 --- a/internal/artifact/reader.go +++ b/internal/artifact/reader.go @@ -41,10 +41,16 @@ func NewCompositeReader() Reader { } } +// NewRestrictedCompositeReader is a temporary bridge for legacy adapter wiring. +// It has no compatibility promise and will be removed when those adapters use +// the public HTTP artifact reader. func NewRestrictedCompositeReader(root string) (Reader, error) { return NewRestrictedCompositeReaderWithLimit(root, 0) } +// NewRestrictedCompositeReaderWithLimit is a temporary bridge for legacy +// adapter wiring. It has no compatibility promise and will be removed when +// those adapters use the public HTTP artifact reader. func NewRestrictedCompositeReaderWithLimit(root string, maxBytes int64) (Reader, error) { fileReader, err := newRestrictedFileReader(root, maxBytes) if err != nil { diff --git a/internal/artifact/reader_test.go b/internal/artifact/reader_test.go index bf27600..442e811 100644 --- a/internal/artifact/reader_test.go +++ b/internal/artifact/reader_test.go @@ -4,7 +4,6 @@ import ( "context" "errors" "os" - "path/filepath" "testing" "gitea.maximumdirect.net/eric/scriptorium/internal/domain" @@ -57,157 +56,6 @@ func TestCompositeReader_Read(t *testing.T) { }) } -func TestRestrictedCompositeReader(t *testing.T) { - ctx := context.Background() - root := t.TempDir() - outside := t.TempDir() - - if err := os.WriteFile(filepath.Join(root, "input.txt"), []byte("allowed"), 0o644); err != nil { - t.Fatal(err) - } - if err := os.Mkdir(filepath.Join(root, "nested"), 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(outside, "secret.txt"), []byte("denied"), 0o644); err != nil { - t.Fatal(err) - } - - reader, err := NewRestrictedCompositeReader(root) - if err != nil { - t.Fatalf("expected restricted reader construction, got %v", err) - } - - t.Run("accepts relative contained path", func(t *testing.T) { - art, err := reader.Read(ctx, domain.ArtifactRef{Type: domain.ArtifactRefFile, URI: "nested/../input.txt"}) - if err != nil { - t.Fatalf("expected contained relative path to succeed, got %v", err) - } - if string(art.Body) != "allowed" { - t.Fatalf("unexpected artifact body: %q", string(art.Body)) - } - }) - - t.Run("accepts absolute contained path", func(t *testing.T) { - art, err := reader.Read(ctx, domain.ArtifactRef{Type: domain.ArtifactRefFile, URI: filepath.Join(root, "input.txt")}) - if err != nil { - t.Fatalf("expected contained absolute path to succeed, got %v", err) - } - if art.Name != "input.txt" { - t.Fatalf("unexpected artifact name: %q", art.Name) - } - }) - - t.Run("rejects relative traversal outside root", func(t *testing.T) { - _, err := reader.Read(ctx, domain.ArtifactRef{Type: domain.ArtifactRefFile, URI: filepath.Join("..", filepath.Base(outside), "secret.txt")}) - if !errors.Is(err, ErrFileOutsideRoot) { - t.Fatalf("expected ErrFileOutsideRoot, got %v", err) - } - }) - - t.Run("rejects absolute path outside root", func(t *testing.T) { - _, err := reader.Read(ctx, domain.ArtifactRef{Type: domain.ArtifactRefFile, URI: filepath.Join(outside, "secret.txt")}) - if !errors.Is(err, ErrFileOutsideRoot) { - t.Fatalf("expected ErrFileOutsideRoot, got %v", err) - } - }) -} - -func TestRestrictedCompositeReaderFollowsSymlinkInsideRoot(t *testing.T) { - ctx := context.Background() - root := t.TempDir() - outside := t.TempDir() - - target := filepath.Join(outside, "linked.txt") - if err := os.WriteFile(target, []byte("linked outside root"), 0o644); err != nil { - t.Fatal(err) - } - link := filepath.Join(root, "linked.txt") - if err := os.Symlink(target, link); err != nil { - t.Skipf("symlink creation unavailable: %v", err) - } - - reader, err := NewRestrictedCompositeReader(root) - if err != nil { - t.Fatalf("expected restricted reader construction, got %v", err) - } - - art, err := reader.Read(ctx, domain.ArtifactRef{Type: domain.ArtifactRefFile, URI: "linked.txt"}) - if err != nil { - t.Fatalf("expected symlink inside root to be followed, got %v", err) - } - if string(art.Body) != "linked outside root" { - t.Fatalf("unexpected artifact body: %q", string(art.Body)) - } -} - -func TestRestrictedCompositeReaderWithoutRootDeniesFileRefs(t *testing.T) { - reader, err := NewRestrictedCompositeReader("") - if err != nil { - t.Fatalf("expected restricted reader construction, got %v", err) - } - - art, err := reader.Read(context.Background(), domain.ArtifactRef{Type: domain.ArtifactRefInline, Body: "inline"}) - if err != nil { - t.Fatalf("expected inline ref to work without artifact root, got %v", err) - } - if string(art.Body) != "inline" { - t.Fatalf("unexpected inline body: %q", string(art.Body)) - } - - _, err = reader.Read(context.Background(), domain.ArtifactRef{Type: domain.ArtifactRefFile, URI: "input.txt"}) - if !errors.Is(err, ErrFileNotAllowed) { - t.Fatalf("expected ErrFileNotAllowed, got %v", err) - } -} - -func TestRestrictedCompositeReaderFileSizeLimit(t *testing.T) { - ctx := context.Background() - root := t.TempDir() - if err := os.WriteFile(filepath.Join(root, "exact.txt"), []byte("12345"), 0o644); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(root, "large.txt"), []byte("123456"), 0o644); err != nil { - t.Fatal(err) - } - - reader, err := NewRestrictedCompositeReaderWithLimit(root, 5) - if err != nil { - t.Fatalf("expected restricted reader construction, got %v", err) - } - - art, err := reader.Read(ctx, domain.ArtifactRef{Type: domain.ArtifactRefFile, URI: "exact.txt"}) - if err != nil { - t.Fatalf("expected file at limit to succeed, got %v", err) - } - if string(art.Body) != "12345" { - t.Fatalf("unexpected artifact body: %q", string(art.Body)) - } - - _, err = reader.Read(ctx, domain.ArtifactRef{Type: domain.ArtifactRefFile, URI: "large.txt"}) - if !errors.Is(err, ErrFileTooLarge) { - t.Fatalf("expected ErrFileTooLarge, got %v", err) - } -} - -func TestRestrictedCompositeReaderFileSizeLimitZeroDisablesLimit(t *testing.T) { - root := t.TempDir() - if err := os.WriteFile(filepath.Join(root, "large.txt"), []byte("123456"), 0o644); err != nil { - t.Fatal(err) - } - - reader, err := NewRestrictedCompositeReaderWithLimit(root, 0) - if err != nil { - t.Fatalf("expected restricted reader construction, got %v", err) - } - art, err := reader.Read(context.Background(), domain.ArtifactRef{Type: domain.ArtifactRefFile, URI: "large.txt"}) - if err != nil { - t.Fatalf("expected unlimited reader to succeed, got %v", err) - } - if string(art.Body) != "123456" { - t.Fatalf("unexpected artifact body: %q", string(art.Body)) - } -} - func TestFileReader_Read(t *testing.T) { content := []byte("test file content") tmpFile, err := os.CreateTemp("", "artifact_test_*.txt")