perf(sandbox-providers): drop nvm sourcing from exec wrappers (#10443)
## Thinking Path > - Paperclip keeps agent work on a controlled execution plane. > - Sandbox exec wrappers run on the hot path for agent commands. > - The current change removes the explicit `nvm.sh` load step from those wrappers. > - The sandbox image already restores PATH through profile startup. > - This pull request keeps profile sourcing where the wrapper still needs it and drops only the `nvm.sh` load step. > - The result is a smaller command path with the same node and agent CLI resolution. ## Linked Issues or Issue Description No public GitHub issue exists for this change. Problem: The sandbox exec wrappers spent extra time sourcing `nvm.sh` before each command. The sandbox image already restores PATH in `/etc/profile.d/00-restore-env.sh`, so that explicit `nvm.sh` work was redundant. Proposed solution: Remove the `nvm.sh` source step from all six wrappers. Keep the profile sourcing that the provider still needs for PATH setup. Alternatives considered: Keep the existing shell setup and accept the launch cost. That keeps the current behavior, but it leaves the hot path slower than needed. Roadmap alignment: This change keeps the sandbox command path small and predictable. It does not change the adapter contract or the node resolution rules. ## What Changed - Removed `nvm.sh` sourcing from all six sandbox exec wrappers. - Kept profile sourcing where the provider still needs it for PATH setup. - Switched Modal to a non-login shell because the script now sources profiles itself. - Updated wrapper tests to assert that built commands do not source `nvm.sh`. ## Verification - Local TypeScript typecheck passed in each changed package. - Focused provider tests passed for Daytona, E2B, Modal, exe-dev, Cloudflare bridge, and adapter-utils. - One Daytona test failure is pre-existing and unrelated to this change. ## Risks - This change alters shell startup for sandbox exec paths. - A provider that depends on implicit shell setup may need a follow-up. - The current tests cover command shape, but they do not cover every runtime shell path. ## Model Used OpenAI Codex, GPT-5, tool use. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used with version and capability details - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal or instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Nicky Leach
GitHub
parent
4c8d92f086
commit
d6e235cbcf
|
|
@ -170,6 +170,49 @@ describe("ssh env-lab fixture", () => {
|
|||
await stopSshEnvLabFixture(statePath);
|
||||
}, SSH_FIXTURE_TEST_TIMEOUT_MS);
|
||||
|
||||
it("builds a remote script that sources login profiles but no nvm", async () => {
|
||||
const target = await buildSshSpawnTarget({
|
||||
spec: {
|
||||
host: "ssh.example.test",
|
||||
port: 22,
|
||||
username: "ssh-user",
|
||||
remoteCwd: "/srv/paperclip/workspace",
|
||||
remoteWorkspacePath: "/srv/paperclip/workspace",
|
||||
privateKey: null,
|
||||
knownHosts: null,
|
||||
strictHostKeyChecking: true,
|
||||
},
|
||||
command: "node",
|
||||
args: ["--version"],
|
||||
env: { FOO: "bar" },
|
||||
});
|
||||
|
||||
// The remote script rides the last ssh argument. The SSH target is an
|
||||
// operator-configured host that can expose `node` only through a login
|
||||
// profile, so the wrapper sources the profiles. It no longer sources
|
||||
// `nvm.sh`; a profile that adds nvm still runs.
|
||||
const remoteScript = String(target.args.at(-1) ?? "");
|
||||
expect(remoteScript).not.toContain("nvm.sh");
|
||||
expect(remoteScript).not.toContain("NVM_DIR");
|
||||
// Source /etc/profile so a host that exposes the PATH through
|
||||
// /etc/profile.d scripts still resolves node and the agent CLI.
|
||||
expect(remoteScript).toContain("/etc/profile");
|
||||
expect(remoteScript).toContain(".profile");
|
||||
expect(remoteScript).toContain(".bash_profile");
|
||||
expect(remoteScript).toContain(".zprofile");
|
||||
// Fall back to .bashrc when no .bash_profile exists, so a host that adds
|
||||
// nvm in .bashrc still resolves node under a non-login SSH command.
|
||||
expect(remoteScript).toContain(".bashrc");
|
||||
// The last ssh argument wraps the script as `sh -c '...'`, so the inner
|
||||
// quotes are escaped. Assert the command still runs: cd, env, and the argv.
|
||||
expect(remoteScript).toContain("cd ");
|
||||
expect(remoteScript).toContain("/srv/paperclip/workspace");
|
||||
expect(remoteScript).toContain("exec env ");
|
||||
expect(remoteScript).toContain("node");
|
||||
expect(remoteScript).toContain("--version");
|
||||
await target.cleanup();
|
||||
});
|
||||
|
||||
it("rejects invalid environment variable keys when constructing SSH spawn targets", async () => {
|
||||
await expect(
|
||||
buildSshSpawnTarget({
|
||||
|
|
|
|||
|
|
@ -1166,14 +1166,22 @@ export async function runSshCommand(
|
|||
}
|
||||
}
|
||||
|
||||
// Mirror buildSshSpawnTarget: source login profiles first, then run
|
||||
// `env KEY=VAL cmd` so user-supplied identity overrides win over anything
|
||||
// a profile re-exports. Without this, a remote profile that resets HOME
|
||||
// / NVM_DIR / etc. would silently undo the explicit env passed in here.
|
||||
// Mirror buildSshSpawnTarget: source the login profiles first, then run
|
||||
// `env KEY=VAL cmd` so user-supplied identity overrides win over anything a
|
||||
// profile re-exports. The SSH target is an operator-configured host, not a
|
||||
// Paperclip sandbox image, so it can expose `node` or an agent CLI only
|
||||
// through a login profile; a non-login SSH command would miss that PATH.
|
||||
// Source `/etc/profile` first so a host that exposes the PATH through
|
||||
// `/etc/profile.d` scripts still resolves node and the agent CLI.
|
||||
// The script no longer sources `nvm.sh`; a profile that adds nvm still runs.
|
||||
// .bash_profile typically sources .bashrc itself; only source .bashrc
|
||||
// directly when no .bash_profile exists, so a host that adds nvm in
|
||||
// .bashrc still resolves node without a double-run of the setup.
|
||||
const envArgs = envEntries.map(([key, value]) => `${key}=${shellQuote(value)}`);
|
||||
const remoteScript = [
|
||||
'if [ -f /etc/profile ]; then . /etc/profile >/dev/null 2>&1 || true; fi',
|
||||
'if [ -f "$HOME/.profile" ]; then . "$HOME/.profile" >/dev/null 2>&1 || true; fi',
|
||||
'if [ -f "$HOME/.bash_profile" ]; then . "$HOME/.bash_profile" >/dev/null 2>&1 || true; fi',
|
||||
'if [ -f "$HOME/.bash_profile" ]; then . "$HOME/.bash_profile" >/dev/null 2>&1 || true; elif [ -f "$HOME/.bashrc" ]; then . "$HOME/.bashrc" >/dev/null 2>&1 || true; fi',
|
||||
'if [ -f "$HOME/.zprofile" ]; then . "$HOME/.zprofile" >/dev/null 2>&1 || true; fi',
|
||||
envArgs.length > 0
|
||||
? `exec env ${envArgs.join(" ")} sh -c ${shellQuote(remoteCommand)}`
|
||||
|
|
@ -1223,12 +1231,22 @@ export async function buildSshSpawnTarget(input: {
|
|||
.filter((entry): entry is [string, string] => typeof entry[1] === "string")
|
||||
.map(([key, value]) => `${key}=${shellQuote(value)}`);
|
||||
const remoteCommandParts = [shellQuote(input.command), ...input.args.map((arg) => shellQuote(arg))].join(" ");
|
||||
// Source the login profiles first, then run `env KEY=VAL cmd` so
|
||||
// user-supplied identity overrides win over anything a profile re-exports.
|
||||
// The SSH target is an operator-configured host, not a Paperclip sandbox
|
||||
// image, so it can expose `node` or an agent CLI only through a login
|
||||
// profile; a non-login SSH command would miss that PATH. Source
|
||||
// `/etc/profile` first so a host that exposes the PATH through
|
||||
// `/etc/profile.d` scripts still resolves node and the agent CLI. The script
|
||||
// no longer sources `nvm.sh`; a profile that adds nvm still runs.
|
||||
// .bash_profile typically sources .bashrc itself; only source .bashrc
|
||||
// directly when no .bash_profile exists, so a host that adds nvm in
|
||||
// .bashrc still resolves node without a double-run of the setup.
|
||||
const remoteScript = [
|
||||
'if [ -f /etc/profile ]; then . /etc/profile >/dev/null 2>&1 || true; fi',
|
||||
'if [ -f "$HOME/.profile" ]; then . "$HOME/.profile" >/dev/null 2>&1 || true; fi',
|
||||
'if [ -f "$HOME/.bash_profile" ]; then . "$HOME/.bash_profile" >/dev/null 2>&1 || true; fi',
|
||||
'if [ -f "$HOME/.bash_profile" ]; then . "$HOME/.bash_profile" >/dev/null 2>&1 || true; elif [ -f "$HOME/.bashrc" ]; then . "$HOME/.bashrc" >/dev/null 2>&1 || true; fi',
|
||||
'if [ -f "$HOME/.zprofile" ]; then . "$HOME/.zprofile" >/dev/null 2>&1 || true; fi',
|
||||
'export NVM_DIR="${NVM_DIR:-$HOME/.nvm}"',
|
||||
'[ -s "$NVM_DIR/nvm.sh" ] && . "$NVM_DIR/nvm.sh" >/dev/null 2>&1 || true',
|
||||
`cd ${shellQuote(input.spec.remoteCwd)}`,
|
||||
envArgs.length > 0
|
||||
? `exec env ${envArgs.join(" ")} ${remoteCommandParts}`
|
||||
|
|
|
|||
|
|
@ -0,0 +1,70 @@
|
|||
# Sandbox Runtime Requirements
|
||||
|
||||
This document states the sandbox environment as a contract. The sandbox owner
|
||||
must meet this contract. The Paperclip runtime does not build the environment at
|
||||
exec time. The environment is a requirement, not a build step.
|
||||
|
||||
This document states requirements. It does not state build steps.
|
||||
|
||||
## Required on PATH
|
||||
|
||||
- `node` must be installed and on the PATH.
|
||||
- Each agent CLI that the run uses must be installed and on the PATH. The set of
|
||||
agent CLIs includes `claude`, `codex`, `gemini`, and similar CLIs.
|
||||
- The owner installs only the CLIs that the run uses. The owner does not need to
|
||||
install a CLI that no run uses.
|
||||
|
||||
## Runtime dependencies
|
||||
|
||||
The sandbox execution and synchronization paths need more than `node` and the
|
||||
agent CLIs. The owner must also supply these:
|
||||
|
||||
- A POSIX shell as `sh`, normally `/bin/sh`. The runtime runs each command with
|
||||
`sh -c <script>`. The runtime uses `bash` only when the adapter sets the shell
|
||||
to `bash`.
|
||||
- `tar`. The synchronization path extracts and creates archives with `tar`. A
|
||||
sandbox without `tar` cannot receive or return workspace files.
|
||||
- A writable workspace directory. The runtime extracts the workspace archive
|
||||
into this directory.
|
||||
- A writable home directory. The agent CLIs write state and credentials under
|
||||
the home directory.
|
||||
- A writable cache directory and a writable temporary directory. The runtime and
|
||||
the agent CLIs write intermediate files to these locations.
|
||||
|
||||
## Detection contract
|
||||
|
||||
Paperclip probes each CLI before launch. Paperclip uses the same detection
|
||||
pattern that the runtime Dockerfiles use:
|
||||
|
||||
```bash
|
||||
command -v <cmd> || exit 1
|
||||
```
|
||||
|
||||
Paperclip probes each CLI with `command -v <cmd>`. Paperclip fails loudly when
|
||||
the CLI is absent and no install command is configured for the CLI.
|
||||
|
||||
## Optional CLI installation
|
||||
|
||||
An adapter can configure an install command for a CLI. When an install command
|
||||
is configured, the runtime obeys this flow:
|
||||
|
||||
1. The runtime probes the CLI with `command -v <cmd>`.
|
||||
2. If the CLI is already on the PATH, the runtime skips the install.
|
||||
3. If the CLI is absent, the runtime runs the configured install command one
|
||||
time.
|
||||
4. A failed install is not fatal. The runtime writes a log line and continues.
|
||||
The launch-time probe still reports a missing CLI and fails loudly.
|
||||
|
||||
An owner who relies on a configured install command must also supply the network
|
||||
access, the filesystem write access, and the package tooling that the install
|
||||
command needs. When no install command is configured, the runtime does not
|
||||
install the CLI. The owner must supply the CLI on the PATH.
|
||||
|
||||
## Firm rule
|
||||
|
||||
- The Paperclip runtime never modifies the login profile. The runtime never
|
||||
writes a profile file. The runtime never writes an rc file.
|
||||
- The Paperclip runtime never sources `nvm` on the exec path.
|
||||
- The sandbox owner supplies a ready PATH. The PATH must resolve `node` and each
|
||||
used agent CLI without any action from the runtime, except for a configured
|
||||
install command.
|
||||
|
|
@ -38,6 +38,9 @@ describe("bridge exec", () => {
|
|||
expect(optionsArg).not.toHaveProperty("args");
|
||||
expect(optionsArg).not.toHaveProperty("stdin");
|
||||
expect(commandArg).toContain('. /etc/profile');
|
||||
// The wrapper sources no `nvm.sh`; the sandbox image supplies node on PATH.
|
||||
expect(commandArg).not.toContain("nvm.sh");
|
||||
expect(commandArg).not.toContain("NVM_DIR");
|
||||
expect(commandArg).toContain("cd ");
|
||||
expect(commandArg).toContain("/workspace/paperclip");
|
||||
expect(commandArg).toContain("PAPERCLIP_TEST_FLAG");
|
||||
|
|
|
|||
|
|
@ -45,13 +45,14 @@ export function buildLoginShellScript(input: {
|
|||
.map(([key, value]) => `${key}=${shellQuote(value)}`);
|
||||
const commandParts = [shellQuote(input.command), ...input.args.map(shellQuote)].join(" ");
|
||||
const stdinRedirect = input.stdinFile ? ` < ${shellQuote(input.stdinFile)}` : "";
|
||||
// Source the common login profiles before exec so the command runs with the
|
||||
// interactive-shell PATH. The wrapper sources no `nvm.sh`; the sandbox image
|
||||
// supplies node on the PATH.
|
||||
const lines = [
|
||||
'if [ -f /etc/profile ]; then . /etc/profile >/dev/null 2>&1 || true; fi',
|
||||
'if [ -f "$HOME/.profile" ]; then . "$HOME/.profile" >/dev/null 2>&1 || true; fi',
|
||||
'if [ -f "$HOME/.bash_profile" ]; then . "$HOME/.bash_profile" >/dev/null 2>&1 || true; elif [ -f "$HOME/.bashrc" ]; then . "$HOME/.bashrc" >/dev/null 2>&1 || true; fi',
|
||||
'if [ -f "$HOME/.zprofile" ]; then . "$HOME/.zprofile" >/dev/null 2>&1 || true; fi',
|
||||
'export NVM_DIR="${NVM_DIR:-$HOME/.nvm}"',
|
||||
'[ -s "$NVM_DIR/nvm.sh" ] && . "$NVM_DIR/nvm.sh" >/dev/null 2>&1 || true',
|
||||
];
|
||||
if (input.cwd) {
|
||||
lines.push(`cd ${shellQuote(input.cwd)}`);
|
||||
|
|
|
|||
|
|
@ -1101,11 +1101,12 @@ describe("Daytona sandbox provider plugin", () => {
|
|||
const [command, cwdArg, envArg, timeoutArg] = sandbox.process.executeCommand.mock.calls[0] as [string, unknown, unknown, number];
|
||||
expect(command).toMatch(/\/etc\/profile/);
|
||||
expect(command).toMatch(/"\$HOME\/\.profile"/);
|
||||
expect(command).toMatch(/cd '\/workspace'/);
|
||||
expect(command).not.toMatch(/nvm\.sh/);
|
||||
expect(command).toMatch(/&& cd '\/workspace'/);
|
||||
expect(command).toMatch(/&& env GIT_TERMINAL_PROMPT='0' GCM_INTERACTIVE='Never' GIT_ASKPASS='echo' SSH_ASKPASS='echo' SSH_ASKPASS_REQUIRE='force' FOO='bar' 'printf' 'hello'$/);
|
||||
expect(command).not.toMatch(/(?:^|&& )exec /);
|
||||
// cwd/env are baked into the login-shell command itself; we pass undefined
|
||||
// to the SDK so it doesn't run the cd before profile sourcing.
|
||||
// cwd/env are baked into the command itself; we pass undefined to the SDK
|
||||
// so its own cwd argument does not run before the caller env is applied.
|
||||
expect(cwdArg).toBeUndefined();
|
||||
expect(envArg).toBeUndefined();
|
||||
expect(timeoutArg).toBe(1);
|
||||
|
|
@ -1182,7 +1183,8 @@ describe("Daytona sandbox provider plugin", () => {
|
|||
);
|
||||
const [command] = sandbox.process.executeCommand.mock.calls[0] as [string];
|
||||
expect(command).toMatch(/\/etc\/profile/);
|
||||
expect(command).toMatch(/cd '\/workspace'/);
|
||||
expect(command).not.toMatch(/nvm\.sh/);
|
||||
expect(command).toMatch(/&& cd '\/workspace'/);
|
||||
expect(command).toMatch(/env .* 'cat' < '\/tmp\/paperclip-stdin-/);
|
||||
expect(command).not.toMatch(/(?:^|&& )exec /);
|
||||
expect(sandbox.fs.deleteFile).toHaveBeenCalledWith(expect.stringMatching(/^\/tmp\/paperclip-stdin-/));
|
||||
|
|
@ -1307,37 +1309,12 @@ describe("Daytona sandbox provider plugin", () => {
|
|||
expect(result?.stderr).toMatch(/unreachable|credentials/i);
|
||||
});
|
||||
|
||||
// ─── No-profile fast path (A2) ─────────────────────────────────────────────
|
||||
// The opt-in `noProfile` flag sheds the ~600 ms login-shell profile/nvm
|
||||
// sourcing for command classes whose binary resolves on the default PATH
|
||||
// (file-sync `tar`/`base64`/`mkdir`/`mv`), while every other exec surface
|
||||
// (env prefix, cwd, quoting, stdin, durationMs) is preserved byte-for-byte.
|
||||
it("test_no_profile_fast_path_omits_profile_sourcing", async () => {
|
||||
process.env.DAYTONA_API_KEY = "host-key";
|
||||
const sandbox = createMockSandbox();
|
||||
mockGet.mockResolvedValue(sandbox);
|
||||
|
||||
await plugin.definition.onEnvironmentExecute?.({
|
||||
driverKey: "daytona",
|
||||
companyId: "company-1",
|
||||
environmentId: "env-1",
|
||||
config: { timeoutMs: 300000, reuseLease: false },
|
||||
lease: { providerLeaseId: "sandbox-123", metadata: {} },
|
||||
command: "tar",
|
||||
args: ["-xf", "/workspace/upload.tar", "-C", "/workspace"],
|
||||
cwd: "/workspace",
|
||||
noProfile: true,
|
||||
timeoutMs: 1000,
|
||||
});
|
||||
|
||||
const [command] = sandbox.process.executeCommand.mock.calls[0] as [string];
|
||||
expect(command).not.toMatch(/\/etc\/profile/);
|
||||
expect(command).not.toMatch(/nvm\.sh/);
|
||||
expect(command).not.toMatch(/NVM_DIR/);
|
||||
expect(command).not.toMatch(/\.bash_profile/);
|
||||
});
|
||||
|
||||
it("test_no_profile_fast_path_preserves_env_cwd_and_duration", async () => {
|
||||
// ─── Exec command shape ────────────────────────────────────────────────────
|
||||
// The wrapper sources the login profiles so `node` resolves on the reference
|
||||
// image, then runs the command. It no longer sources `nvm.sh`, while every
|
||||
// other exec surface (env prefix, cwd, quoting, stdin, durationMs) stays
|
||||
// intact.
|
||||
it("test_exec_command_preserves_env_cwd_and_duration", async () => {
|
||||
process.env.DAYTONA_API_KEY = "host-key";
|
||||
const sandbox = createMockSandbox();
|
||||
sandbox.process.executeCommand.mockResolvedValue({
|
||||
|
|
@ -1357,29 +1334,30 @@ describe("Daytona sandbox provider plugin", () => {
|
|||
args: ["-d"],
|
||||
cwd: "/workspace",
|
||||
env: { FOO: "bar" },
|
||||
noProfile: true,
|
||||
timeoutMs: 1000,
|
||||
});
|
||||
|
||||
const [command] = sandbox.process.executeCommand.mock.calls[0] as [string];
|
||||
// The full exec surface is preserved on the fast path — only profile sourcing
|
||||
// is dropped. The command must still start with the `cd` (no profile lines
|
||||
// ahead of it) and carry the env prefix and noninteractive git defaults.
|
||||
expect(command).toMatch(/^cd '\/workspace' && env /);
|
||||
// The command sources the login profiles first, then runs the `cd` and the
|
||||
// env prefix with the noninteractive git defaults.
|
||||
expect(command).toMatch(/^if \[ -f \/etc\/profile \]/);
|
||||
expect(command).toMatch(/&& cd '\/workspace' && env /);
|
||||
expect(command).toMatch(/GIT_TERMINAL_PROMPT='0'/);
|
||||
expect(command).toMatch(/FOO='bar' 'base64' '-d'$/);
|
||||
expect(command).not.toMatch(/\/etc\/profile/);
|
||||
// durationMs attribution is unchanged on the fast path.
|
||||
expect(command).toMatch(/\/etc\/profile/);
|
||||
expect(command).not.toMatch(/nvm\.sh/);
|
||||
// durationMs attribution stays intact.
|
||||
expect(typeof (result!.metadata as Record<string, unknown>)?.durationMs).toBe("number");
|
||||
});
|
||||
|
||||
it("test_default_path_still_sources_profile", async () => {
|
||||
it("test_exec_command_sources_profile_without_nvm", async () => {
|
||||
process.env.DAYTONA_API_KEY = "host-key";
|
||||
const sandbox = createMockSandbox();
|
||||
mockGet.mockResolvedValue(sandbox);
|
||||
|
||||
// No `noProfile` flag: the fail-safe default must still source the login
|
||||
// profile so node-launching execs resolve their nvm/profile PATH.
|
||||
// A node-launching exec resolves `node` through the login profiles, which
|
||||
// Daytona's non-login `executeCommand` shell does not source on its own. The
|
||||
// wrapper sources the profiles but no longer sources `nvm.sh`.
|
||||
await plugin.definition.onEnvironmentExecute?.({
|
||||
driverKey: "daytona",
|
||||
companyId: "company-1",
|
||||
|
|
@ -1394,7 +1372,9 @@ describe("Daytona sandbox provider plugin", () => {
|
|||
|
||||
const [command] = sandbox.process.executeCommand.mock.calls[0] as [string];
|
||||
expect(command).toMatch(/\/etc\/profile/);
|
||||
expect(command).toMatch(/nvm\.sh/);
|
||||
expect(command).toMatch(/"\$HOME\/\.profile"/);
|
||||
expect(command).not.toMatch(/nvm\.sh/);
|
||||
expect(command).not.toMatch(/NVM_DIR/);
|
||||
});
|
||||
|
||||
// ─── Per-lease started-sandbox handle cache ────────────────────────────────
|
||||
|
|
|
|||
|
|
@ -655,18 +655,20 @@ function isGitNetworkCommand(command: string, args: string[]): boolean {
|
|||
return false;
|
||||
}
|
||||
|
||||
// Mirror the E2B sandbox executor: source common login profiles (and nvm)
|
||||
// before running the command so Daytona one-shot calls see the same PATH an
|
||||
// interactive shell would. Without this, adapter probes can fail to resolve
|
||||
// CLIs that are installed via profile-driven PATH mutations inside the
|
||||
// sandbox image.
|
||||
// Build the one-shot exec command. Daytona's `executeCommand` runs the script
|
||||
// in a non-login shell, so it does not source `/etc/profile` on its own. The
|
||||
// Daytona reference image puts `node`, `claude`, and the other CLIs on the PATH
|
||||
// through `/etc/profile.d/00-restore-env.sh`, which only `/etc/profile` sources.
|
||||
// So the wrapper sources the login profiles itself; a non-login shell is then
|
||||
// enough to resolve the CLIs. The wrapper no longer sources `nvm.sh`; the
|
||||
// sandbox image supplies `node` on the PATH. See the sandbox runtime
|
||||
// requirements document.
|
||||
function buildLoginShellScript(input: {
|
||||
command: string;
|
||||
args: string[];
|
||||
cwd?: string;
|
||||
env?: Record<string, string>;
|
||||
stdinPath?: string;
|
||||
noProfile?: boolean;
|
||||
}): string {
|
||||
const callerEnv = input.env ?? {};
|
||||
for (const key of Object.keys(callerEnv)) {
|
||||
|
|
@ -685,25 +687,17 @@ function buildLoginShellScript(input: {
|
|||
: commandParts;
|
||||
// Each `executeCommand` call runs in its own shell, so we don't `exec`-
|
||||
// replace it; running the command as the last `&&`-chained line is enough to
|
||||
// surface the right exit code. Env is interpolated after profile sourcing so
|
||||
// the caller's env wins over any defaults the profile exports.
|
||||
// surface the right exit code.
|
||||
const finalLine = envArgs.length > 0
|
||||
? `env ${envArgs.join(" ")} ${redirectedCommand}`
|
||||
: redirectedCommand;
|
||||
const profileSourcingLines = input.noProfile === true
|
||||
? []
|
||||
: [
|
||||
'if [ -f /etc/profile ]; then . /etc/profile >/dev/null 2>&1 || true; fi',
|
||||
'if [ -f "$HOME/.profile" ]; then . "$HOME/.profile" >/dev/null 2>&1 || true; fi',
|
||||
// .bash_profile typically sources .bashrc itself; only source .bashrc
|
||||
// directly when no .bash_profile exists to avoid double-running setup.
|
||||
'if [ -f "$HOME/.bash_profile" ]; then . "$HOME/.bash_profile" >/dev/null 2>&1 || true; elif [ -f "$HOME/.bashrc" ]; then . "$HOME/.bashrc" >/dev/null 2>&1 || true; fi',
|
||||
'if [ -f "$HOME/.zprofile" ]; then . "$HOME/.zprofile" >/dev/null 2>&1 || true; fi',
|
||||
'export NVM_DIR="${NVM_DIR:-$HOME/.nvm}"',
|
||||
'[ -s "$NVM_DIR/nvm.sh" ] && . "$NVM_DIR/nvm.sh" >/dev/null 2>&1 || true',
|
||||
];
|
||||
const lines = [
|
||||
...profileSourcingLines,
|
||||
'if [ -f /etc/profile ]; then . /etc/profile >/dev/null 2>&1 || true; fi',
|
||||
'if [ -f "$HOME/.profile" ]; then . "$HOME/.profile" >/dev/null 2>&1 || true; fi',
|
||||
// .bash_profile typically sources .bashrc itself; only source .bashrc
|
||||
// directly when no .bash_profile exists to avoid double-running setup.
|
||||
'if [ -f "$HOME/.bash_profile" ]; then . "$HOME/.bash_profile" >/dev/null 2>&1 || true; elif [ -f "$HOME/.bashrc" ]; then . "$HOME/.bashrc" >/dev/null 2>&1 || true; fi',
|
||||
'if [ -f "$HOME/.zprofile" ]; then . "$HOME/.zprofile" >/dev/null 2>&1 || true; fi',
|
||||
];
|
||||
if (input.cwd) {
|
||||
lines.push(`cd ${shellQuote(input.cwd)}`);
|
||||
|
|
@ -1158,16 +1152,15 @@ async function executeOneShot(
|
|||
cwd: params.cwd,
|
||||
env: params.env,
|
||||
stdinPath: stdinPath ?? undefined,
|
||||
noProfile: params.noProfile === true,
|
||||
});
|
||||
|
||||
// Pass cwd undefined: `buildLoginShellScript` already injects `cd` after
|
||||
// profile sourcing when params.cwd is set, and the Daytona executor's own
|
||||
// cwd argument runs before our login-shell init, which is the wrong order
|
||||
// (env from .bashrc would override caller env).
|
||||
// Time only the `executeCommand` REST round-trip (Open Q1) — the ~600ms
|
||||
// login-shell wrapper — so the caller can attribute a step's exec time to
|
||||
// the provider boundary via the free-form `metadata.durationMs`.
|
||||
// Pass cwd undefined: `buildLoginShellScript` already injects the `cd` after
|
||||
// it sources the login profiles, when params.cwd is set. The Daytona
|
||||
// executor's own cwd argument runs before that profile sourcing, which is
|
||||
// the wrong order (a profile could reset the caller env).
|
||||
// Time only the `executeCommand` REST round-trip so the caller can
|
||||
// attribute a step's exec time to the provider boundary through the
|
||||
// free-form `metadata.durationMs`.
|
||||
execStart = timingNow();
|
||||
const result = await sandbox.process.executeCommand(command, undefined, undefined, timeoutSeconds);
|
||||
const durationMs = timingNow() - execStart;
|
||||
|
|
|
|||
|
|
@ -364,6 +364,9 @@ describe("E2B sandbox provider plugin", () => {
|
|||
expect(fgCall).toBeDefined();
|
||||
if (!fgCall) throw new Error("fgCall not found");
|
||||
expect(fgCall[0]).toMatch(/\.profile/);
|
||||
// The wrapper sources no `nvm.sh`; the sandbox image supplies node on PATH.
|
||||
expect(fgCall[0]).not.toMatch(/nvm\.sh/);
|
||||
expect(fgCall[0]).not.toMatch(/NVM_DIR/);
|
||||
expect(fgCall[0]).toMatch(/exec env FOO='bar' 'printf' 'hello'$/);
|
||||
expect(fgCall[1]).toEqual(expect.objectContaining({ cwd: "/workspace", timeoutMs: 1000 }));
|
||||
expect(fgCall[1]).not.toHaveProperty("envs");
|
||||
|
|
|
|||
|
|
@ -152,13 +152,13 @@ function isValidShellEnvKey(value: string) {
|
|||
return /^[A-Za-z_][A-Za-z0-9_]*$/.test(value);
|
||||
}
|
||||
|
||||
// Mirror SSH's buildSshSpawnTarget: source the user's login profiles (and nvm)
|
||||
// before exec so commands run with the same PATH the user sees in an
|
||||
// interactive shell. e2b's `sandbox.commands.run` otherwise spawns a
|
||||
// non-login, non-interactive shell whose PATH does not include npm-globals,
|
||||
// nvm shims, or anything else the template installs via .profile/.bashrc —
|
||||
// which makes the hello probe fail with `exec: <cli>: not found` even when
|
||||
// the binary is on disk.
|
||||
// Source the user's login profiles before exec so commands run with the same
|
||||
// PATH the user sees in an interactive shell. e2b's `sandbox.commands.run`
|
||||
// otherwise spawns a non-login, non-interactive shell whose PATH does not
|
||||
// include npm-globals or anything else the template installs via
|
||||
// .profile/.bashrc — which makes the hello probe fail with
|
||||
// `exec: <cli>: not found` even when the binary is on disk. The wrapper no
|
||||
// longer sources `nvm.sh`; the sandbox image supplies `node` on the PATH.
|
||||
function buildLoginShellScript(input: {
|
||||
command: string;
|
||||
args: string[];
|
||||
|
|
@ -186,8 +186,6 @@ function buildLoginShellScript(input: {
|
|||
// .bash_profile -> .bashrc.
|
||||
'if [ -f "$HOME/.bash_profile" ]; then . "$HOME/.bash_profile" >/dev/null 2>&1 || true; elif [ -f "$HOME/.bashrc" ]; then . "$HOME/.bashrc" >/dev/null 2>&1 || true; fi',
|
||||
'if [ -f "$HOME/.zprofile" ]; then . "$HOME/.zprofile" >/dev/null 2>&1 || true; fi',
|
||||
'export NVM_DIR="${NVM_DIR:-$HOME/.nvm}"',
|
||||
'[ -s "$NVM_DIR/nvm.sh" ] && . "$NVM_DIR/nvm.sh" >/dev/null 2>&1 || true',
|
||||
execLine,
|
||||
].join(" && ");
|
||||
}
|
||||
|
|
|
|||
|
|
@ -576,8 +576,12 @@ describe("exe.dev sandbox provider plugin", () => {
|
|||
|
||||
expect(spawnMock).toHaveBeenCalledTimes(1);
|
||||
expect(spawnMock.mock.calls[0]?.[0]).toBe("ssh");
|
||||
expect(String(spawnMock.mock.calls[0]?.[1]?.at(-1) ?? "")).toContain("/workspace");
|
||||
expect(String(spawnMock.mock.calls[0]?.[1]?.at(-1) ?? "")).toContain("FOO='");
|
||||
const remoteScript = String(spawnMock.mock.calls[0]?.[1]?.at(-1) ?? "");
|
||||
expect(remoteScript).toContain("/workspace");
|
||||
expect(remoteScript).toContain("FOO='");
|
||||
// The wrapper sources no `nvm.sh`; the sandbox image supplies node on PATH.
|
||||
expect(remoteScript).not.toContain("nvm.sh");
|
||||
expect(remoteScript).not.toContain("NVM_DIR");
|
||||
const child = spawnMock.mock.results[0]?.value as MockChildProcess;
|
||||
expect(child.stdin.written).toBe("input-body");
|
||||
expect(child.stdin.ended).toBe(true);
|
||||
|
|
|
|||
|
|
@ -537,13 +537,14 @@ function buildLoginShellScript(input: {
|
|||
const finalLine = envArgs.length > 0
|
||||
? `exec env ${envArgs.join(" ")} ${commandParts}`
|
||||
: `exec ${commandParts}`;
|
||||
// Source the common login profiles before exec so the command runs with the
|
||||
// interactive-shell PATH. The wrapper sources no `nvm.sh`; the sandbox image
|
||||
// supplies node on the PATH.
|
||||
const lines = [
|
||||
'if [ -f /etc/profile ]; then . /etc/profile >/dev/null 2>&1 || true; fi',
|
||||
'if [ -f "$HOME/.profile" ]; then . "$HOME/.profile" >/dev/null 2>&1 || true; fi',
|
||||
'if [ -f "$HOME/.bash_profile" ]; then . "$HOME/.bash_profile" >/dev/null 2>&1 || true; elif [ -f "$HOME/.bashrc" ]; then . "$HOME/.bashrc" >/dev/null 2>&1 || true; fi',
|
||||
'if [ -f "$HOME/.zprofile" ]; then . "$HOME/.zprofile" >/dev/null 2>&1 || true; fi',
|
||||
'export NVM_DIR="${NVM_DIR:-$HOME/.nvm}"',
|
||||
'[ -s "$NVM_DIR/nvm.sh" ] && . "$NVM_DIR/nvm.sh" >/dev/null 2>&1 || true',
|
||||
];
|
||||
if (input.cwd) {
|
||||
lines.push(`cd ${shellQuote(input.cwd)}`);
|
||||
|
|
|
|||
|
|
@ -541,7 +541,7 @@ describe("Modal sandbox provider plugin", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("executes commands with a login-shell wrapper that injects env after profile sourcing", async () => {
|
||||
it("executes commands with a non-login-shell wrapper that injects env after profile sourcing", async () => {
|
||||
const sandbox = createFakeSandbox({
|
||||
execImpl: async (argv: string[]) =>
|
||||
makeFakeProcess({
|
||||
|
|
@ -568,9 +568,12 @@ describe("Modal sandbox provider plugin", () => {
|
|||
expect(sandbox.execCalls).toHaveLength(1);
|
||||
const call = sandbox.execCalls[0]!;
|
||||
expect(call.argv[0]).toBe("sh");
|
||||
expect(call.argv[1]).toBe("-lc");
|
||||
expect(call.argv[1]).toBe("-c");
|
||||
const script = call.argv[2]!;
|
||||
expect(script).toMatch(/\/etc\/profile/);
|
||||
// The wrapper sources no `nvm.sh`; the sandbox image supplies node on PATH.
|
||||
expect(script).not.toMatch(/nvm\.sh/);
|
||||
expect(script).not.toMatch(/NVM_DIR/);
|
||||
expect(script).toMatch(/cd '\/srv\/work'/);
|
||||
expect(script).toMatch(/&& exec env FOO='bar' 'printf' 'hello'$/);
|
||||
expect(call.params).toMatchObject({
|
||||
|
|
|
|||
|
|
@ -213,11 +213,13 @@ function isValidShellEnvKey(value: string): boolean {
|
|||
|
||||
// Modal's `sandbox.exec` takes an argv array and bypasses the shell entirely,
|
||||
// so adapter probes that rely on PATH mutations from /etc/profile or ~/.bashrc
|
||||
// do not work without an explicit login shell. Mirroring the Daytona / E2B
|
||||
// providers, wrap the user command in a `sh -lc` script that sources common
|
||||
// login profiles plus nvm before invoking it. Env is set after profile sourcing
|
||||
// so caller env wins; stdin is staged to a temp file and shell-redirected so
|
||||
// fast-failing commands do not race a streaming stdin writer.
|
||||
// do not work without an explicit profile source. Wrap the user command in a
|
||||
// `sh -c` script that sources the common login profiles before it runs. The
|
||||
// script sources no `nvm.sh`; the sandbox image supplies node on the PATH. The
|
||||
// script sources the profiles itself, so a non-login shell (`sh -c`) is enough.
|
||||
// Env is set after profile sourcing so caller env wins; stdin is staged to a
|
||||
// temp file and shell-redirected so fast-failing commands do not race a
|
||||
// streaming stdin writer.
|
||||
function buildLoginShellScript(input: {
|
||||
command: string;
|
||||
args: string[];
|
||||
|
|
@ -244,8 +246,6 @@ function buildLoginShellScript(input: {
|
|||
'if [ -f "$HOME/.profile" ]; then . "$HOME/.profile" >/dev/null 2>&1 || true; fi',
|
||||
'if [ -f "$HOME/.bash_profile" ]; then . "$HOME/.bash_profile" >/dev/null 2>&1 || true; elif [ -f "$HOME/.bashrc" ]; then . "$HOME/.bashrc" >/dev/null 2>&1 || true; fi',
|
||||
'if [ -f "$HOME/.zprofile" ]; then . "$HOME/.zprofile" >/dev/null 2>&1 || true; fi',
|
||||
'export NVM_DIR="${NVM_DIR:-$HOME/.nvm}"',
|
||||
'[ -s "$NVM_DIR/nvm.sh" ] && . "$NVM_DIR/nvm.sh" >/dev/null 2>&1 || true',
|
||||
];
|
||||
if (input.cwd) {
|
||||
lines.push(`cd ${shellQuote(input.cwd)}`);
|
||||
|
|
@ -624,7 +624,7 @@ const plugin = definePlugin({
|
|||
env: params.env,
|
||||
stdinPath: stdinPath ?? undefined,
|
||||
});
|
||||
const proc = await sandbox.exec(["sh", "-lc", script], {
|
||||
const proc = await sandbox.exec(["sh", "-c", script], {
|
||||
timeoutMs: callerTimeoutMs,
|
||||
stdout: "pipe",
|
||||
stderr: "pipe",
|
||||
|
|
|
|||
Loading…
Reference in New Issue