Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions internal/executionplane/artifact_collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ import (
"encoding/json"
"fmt"
"mime"
"net/url"
"os"
"path/filepath"
"sort"
"strings"

"github.com/devlikebear/tars/internal/atomicwrite"
"github.com/devlikebear/tars/internal/fileuri"
)

const (
Expand Down Expand Up @@ -324,17 +324,17 @@ func writeCollectedArtifact(root, kind, name string, raw []byte) (CollectedArtif
mediaType = "application/octet-stream"
}
return CollectedArtifact{
Kind: kind, Name: filepath.ToSlash(name), URI: (&url.URL{Scheme: "file", Path: path}).String(),
Kind: kind, Name: filepath.ToSlash(name), URI: fileuri.New(path),
Digest: fmt.Sprintf("sha256:%x", digest[:]), MediaType: mediaType, SizeBytes: int64(len(raw)),
}, nil
}

func filepathFromURI(raw string) (string, error) {
parsed, err := url.Parse(raw)
if err != nil || parsed.Scheme != "file" || parsed.Host != "" {
return "", fmt.Errorf("executionplane: invalid file artifact URI")
path, err := fileuri.Path(raw)
if err != nil {
return "", fmt.Errorf("executionplane: invalid file artifact URI: %w", err)
}
return filepath.FromSlash(parsed.Path), nil
return path, nil
}

var _ ArtifactCollector = (*FileArtifactCollector)(nil)
4 changes: 2 additions & 2 deletions internal/executionplane/local_environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ import (
"context"
"encoding/json"
"fmt"
"net/url"
"os"
"path/filepath"
"strings"
"time"

"github.com/devlikebear/tars/internal/fileuri"
"github.com/devlikebear/tars/internal/proofverifier"
"github.com/devlikebear/tars/internal/workstore"
)
Expand Down Expand Up @@ -87,7 +87,7 @@ func (provider *LocalEnvironmentProvider) Sync(ctx context.Context, environment
now := provider.now().UTC()
return EnvironmentSnapshot{
ID: "snapshot:" + strings.TrimPrefix(digest, "sha256:"), Digest: digest,
URI: (&url.URL{Scheme: "file", Path: recovered.RootDir}).String(),
URI: fileuri.New(recovered.RootDir),
MetadataJSON: metadata, CreatedAt: now,
}, nil
}
Expand Down
4 changes: 2 additions & 2 deletions internal/executionplane/worktree_environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ import (
"crypto/sha256"
"encoding/json"
"fmt"
"net/url"
"os"
"os/exec"
"path/filepath"
"strings"
"time"

"github.com/devlikebear/tars/internal/atomicwrite"
"github.com/devlikebear/tars/internal/fileuri"
"github.com/devlikebear/tars/internal/proofverifier"
"github.com/devlikebear/tars/internal/workstore"
)
Expand Down Expand Up @@ -161,7 +161,7 @@ func (provider *ManagedWorktreeProvider) Sync(ctx context.Context, environment E
now := provider.now().UTC()
return EnvironmentSnapshot{
ID: "snapshot:" + strings.TrimPrefix(digest, "sha256:"), Digest: digest,
URI: (&url.URL{Scheme: "file", Path: recovered.RootDir}).String(),
URI: fileuri.New(recovered.RootDir),
MetadataJSON: metadata, CreatedAt: now,
}, nil
}
Expand Down
67 changes: 67 additions & 0 deletions internal/fileuri/fileuri.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// Package fileuri converts between local paths and file: URIs.
//
// It exists because url.URL{Scheme: "file", Path: path} is wrong on Windows:
// `C:\dir\artifact.txt` renders as file://C:%5Cdir%5Cartifact.txt, where the
// drive letter becomes the URI host and every separator is percent-encoded.
// Such a URI is not resolvable — the parser here rejects it, and a naive
// consumer that strips the scheme gets a path that cannot be opened.
//
// A file URI needs forward slashes and a leading one ahead of the drive
// letter: file:///C:/dir/artifact.txt. On unix the absolute path already
// starts with a slash, so both forms agree and existing URIs stay valid.
package fileuri

import (
"fmt"
"net/url"
"path/filepath"
"strings"
)

// New returns the file: URI for path.
//
// It takes no error because it cannot fail for its callers. path is made
// absolute so the URI identifies the same file from any working directory,
// and filepath.Abs only fails when the working directory cannot be
// determined — unreachable here, since every caller already holds an absolute
// path. Returning an error anyway would put a branch in four callers that no
// test could ever reach.
func New(path string) string {
resolved := strings.TrimSpace(path)
if absolute, err := filepath.Abs(resolved); err == nil {
resolved = absolute
}
uriPath := filepath.ToSlash(resolved)
if !strings.HasPrefix(uriPath, "/") {
uriPath = "/" + uriPath
}
return (&url.URL{Scheme: "file", Path: uriPath}).String()
}

// Path returns the local path a file: URI refers to.
//
// A URI with a host is rejected rather than guessed at: it is either a remote
// UNC reference this code does not handle, or the malformed shape New exists
// to avoid.
func Path(raw string) (string, error) {
parsed, err := url.Parse(strings.TrimSpace(raw))
if err != nil {
return "", fmt.Errorf("fileuri: parse %q: %w", raw, err)
}
if parsed.Scheme != "file" {
return "", fmt.Errorf("fileuri: %q is not a file URI", raw)
}
if parsed.Host != "" {
return "", fmt.Errorf("fileuri: %q has a host", raw)
}
uriPath := parsed.Path
// file:///C:/dir becomes /C:/dir when parsed; the leading slash is part of
// the URI grammar, not of the Windows path.
if len(uriPath) > 2 && uriPath[0] == '/' && uriPath[2] == ':' {
uriPath = uriPath[1:]
}
if uriPath == "" {
return "", fmt.Errorf("fileuri: %q has no path", raw)
}
return filepath.FromSlash(uriPath), nil
}
68 changes: 68 additions & 0 deletions internal/fileuri/fileuri_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package fileuri

import (
"path/filepath"
"strings"
"testing"
)

// TestRoundTripPreservesPath is the property every caller depends on: a path
// turned into a URI and back must name the same file.
func TestRoundTripPreservesPath(t *testing.T) {
want := filepath.Join(t.TempDir(), "nested", "artifact.txt")

uri := New(want)
got, err := Path(uri)
if err != nil {
t.Fatalf("Path(%q): %v", uri, err)
}
if got != want {
t.Fatalf("round trip = %q, want %q", got, want)
}
}

// TestNewHasNoHostAndForwardSlashes pins the shape that was wrong on Windows,
// where url.URL{Scheme: "file", Path: path} put the drive letter in the host
// and percent-encoded every separator.
func TestNewHasNoHostAndForwardSlashes(t *testing.T) {
uri := New(filepath.Join(t.TempDir(), "artifact.txt"))
if !strings.HasPrefix(uri, "file:///") {
t.Fatalf("uri = %q, want a file:/// prefix", uri)
}
if strings.Contains(uri, "%5C") || strings.Contains(uri, `\`) {
t.Fatalf("uri = %q, want forward slashes only", uri)
}
}

func TestNewResolvesRelativePaths(t *testing.T) {
t.Chdir(t.TempDir())

uri := New("artifact.txt")
path, err := Path(uri)
if err != nil {
t.Fatalf("Path: %v", err)
}
if !filepath.IsAbs(path) {
t.Fatalf("path = %q, want an absolute path", path)
}
if filepath.Base(path) != "artifact.txt" {
t.Fatalf("path = %q, want it to end at the requested file", path)
}
}

func TestPathRejectsUnusableURIs(t *testing.T) {
cases := map[string]string{
"other scheme": "https://example.test/artifact.txt",
"host present": "file://example.test/artifact.txt",
"no path": "file://",
"not a uri": "::not a uri::",
"malformed old": `file://C:%5Cdir%5Cartifact.txt`,
}
for name, raw := range cases {
t.Run(name, func(t *testing.T) {
if got, err := Path(raw); err == nil {
t.Fatalf("Path(%q) = %q, want an error", raw, got)
}
})
}
}
12 changes: 7 additions & 5 deletions internal/tarsserver/work_scheduler_runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/devlikebear/tars/internal/agentruntime"
"github.com/devlikebear/tars/internal/config"
"github.com/devlikebear/tars/internal/executionplane"
"github.com/devlikebear/tars/internal/fileuri"
"github.com/devlikebear/tars/internal/session"
"github.com/devlikebear/tars/internal/workerprotocol"
"github.com/devlikebear/tars/internal/workscheduler"
Expand Down Expand Up @@ -639,12 +640,13 @@ func shellQuoteForTest(value string) string {
return "'" + strings.ReplaceAll(value, "'", "'\\''") + "'"
}

// filepathFromArtifactURI resolves an artifact URI the way production does.
//
// It used to strip "file://" and treat the rest as a path, which quietly
// accepted the malformed URIs this code used to produce on Windows and handed
// back a percent-encoded string that could not be opened.
func filepathFromArtifactURI(raw string) (string, error) {
const prefix = "file://"
if !strings.HasPrefix(raw, prefix) {
return "", os.ErrInvalid
}
return strings.TrimPrefix(raw, prefix), nil
return fileuri.Path(raw)
}

func TestBuildNativeWorkExecutionPlaneSelectsHonestEnvironment(t *testing.T) {
Expand Down
3 changes: 2 additions & 1 deletion internal/workerprotocol/artifact_quarantine.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"unicode/utf8"

"github.com/devlikebear/tars/internal/atomicwrite"
"github.com/devlikebear/tars/internal/fileuri"
"github.com/devlikebear/tars/internal/secrets"
)

Expand Down Expand Up @@ -217,7 +218,7 @@ func (quarantine *ArtifactQuarantine) release(ctx context.Context, placementID s

func releasedArtifact(path string, artifact WireArtifact) ReleasedArtifact {
return ReleasedArtifact{
Name: artifact.Name, URI: (&url.URL{Scheme: "file", Path: path}).String(),
Name: artifact.Name, URI: fileuri.New(path),
Digest: artifact.Digest, MediaType: artifact.MediaType, SizeBytes: int64(len(artifact.Data)),
}
}
Expand Down
Loading