Skip to content

Redirect direct backend git global config to an isolated per-task file #77

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jasonkeung merged 3 commits into from
Jun 1, 2026
Merged

Redirect direct backend git global config to an isolated per-task file #77

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d099ade
Redirect direct backend git global config to an isolated per-task file
jasonkeung May 28, 2026
12c79c2
Validate task ID to prevent path traversal
jasonkeung May 29, 2026
fceda8d
Add direct backend git config setup tests
jasonkeung May 29, 2026
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
42 changes: 37 additions & 5 deletions internal/worker/direct.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,29 @@ func hostBaseEnv() []string {
return base
}

// prepareTaskGitConfig returns the path to use as the task's global git config
// (GIT_CONFIG_GLOBAL) along with a cleanup function. Redirecting only git's
// global config keeps writes like `git config --global url.<x>.insteadOf` out of
// the developer's real ~/.gitconfig (and $XDG_CONFIG_HOME/git/config) without
// repointing HOME for every tool the agent runs.
func prepareTaskGitConfig(workspaceDir string, usingTargetDir bool) (string, func(), error) {

@harryalbert harryalbert May 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems like we're not gating this on the oz-local script — OOC is there a reason to do this across the board instead of limiting to only when debugging (maybe self hosted)?

@bnavetta bnavetta Jun 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should definitely use a per-task config file for any run with the direct backend. There may be multiple concurrent runs on the same host, and while we can't fully isolate them, we should do so as much as possible.

@jasonkeung jasonkeung Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also dont think its as straight forward to differentiate between selfhosted workers (used by real clients) and local, and ideally our oz-local is as similar to real production behavior as possible too

if !usingTargetDir {
return filepath.Join(workspaceDir, ".gitconfig"), func() {}, nil
}

// In shared target-dir mode the workspace is the user's real checkout, so keep
// the throwaway global config in a temporary directory outside of it.
dir, err := os.MkdirTemp("", "oz-gitconfig-")
if err != nil {
return "", nil, fmt.Errorf("failed to create temporary git config directory: %w", err)
}
return filepath.Join(dir, ".gitconfig"), func() {
if err := os.RemoveAll(dir); err != nil {
log.Warnf(context.Background(), "Failed to remove temporary git config dir %s: %v", dir, err)
}
}, nil
}

// DirectBackendConfig holds configuration specific to the direct (non-containerized) backend.
type DirectBackendConfig struct {
WorkspaceRoot string
Expand Down Expand Up @@ -124,18 +147,24 @@ func (b *DirectBackend) ExecuteTask(ctx context.Context, params *TaskParams) err
}
log.Infof(ctx, "Created workspace: %s", workspaceDir)
}
gitConfigPath, cleanupGitConfig, err := prepareTaskGitConfig(workspaceDir, usingTargetDir)
if err != nil {
return err
}
defer cleanupGitConfig()
gitConfigEnv := []string{fmt.Sprintf("GIT_CONFIG_GLOBAL=%s", gitConfigPath)}

@bnavetta bnavetta Jun 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may need to escape the path, in case the workspace dir is set to something with spaces

@jasonkeung jasonkeung Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added testing for a workspace path with spaces. Since we are using exec.Cmd.Env, it shouldnt need escaping


defer func() {
if usingTargetDir {
// Don't clean up the shared target directory.
b.runTeardownIfConfigured(ctx, taskID, workspaceDir)
b.runTeardownIfConfigured(ctx, taskID, workspaceDir, gitConfigPath)
return
}
if b.config.NoCleanup {
log.Infof(ctx, "Skipping cleanup for workspace: %s", workspaceDir)
return
}
b.cleanup(ctx, taskID, workspaceDir)
b.cleanup(ctx, taskID, workspaceDir, gitConfigPath)
}()

// 2. Create temp environment file for setup script to write to.
Expand All @@ -160,6 +189,7 @@ func (b *DirectBackend) ExecuteTask(ctx context.Context, params *TaskParams) err
envVars = append(envVars, fmt.Sprintf("%s=%s", key, value))
}
envVars = mergeEnvVars(envVars, harnessEnvVars(workspaceDir, params))
envVars = mergeEnvVars(envVars, gitConfigEnv)

// 4. Run setup command if configured.
if b.config.SetupCommand != "" {
Expand Down Expand Up @@ -187,6 +217,7 @@ func (b *DirectBackend) ExecuteTask(ctx context.Context, params *TaskParams) err
setupScriptVars = append(setupScriptVars, fmt.Sprintf("%s=%s", key, value))
}
envVars = mergeEnvVars(envVars, setupScriptVars)
envVars = mergeEnvVars(envVars, gitConfigEnv)

// 6. Invoke oz CLI with base args.
// Start from a minimal host base (HOME, TMPDIR, PATH) and overlay task env vars,
Expand Down Expand Up @@ -235,12 +266,13 @@ func (b *DirectBackend) PreservesTasksOnShutdown() bool {
}

// runTeardownIfConfigured runs the teardown command if one is configured.
func (b *DirectBackend) runTeardownIfConfigured(ctx context.Context, taskID, workspaceDir string) {
func (b *DirectBackend) runTeardownIfConfigured(ctx context.Context, taskID, workspaceDir, gitConfigPath string) {
if b.config.TeardownCommand == "" {
return
}
teardownEnv := []string{
fmt.Sprintf("OZ_WORKSPACE_ROOT=%s", workspaceDir),
fmt.Sprintf("GIT_CONFIG_GLOBAL=%s", gitConfigPath),
"OZ_WORKER_BACKEND=direct",
fmt.Sprintf("OZ_RUN_ID=%s", taskID),
}
Expand All @@ -255,8 +287,8 @@ func (b *DirectBackend) runTeardownIfConfigured(ctx context.Context, taskID, wor
}

// cleanup runs the teardown command (if configured) and removes the workspace directory.
func (b *DirectBackend) cleanup(ctx context.Context, taskID, workspaceDir string) {
b.runTeardownIfConfigured(ctx, taskID, workspaceDir)
func (b *DirectBackend) cleanup(ctx context.Context, taskID, workspaceDir, gitConfigPath string) {
b.runTeardownIfConfigured(ctx, taskID, workspaceDir, gitConfigPath)

log.Infof(ctx, "Removing workspace: %s", workspaceDir)
if err := os.RemoveAll(workspaceDir); err != nil {
Expand Down
Loading
Loading