From 33dcb05f7419c7f24bcfaf15af660a9af869a6c8 Mon Sep 17 00:00:00 2001 From: webster Date: Fri, 22 May 2026 16:48:56 -0700 Subject: [PATCH 1/2] Make workdir consistent with the sidecar structure. Chunk sidecars use /home/user as the home dir, this makes the command logic reference that known location instead of relative paths. --- internal/cmd/sidecar.go | 2 +- internal/cmd/validate.go | 2 +- internal/sidecar/active_test.go | 2 +- internal/sidecar/sync.go | 6 +++--- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/cmd/sidecar.go b/internal/cmd/sidecar.go index 682f0fa7..0ecc464a 100644 --- a/internal/cmd/sidecar.go +++ b/internal/cmd/sidecar.go @@ -447,7 +447,7 @@ func newSidecarSyncCmd() *cobra.Command { cmd.Flags().StringVar(&sidecarID, "sidecar-id", "", "Sidecar ID (defaults to active sidecar)") cmd.Flags().StringVar(&identityFile, "identity-file", "", "SSH identity file") - cmd.Flags().StringVar(&workdir, "workdir", "", "Destination path on sidecar (auto-detected as ~/workspace/ when omitted)") + cmd.Flags().StringVar(&workdir, "workdir", "", "Destination path on sidecar (defaults to /home/user/ when omitted)") return cmd } diff --git a/internal/cmd/validate.go b/internal/cmd/validate.go index cc55a2e2..26cba27b 100644 --- a/internal/cmd/validate.go +++ b/internal/cmd/validate.go @@ -111,7 +111,7 @@ func newValidateCmd() *cobra.Command { cmd.Flags().StringVar(&opts.sidecarID, "sidecar-id", "", "Sidecar ID for remote execution") cmd.Flags().StringVar(&opts.orgID, "org-id", "", "Organization ID (used when creating a new sidecar)") cmd.Flags().StringVar(&opts.identityFile, "identity-file", "", "SSH identity file (uses ssh-agent or ~/.ssh/chunk_ai when omitted)") - cmd.Flags().StringVar(&opts.workdir, "workdir", "", "Working directory on sidecar (reads from sidecar.json, defaults to ./workspace)") + cmd.Flags().StringVar(&opts.workdir, "workdir", "", "Working directory on sidecar (reads from sidecar.json, defaults to /home/user/)") cmd.Flags().BoolVar(&opts.dryRun, "dry-run", false, "Show commands without executing") cmd.Flags().BoolVar(&opts.list, "list", false, "List all configured commands") cmd.Flags().BoolVar(&opts.jsonOut, "json", false, "Output as JSON (only applies with --list)") diff --git a/internal/sidecar/active_test.go b/internal/sidecar/active_test.go index 3ffd8627..8c6a3671 100644 --- a/internal/sidecar/active_test.go +++ b/internal/sidecar/active_test.go @@ -238,5 +238,5 @@ func TestResolveWorkspaceDefaultFallback(t *testing.T) { setupXDGData(t) got := ResolveWorkspace(context.Background(), "", "myrepo") - assert.Equal(t, got, "./workspace/myrepo") + assert.Equal(t, got, "/home/user/myrepo") } diff --git a/internal/sidecar/sync.go b/internal/sidecar/sync.go index 5d491c9b..a9890a00 100644 --- a/internal/sidecar/sync.go +++ b/internal/sidecar/sync.go @@ -14,10 +14,10 @@ import ( "github.com/CircleCI-Public/chunk-cli/internal/iostream" ) -const workspaceDir = "./workspace" +const workspaceDir = "/home/user" // ResolveWorkspace determines the workspace path. Priority: -// 1. CLI --workdir flag 2. sidecar.json workspace 3. default ./workspace/. +// 1. CLI --workdir flag 2. sidecar.json workspace 3. default /home/user/. func ResolveWorkspace(ctx context.Context, cliWorkdir, repo string) string { if cliWorkdir != "" { return cliWorkdir @@ -48,7 +48,7 @@ func persistWorkspace(ctx context.Context, workspace string) error { // Sync synchronises local changes to a sidecar over SSH. // It ensures the workspace base exists, clones the repo into workdir if absent, // then resets to the remote base and applies a patch of local changes. -// workdir overrides the destination path; defaults to /workspace/. +// workdir overrides the destination path; defaults to /home/user/. func Sync(ctx context.Context, client *circleci.Client, sidecarID, identityFile, authSock, workdir string, status iostream.StatusFunc) error { From da61cde49407c4996123f91ede6a559916937894 Mon Sep 17 00:00:00 2001 From: webster Date: Wed, 27 May 2026 11:53:05 -0400 Subject: [PATCH 2/2] Address review: validate workspace path depth, derive sidecar home from env - ResolveWorkspace now returns (string, error) and errors when repo is empty with no saved workspace, preventing rm -rf on the bare home dir - Replace hardcoded /home/user constant with sidecarHome() which reads CHUNK_SIDECAR_HOME env var, falling back to /home/user - Update callers in sync.go and validate.go - Add tests for the empty-repo error and env var override Co-Authored-By: Claude Sonnet 4.6 --- .gitignore | 1 + internal/cmd/validate.go | 5 ++++- internal/sidecar/active_test.go | 30 +++++++++++++++++++++++++++--- internal/sidecar/sync.go | 29 +++++++++++++++++++++-------- 4 files changed, 53 insertions(+), 12 deletions(-) diff --git a/.gitignore b/.gitignore index 46263800..cf35ef98 100644 --- a/.gitignore +++ b/.gitignore @@ -42,3 +42,4 @@ PLANS.md # unikraft .unikraft/ +test-fetch-on-stale-sidecar.log diff --git a/internal/cmd/validate.go b/internal/cmd/validate.go index 26cba27b..afb10420 100644 --- a/internal/cmd/validate.go +++ b/internal/cmd/validate.go @@ -423,7 +423,10 @@ func openSSHSession(ctx context.Context, client *circleci.Client, sidecarID, ide } cwd, _ := os.Getwd() _, repo, _ := gitremote.DetectOrgAndRepo(cwd) - dest := sidecar.ResolveWorkspace(ctx, workdir, repo) + dest, err := sidecar.ResolveWorkspace(ctx, workdir, repo) + if err != nil { + return nil, "", &userError{msg: "Could not determine workspace path.", err: err} + } merged := hostForwardEnv(rc.CircleCIToken) if merged == nil { merged = make(map[string]string, len(envVars)) diff --git a/internal/sidecar/active_test.go b/internal/sidecar/active_test.go index 8c6a3671..9435c4e9 100644 --- a/internal/sidecar/active_test.go +++ b/internal/sidecar/active_test.go @@ -216,7 +216,8 @@ func TestResolveWorkspaceCLIFlagWins(t *testing.T) { ctx := context.Background() assert.NilError(t, SaveActive(ctx, ActiveSidecar{SidecarID: "sb-1", Workspace: "/workspace/saved"})) - got := ResolveWorkspace(ctx, "/workspace/override", "myrepo") + got, err := ResolveWorkspace(ctx, "/workspace/override", "myrepo") + assert.NilError(t, err) assert.Equal(t, got, "/workspace/override") } @@ -228,7 +229,8 @@ func TestResolveWorkspaceSidecarFallback(t *testing.T) { ctx := context.Background() assert.NilError(t, SaveActive(ctx, ActiveSidecar{SidecarID: "sb-1", Workspace: "/workspace/saved"})) - got := ResolveWorkspace(ctx, "", "myrepo") + got, err := ResolveWorkspace(ctx, "", "myrepo") + assert.NilError(t, err) assert.Equal(t, got, "/workspace/saved") } @@ -237,6 +239,28 @@ func TestResolveWorkspaceDefaultFallback(t *testing.T) { t.Chdir(dir) setupXDGData(t) - got := ResolveWorkspace(context.Background(), "", "myrepo") + got, err := ResolveWorkspace(context.Background(), "", "myrepo") + assert.NilError(t, err) assert.Equal(t, got, "/home/user/myrepo") } + +func TestResolveWorkspaceEmptyRepoErrors(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + setupXDGData(t) + + _, err := ResolveWorkspace(context.Background(), "", "") + assert.ErrorContains(t, err, "repo name is empty") +} + +func TestResolveWorkspaceSidecarHomeEnvVar(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + setupXDGData(t) + + t.Setenv("CHUNK_SIDECAR_HOME", "/home/runner") + + got, err := ResolveWorkspace(context.Background(), "", "myrepo") + assert.NilError(t, err) + assert.Equal(t, got, "/home/runner/myrepo") +} diff --git a/internal/sidecar/sync.go b/internal/sidecar/sync.go index a9890a00..974890a9 100644 --- a/internal/sidecar/sync.go +++ b/internal/sidecar/sync.go @@ -14,21 +14,31 @@ import ( "github.com/CircleCI-Public/chunk-cli/internal/iostream" ) -const workspaceDir = "/home/user" +// sidecarHome returns the base home directory on the sidecar. It reads +// CHUNK_SIDECAR_HOME so the default "/home/user" can be overridden when the +// image uses a different OS user. +func sidecarHome() string { + if h := os.Getenv("CHUNK_SIDECAR_HOME"); h != "" { + return h + } + return "/home/user" +} // ResolveWorkspace determines the workspace path. Priority: -// 1. CLI --workdir flag 2. sidecar.json workspace 3. default /home/user/. -func ResolveWorkspace(ctx context.Context, cliWorkdir, repo string) string { +// 1. CLI --workdir flag 2. sidecar.json workspace 3. default /. +// Returns an error if no repo-specific path can be determined (repo empty and no +// saved workspace), because the bare home dir is not safe to pass to rm -rf. +func ResolveWorkspace(ctx context.Context, cliWorkdir, repo string) (string, error) { if cliWorkdir != "" { - return cliWorkdir + return cliWorkdir, nil } if active, err := LoadActive(ctx); err == nil && active != nil && active.Workspace != "" { - return active.Workspace + return active.Workspace, nil } if repo == "" { - return workspaceDir + return "", fmt.Errorf("sync: cannot determine workspace: repo name is empty and no workspace is saved") } - return workspaceDir + "/" + repo + return sidecarHome() + "/" + repo, nil } // persistWorkspace saves the resolved workspace back to the sidecar file if it @@ -67,7 +77,10 @@ func Sync(ctx context.Context, return fmt.Errorf("sync: %w", err) } - repoPath := ResolveWorkspace(ctx, workdir, repo) + repoPath, err := ResolveWorkspace(ctx, workdir, repo) + if err != nil { + return err + } if err := persistWorkspace(ctx, repoPath); err != nil { status(iostream.LevelWarn, fmt.Sprintf("Could not save workspace: %v", err))