ReadPassword races with its worker goroutine when the context is canceled

Output of restic version

Source review of current master:

restic 0.19.1-dev
commit ba802d42b7294c98b62c16d1157ea3e80820c019

What backend/service did you use to store the repository?

None. The problem is in interactive terminal password input and occurs before backend access.

Problem description / Steps to reproduce

internal/terminal.ReadPassword starts a goroutine which writes to the function's named err result parameter:

func ReadPassword(ctx context.Context, inFd int, out io.Writer, prompt string) (password string, err error) {
	// ...
	var buf []byte

	go func() {
		defer close(done)
		_, err = fmt.Fprint(out, prompt)
		if err != nil {
			return
		}
		buf, err = term.ReadPassword(inFd)
		if err != nil {
			return
		}
		_, err = fmt.Fprintln(out)
	}()

	select {
	case <-ctx.Done():
		// ...
		return "", ctx.Err()
	case <-done:
	}
	// ...
}

When ctx.Done() wins the select, the receive from done has not occurred. The explicit return assigns ctx.Err() to the named err result while the worker goroutine can concurrently read or write that same variable. There is no happens-before relation between those accesses.

A deterministic race-detector test can exercise the path with a pseudo-terminal:

1. Call ReadPassword with the PTY file descriptor and a cancelable context.
2. Wait until the worker has printed the prompt and is blocked in term.ReadPassword.
3. Cancel the context and wait for ReadPassword to return.
4. Keep the process alive and write a password plus newline to the PTY, allowing the leaked worker to resume and write err again.

I found this with Bifrost's public bifrost.correctness.go-data-race policy. It reports one definite finding at the cancellation return, comprising the five conflicting accesses to err on lines 27, 28, 31, 32, and 35 of internal/terminal/password.go. Manual source inspection confirms the conflicting goroutine lifetimes.

The current source already documents the related lifetime problem: "If the context is canceled, the function leaks the password reading goroutine."

This appears distinct from #5477 / #5554. Those addressed delayed terminal-status output hiding the password prompt by flushing before password input. The unsynchronized cancellation path inside ReadPassword is unchanged on current master.

Expected behavior

Canceling interactive password input should return the context error without concurrent access to the function's result state. If the underlying terminal read cannot be canceled, the remaining worker should at least own its local result and communicate it through a channel rather than capture the caller's named return variables.

Actual behavior

On cancellation, ReadPassword returns while its worker remains alive. The worker can subsequently access err concurrently with the return assignment. Depending on timing, this can race with or overwrite the intended ctx.Err() result. The worker can also resume later and write a newline to the terminal after the caller has already restored terminal state and continued cancellation/shutdown handling.

This is most likely to occur when a user sends SIGINT or otherwise cancels a command while restic is waiting at an interactive password prompt. It is a correctness and shutdown-reliability issue rather than a security or repository-integrity issue.

Do you have any idea what may have caused this?

The worker goroutine closes done on normal completion, which correctly orders its writes before the normal <-done arm. The cancellation arm deliberately returns without receiving from done, so that ordering does not exist there. Capturing the function's named err return variable makes the cancellation return and the leaked worker share mutable state.

One possible direction is for the goroutine to use only local variables and send a {password, err} value over a buffered result channel. The normal path can receive that value, while the cancellation path can return without sharing the named result. This removes the data race even if an uncancelable terminal read means the goroutine itself cannot immediately be stopped.

Did restic help you today? Did it make you happy in any way?

Yes. The code and comments made this concurrency path unusually straightforward to investigate. Thank you for maintaining restic.