Add HTTP restricted artifact reader

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
+filesystem. `internal/adapter/http` provides the restricted public artifact
-inline reading with a rooted file reader and optional byte limit.
+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`

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))
+}

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)
+		}
+	}
+}

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 {

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")