diff --git a/docs/config.md b/docs/config.md index fd2b6b2..cf4ca3a 100644 --- a/docs/config.md +++ b/docs/config.md @@ -77,9 +77,9 @@ HTTP artifact root behavior: - `server.artifact_root` applies only to `serve`. - HTTP `inline` input references work without an artifact root. -- HTTP `file` input references are resolved against `server.artifact_root` and must stay inside it. -- Relative traversal and absolute paths outside the root are rejected. -- Symlinks inside the root are followed by the operating system; do not make the artifact root writable by untrusted users. +- HTTP `file` input references are resolved against `server.artifact_root` with lexical path checks. +- Relative traversal and absolute paths that are lexically outside the root are rejected. +- Symlinks inside the root are followed by the operating system, including symlinks that point outside the root. Do not make the artifact root writable by untrusted users. - CLI `run` and `render` file inputs keep their normal direct filesystem path behavior. HTTP size-limit behavior: @@ -298,9 +298,9 @@ Rules: - Invalid generated JSON causes validation status `failed` (not a runtime error). Supported artifact reference types for request inputs are `file` and `inline`. -For HTTP `serve`, `file` references require `server.artifact_root` and must stay -inside that root. CLI `run` and `render` file inputs are not restricted by -`server.artifact_root`. +For HTTP `serve`, `file` references require `server.artifact_root` and must pass +lexical containment checks against that root. CLI `run` and `render` file inputs +are not restricted by `server.artifact_root`. ## Secrets Handling diff --git a/docs/integrations/http-api.md b/docs/integrations/http-api.md index 020dabb..28f510a 100644 --- a/docs/integrations/http-api.md +++ b/docs/integrations/http-api.md @@ -71,10 +71,13 @@ Input reference types currently supported by runtime artifact loading: - `inline` HTTP `file` references require `server.artifact_root` or `serve --artifact-root`. -Relative file URIs resolve inside that root. Absolute file URIs are accepted -only when they remain inside the root. Requests that escape the root, including -`..` traversal and absolute paths outside the root, return -`400 artifact_not_allowed`. `inline` references do not require an artifact root. +Relative file URIs resolve against that root. Absolute file URIs are accepted +only when they are lexically inside the root. Requests that escape the root by +lexical traversal, including `..` traversal and absolute paths outside the root, +return `400 artifact_not_allowed`. Symlinks inside the root are followed by the +operating system, including symlinks that point outside the root. The artifact +root must not be writable by untrusted users. `inline` references do not require +an artifact root. HTTP file artifacts above the configured artifact limit return `413 artifact_too_large`. Inline bodies are bounded by the request body limit. diff --git a/docs/internal/adapters.md b/docs/internal/adapters.md index 675401a..eb6cd6d 100644 --- a/docs/internal/adapters.md +++ b/docs/internal/adapters.md @@ -124,10 +124,10 @@ Artifact refs: - Supported reference types: `inline`, `file`. - Unsupported types return `ErrUnsupportedRefType`. - CLI `run` and `render` use direct filesystem file reads for `file` references. -- HTTP `serve` uses a restricted artifact reader: `inline` references work without a root, while `file` references require `server.artifact_root` or `--artifact-root` and must stay inside that root. +- HTTP `serve` uses a restricted artifact reader: `inline` references work without a root, while `file` references require `server.artifact_root` or `--artifact-root` and must pass lexical containment checks against that root. - HTTP `serve` applies request-body, file-artifact, and encoded-response size limits. CLI `run` and `render` do not use these HTTP limits. -- HTTP file paths are resolved with clean absolute paths and containment checks, not string-prefix checks. -- Symlinks inside the root are followed by the operating system; the configured root must not be writable by untrusted users. +- HTTP file paths are resolved with clean absolute paths and lexical containment checks, not string-prefix checks. +- Symlinks inside the root are followed by the operating system, including symlinks that point outside the root; the configured root must not be writable by untrusted users. LLM adapter: diff --git a/docs/operations.md b/docs/operations.md index 120cba2..7e6a100 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -80,14 +80,14 @@ Current inbound API behavior: - JSON request parsing rejects unknown fields. - Validation content failures still return `200 OK` with `validation.status: "failed"`. - `inline` input references work without filesystem configuration. -- `file` input references require `server.artifact_root` or `serve --artifact-root`; relative paths resolve inside that root and paths outside it are rejected. +- `file` input references require `server.artifact_root` or `serve --artifact-root`; relative traversal and absolute paths that are lexically outside that root are rejected. - Request bodies, HTTP file input artifacts, and encoded JSON responses are limited by `server.max_request_bytes`, `server.max_artifact_bytes`, and `server.max_response_bytes`. Security caveat: - `serve` has no built-in authentication or authorization. - Deploy only behind trusted controls (private network boundary, authenticated reverse proxy, API gateway, or equivalent). -- Keep the HTTP artifact root as narrow as practical and do not make it writable by untrusted users. +- Keep the HTTP artifact root as narrow as practical and do not make it writable by untrusted users. Symlinks inside the root are followed by the operating system, including symlinks that point outside the root. Sizing guidance: diff --git a/internal/artifact/reader.go b/internal/artifact/reader.go index 0b7fde3..721eeb5 100644 --- a/internal/artifact/reader.go +++ b/internal/artifact/reader.go @@ -158,14 +158,15 @@ func (r *restrictedFileReader) Read(ctx context.Context, ref domain.ArtifactRef) return nil, ErrMissingFilePath } - path, err := r.resolve(ref.URI) + path, err := r.resolveLexicalPath(ref.URI) if err != nil { return nil, err } return readFileArtifactWithLimit(path, r.maxBytes) } -func (r *restrictedFileReader) resolve(rawPath string) (string, error) { +// resolveLexicalPath checks cleaned path containment without resolving symlinks. +func (r *restrictedFileReader) resolveLexicalPath(rawPath string) (string, error) { cleanPath := filepath.Clean(strings.TrimSpace(rawPath)) var candidate string if filepath.IsAbs(cleanPath) { diff --git a/internal/artifact/reader_test.go b/internal/artifact/reader_test.go index e1cb298..bf27600 100644 --- a/internal/artifact/reader_test.go +++ b/internal/artifact/reader_test.go @@ -112,6 +112,34 @@ func TestRestrictedCompositeReader(t *testing.T) { }) } +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 {