Skip to content

fix: Workspace.path returns leading slash for API consistency - #12993

Closed
paikend wants to merge 9 commits into
dagger:mainfrom
paikend:fix/workspace-path-leading-slash
Closed

paikend wants to merge 9 commits into
dagger:mainfrom
paikend:fix/workspace-path-leading-slash

Conversation

@paikend

@paikend paikend commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Workspace.path now returns paths with a leading / (e.g. /app/frontend instead of app/frontend, / instead of .)
  • Aligns with the rest of the Workspace API which already uses leading-slash paths

Changes

  • engine/server/session_workspaces.go: use path.Join("/", detected.Path) when setting core.Workspace.Path
  • core/workspace.go: update field doc to reflect the new format

Closes #12970

Test plan

  • TestLocalWorkspaceAddress passes (Docker, Go 1.25, Linux)
  • TestRemoteWorkspaceAddress passes
  • Internal workspace.Workspace.Path (used in filepath.Join) unchanged — no side effects

🤖 Generated with Claude Code

@paikend
paikend force-pushed the fix/workspace-path-leading-slash branch 2 times, most recently from 6797c49 to 1c53914 Compare April 20, 2026 11:15
paikend and others added 3 commits May 4, 2026 21:31
Closes dagger#12970

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: kent paik <paikend@gmail.com>
Per issue dagger#12970, Workspace.path now returns a leading-slash path
from the workspace boundary. Update the integration test assertion
accordingly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: kent paik <paikend@gmail.com>
…s.Path

The prior approach set `core.Workspace.Path` to `path.Join("/", detected.Path)`
which made the internal boundary-relative path absolute. This silently weakened
workspace path-traversal protection: `resolveWorkspacePath("../..", "/")` =
`"/"` because `filepath.Join` absorbs traversal at the root, so
`ws.directory("../..")` resolved to the workspace root instead of erroring
with "resolves outside root" (see `TestWorkspacePathTraversal`).

Keep `ws.Path` stored as boundary-relative (no leading slash) and expose it
over GraphQL via a custom `path` resolver that applies the existing
`workspaceAPIPath` helper. This leaves internal consumers
(`resolveWorkspacePath`, `filepath.Join(ws.Root, ws.Path, ...)`, etc.) and
their traversal checks unchanged while still satisfying the API contract
from dagger#12970.

- Revert `engine/server/session_workspaces.go` to `Path: detected.Path`
- Drop `field:"true"` from `core.Workspace.Path` so auto-exposure is off
- Register `dagql.Func("path", s.path)` with the same doc string the SDKs
  already generate, so codegen stays in sync

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: kent paik <paikend@gmail.com>
@paikend
paikend force-pushed the fix/workspace-path-leading-slash branch from 2ae9fe3 to 88fc5a7 Compare May 4, 2026 12:31
Signed-off-by: kent paik <paikend@gmail.com>
@paikend
paikend force-pushed the fix/workspace-path-leading-slash branch from 8979ee4 to 09ec220 Compare May 14, 2026 00:25
@paikend paikend closed this Jun 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Workspace.path should return paths with a leading /

1 participant