fix(sdkserver): marshal workspace tool input as JSON #17
No reviewers
Labels
No labels
area/engine
clarity/clear
enhancement
security
size/M
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
basepeak-ai/gptscript!17
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/sdkserver-workspace-input-json"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Impact
The workspace handlers in
pkg/sdkserver/workspaces.gobuilt tool input withfmt.Sprintf(e.g.{"workspace_id": "%s", "file_path": "%s"}). A"in any caller-supplied value could append a second"workspace_id"key, and Go's JSON decoding keeps the last duplicate, so a crafted file path (or prefix / revision ID / workspace ID) could address another workspace: read, write, list or delete files and revisions there.Fix
All ten affected handlers now build their input through
workspaceToolInput, whichjson.Marshals a map and setsworkspace_idlast so no field can override it. The write-file path already usedjson.Marshal. No other Sprintf-built JSON inpkg/sdkserver.Test
workspaces_test.goadds tests showing that a file path containing", "workspace_id": "otherleavesworkspace_idunchanged and round-trips exactly, that the workspace ID itself is escaped, and that aworkspace_idfield cannot replace it. They failed against the Sprintf-equivalent helper and pass now.go test ./pkg/sdkserver/...andgo vet ./...pass.golangci-lintreports nothing new; the 32 issues onmainare still there.The workspace handlers built the tool input with fmt.Sprintf, e.g. `{"workspace_id": "%s", "file_path": "%s"}`. A quote in any caller-supplied value (file path, prefix, revision ID, or the workspace ID itself) escaped its string and could append a second "workspace_id" key. Go's JSON decoding keeps the last duplicate key, so a crafted file path could make the tool read, write, or delete in another workspace. Build the input for all ten affected handlers through a single workspaceToolInput helper that json.Marshals a map and sets workspace_id last, so no field can override or replace it.Fresh-context review: nothing blocking. No existing caller sees a change: key names, the string-typed
with_latest_revision_id, and empty fields are all identical.Behaviour change, for the better: backslashes and control characters in a path used to be reinterpreted, or produced invalid JSON that went out with an empty workspace id. They now arrive exactly as given. Pinned by
TestWorkspaceToolInputKeepsBackslashesAndControlCharacters(0202a55).CI: the
testfailure is pre-existing onmain. The runner cannotgit ls-remotethe privategit.basepeak.devmodules, somake tidyfails before any test runs. Locally,go test -count=1 ./pkg/sdkserver/...andgo vetpass.Follow-ups noted, out of scope:
pkg/prompt/credential.go:22builds JSON with Sprintf from provider-supplied text...traversal throughfile_path, depending on workspace-provider's path handling; needs a separate look.