fix(shellenv): preserve literal newlines in exported env values (ambiguous redirect)

[mikeland73]

Summary

Fixes #2814.

eval "$(devbox global shellenv)" produced the following error on every new prompt for users whose shell sets a multi-line PROMPT_COMMAND (e.g. via bash-preexec, as the elementary OS terminal does):

bash: 1__bp_interactive_mode: ambiguous redirect

Root cause

exportify (internal/devbox/envvars.go) builds export KEY="value"; statements and escaped newlines inside the value by writing a backslash before the newline:

case '$', '`', '"', '\\', '\n':
    strb.WriteRune('\\')

Inside a bash double-quoted string, a backslash-newline is a line continuation: the shell removes both the backslash and the newline and joins the adjacent lines. So a multi-line value like:

__bp_precmd_invoke_cmd
dbus-send ... >/dev/null 2>&1
__bp_interactive_mode

was emitted as ... 2>&1\<newline>__bp_interactive_mode and collapsed by the shell into ... 2>&1__bp_interactive_mode. The redirect target became 1__bp_interactive_mode, hence the ambiguous redirect error.

Newlines are not special inside double quotes (see the POSIX shell quoting rules already cited in the code) — they are preserved literally. The fix is simply to stop escaping them.

Quick demonstration of the difference:

$ eval 'X="a 2>&1\
__bp"'; printf '[%s]\n' "$X"
[a 2>&1__bp]          # old behavior: newline eaten, lines joined

$ eval 'X="a 2>&1
__bp"'; printf '[%s]\n' "$X"
[a 2>&1
__bp]                # new behavior: newline preserved

Changes

• internal/devbox/envvars.go: drop '\n' from the set of characters exportify escapes, with a comment explaining why newlines must stay literal. The other shell-special characters ($, `, ", \) are still escaped.
• internal/devbox/envvars_test.go: new TestExportify covering simple values, special-character escaping, and the multi-line regression from this issue.

This affects both devbox shellenv output and the env exports written into the generated shellrc; in both contexts the output is sourced as bash, so literal newlines inside double quotes are correct.

How was it tested?

• go test ./internal/devbox/ -run TestExportify -v — passes.
• go build ./internal/devbox/ and go vet ./internal/devbox/ — clean.
• Verified the bash line-continuation behavior directly (see demonstration above).

---

cc @proedie (reporter) — thanks for the detailed write-up and the precise pinpointing of the offending line; that made this a quick fix.

🤖 Generated with Claude Code

---

Generated by Claude Code

[Copilot]

Pull request overview

Fixes a bash quoting bug in devbox global shellenv / generated shellrc exports where multi-line environment values (notably PROMPT_COMMAND) were being mangled due to emitting backslash-newline sequences inside double quotes, which bash treats as a line continuation and removes.

Changes:

• Stop escaping '\n' in exportify so multi-line env values preserve literal newlines inside double quotes.
• Add focused unit tests for exportify, including special-character escaping and the multi-line regression from #2814.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
internal/devbox/envvars.go	Updates bash export escaping rules to preserve literal newlines in double-quoted values, avoiding line-continuation behavior.
internal/devbox/envvars_test.go	Adds TestExportify to cover simple exports, special-character escaping, and the multi-line newline-preservation regression.

---

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

[mikeland73]

@claude please review this PR for correctness, and flag anything that would block merging.

[claude]

Claude finished @mikeland73's task in 2m 56s —— View job

---

Review of PR #2846

• Gather context (diff, related code)
• Understand the request (correctness review, flag merge blockers)
• Review internal/devbox/envvars.go changes
• Review internal/devbox/envvars_test.go changes
• Check for other callers/edge cases affected by the newline behavior change
• Post final review

Verdict: Correct. No blockers.

Root-cause analysis checks out

The diagnosis is accurate: inside a POSIX double-quoted string, \ immediately followed by a newline is a line continuation — both characters are removed and the surrounding lines are joined. This is documented POSIX/bash behavior (and matches the existing comment/link in the code), not an edge case the PR is guessing at. Removing '\n' from the escaped-character set in exportify (internal/devbox/envvars.go:100-106) is the right fix — a literal newline inside double quotes is not special and is preserved verbatim.

I traced the interaction with the other escaped characters to make sure nothing regresses:

• A literal \ immediately preceding a real newline in a value is still escaped first (\\), so the output is \\ + newline — decodes back to a literal backslash followed by a literal newline. No re-introduction of the line-continuation bug.
• $, `, " are still escaped as before; unaffected by this change.

Checked all consumers of exportify, not just the reported bash case

exportify (as opposed to exportifyNushell) is used in three places:

• EnvExports (internal/devbox/devbox.go:399) → backs devbox shellenv, eval'd directly by the user's shell.
• shell.go:341 → feeds .ExportEnv in both shellrc.tmpl (bash/zsh/ksh/posix) and shellrc_fish.tmpl (fish), sourced as a generated shellrc file.

Since the same function is reused for fish, I checked fish's quoting rules too: in fish double-quoted strings, only \\, \$, and \" are recognized escapes — \ followed by a literal newline is not a recognized escape, so previously fish would have inserted a spurious literal backslash before the newline (a different but related corruption). With the fix, the newline is emitted unescaped and is preserved literally in fish as well, same as bash/zsh/ksh. So the fix is correct for every shell that consumes exportify's output, and incidentally improves the fish case too. exportifyNushell is untouched, which is correct since it's a separate code path not affected by this bug.

Both consumers write this output to a file that's sourced, or print it for eval, never embed it in a single-line/JSON context — so multi-line values are safe to emit as-is.

Tests

TestExportify in internal/devbox/envvars_test.go covers a plain value, special-character escaping, and the exact multi-line regression from #2814. I hand-traced the expected Go string literals (particularly the raw-string-concatenation in the "escapes shell-special characters" case) against the escaping logic and they match. TestIsValidEnvName, TestExportifySkipsInvalidNames, and TestExportifyNushellSkipsInvalidNames are pre-existing/unrelated and still look consistent with current behavior.

Note: I wasn't able to actually execute go test/go build in this sandboxed environment (commands required approval that wasn't available), so this is a careful manual trace rather than an executed run. Given the PR description states these commands were run and passed locally, and the logic checks out under manual tracing, I have high confidence in correctness, but a maintainer may want to confirm CI runs green.

Minor, non-blocking observations

• The backtick (`) is still escaped as \` for both bash and fish. This is pre-existing behavior (unchanged by this PR) and is technically slightly wrong for fish (fish doesn't recognize \` as an escape inside double quotes, so it'd insert a literal backslash before an already-inert backtick) — but that's an existing, unrelated quirk, not something this PR introduces or needs to fix.

Nothing here blocks merging.