Skip to content

Rework `internal/handler/exec.go` to remove `os.Chdir` calls and respect parent context.

See func getConn

	// Use a working directory that won't get removed or unmounted.
	if err := os.Chdir("/"); err != nil {
		return nil, err
	}

When opening a connection to Gitaly, we first change the working directory of the whole process 😬

When gitlab-shell was written in ruby, it shelled out to little Go subprocesses that run this code, so it was fine in context.

When we rewrote gitlab-shell into Go, we switched to calling this code in-process, but it was still a process per connection, and this happened late in the cycle. It's probably fine right now.

In gitlab-sshd, we run this code inside a long-lived server process, where it's definitely not fine 😬. It could cause a range of unexpected outcomes.

In addition to the Chdir call, each connection is used for a single RPC before being called, which is inefficient, and the connections have an independent context (and opentracing span), so cancellation and accounting doesn't work correctly in either the gitlab-shell or gitlab-sshd contexts.

Edited by Nick Thomas