Skip to content

feat(BREV-2866): open shell in expected container working directory#308

Open
nisolanki1209 wants to merge 2 commits intomainfrom
nisolanki1209/BREV-2866
Open

feat(BREV-2866): open shell in expected container working directory#308
nisolanki1209 wants to merge 2 commits intomainfrom
nisolanki1209/BREV-2866

Conversation

@nisolanki1209
Copy link
Contributor

@nisolanki1209 nisolanki1209 commented Mar 4, 2026

Problem

When launching a custom container, brev shell did not take the container’s WORKDIR into account. It opened a standard shell session, which defaults to the root user’s home directory, instead of the container’s WORKDIR.

As a result, users were placed in /root rather than the container’s intended WORKDIR.

Solution

Updated the runSSH function to detect and respect the container’s WORKDIR when launching brev shell.

  • Detect the container WORKDIR by reading /proc/1/cwd (PID 1 working directory).
  • Fallback to pwd if /proc/1/cwd is unavailable.
  • Change into the resolved directory before starting the shell.
  • Print a warning if the directory is not found, but still launch the shell to avoid blocking access to the container.
  • Use ${SHELL} with /bin/sh as a fallback for minimal containers.

@nisolanki1209 nisolanki1209 self-assigned this Mar 4, 2026
@nisolanki1209 nisolanki1209 marked this pull request as ready for review March 4, 2026 08:37
@nisolanki1209 nisolanki1209 requested a review from a team as a code owner March 4, 2026 08:37
Copilot AI review requested due to automatic review settings March 4, 2026 08:37
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the brev shell command to respect the container's WORKDIR when opening an interactive shell session in a custom container. Previously, users were placed in the default SSH login directory (/root) regardless of the container's intended working directory.

Changes:

  • The runSSH function signature is updated to accept a host bool parameter, and the call site is updated accordingly.
  • When host=false (the container path), the SSH command now reads /proc/1/cwd via readlink (with pwd as a fallback) to determine the container's working directory, then cds into it before executing the shell.
  • The host=true path retains the original simple ssh command behavior without modifications.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

cmd = fmt.Sprintf("%s && ssh %s", sshAgentEval, sshAlias)
} else {
// SSH into VM and respect container WORKDIR if containerized, otherwise use default directory
cmd = fmt.Sprintf("%s && ssh -t %s 'DIR=$(readlink -f /proc/1/cwd 2>/dev/null || pwd); cd \"$DIR\" || echo \"Warning: Could not access container directory\" >&2; exec ${SHELL:-/bin/sh}'", sshAgentEval, sshAlias)
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

The shell is launched via exec ${SHELL:-/bin/sh} without the login flag (-l). This means the shell doesn't source login initialization files (such as /etc/profile, ~/.profile, or ~/.bash_profile). As a result, environment variables such as PATH and other shell customizations set in those files may not be available, which could break user tools and workflows. The original ssh alias command would spawn a login shell by default. To preserve login shell behavior, use exec -l ${SHELL:-/bin/sh} (POSIX) or exec ${SHELL:-/bin/sh} -l.

Suggested change
cmd = fmt.Sprintf("%s && ssh -t %s 'DIR=$(readlink -f /proc/1/cwd 2>/dev/null || pwd); cd \"$DIR\" || echo \"Warning: Could not access container directory\" >&2; exec ${SHELL:-/bin/sh}'", sshAgentEval, sshAlias)
cmd = fmt.Sprintf("%s && ssh -t %s 'DIR=$(readlink -f /proc/1/cwd 2>/dev/null || pwd); cd \"$DIR\" || echo \"Warning: Could not access container directory\" >&2; exec -l ${SHELL:-/bin/sh}'", sshAgentEval, sshAlias)

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants