Skip to content

Quote activation exports and paths for space-containing roots#545

Merged
wsutina merged 3 commits intocashapp:masterfrom
robmaceachern:fix/quote-hermit-activate-exports-3kjbge
Feb 18, 2026
Merged

Quote activation exports and paths for space-containing roots#545
wsutina merged 3 commits intocashapp:masterfrom
robmaceachern:fix/quote-hermit-activate-exports-3kjbge

Conversation

@robmaceachern
Copy link
Copy Markdown
Contributor

@robmaceachern robmaceachern commented Feb 12, 2026

Note

Disclaimer: I'm not a go dev but hit this bug today and figured I'd throw up a PR with the assistance of AI. Feel free to mock and/or shame me 😂

Summary

  • quote all path-derived values emitted by Hermit activation scripts so eval is safe when environment roots contain spaces
  • quote command substitutions and path arguments used during activate/deactivate/update flows in POSIX and fish templates
  • add regression unit tests for generated activation scripts with space-containing roots
  • add an integration test that activates an environment located in a directory with spaces

How This Was Discovered

This surfaced while running Hermit from a CodexMonitor-managed git worktree. CodexMonitor stores worktrees under:

~/Library/Application Support/com.dimillian.codexmonitor/worktrees/...

Because Application Support contains a space, source bin/activate-hermit failed when hermit activate emitted unquoted export values. Shell eval split those values, causing activation errors in zsh/bash.

Testing

  • go test ./...
  • go test -tags=integration ./integration -run 'TestIntegration/ActivationWorksWhenPathContainsSpaces' -count=1

This response was drafted with AI assistance.

@robmaceachern robmaceachern marked this pull request as ready for review February 12, 2026 22:17
Comment thread integration/integration_test.go Outdated
{
name: "ActivationWorksWhenPathContainsSpaces",
script: `
mkdir -p "env with spaces"
Copy link
Copy Markdown
Contributor

@wsutina wsutina Feb 16, 2026

Choose a reason for hiding this comment

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

I found that this example passes before the fix. Can we use a more complex path like your unit test "/tmp/Application Support/hermit env"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch! Updated in 023c4ee and codex's explanation on why the original test shape didn't actually reveal the issue.

Details

export accepts both of these argument forms:

  1. NAME=value (assign + export)
  2. NAME (export an existing variable, or create empty one)

So when unquoted output is:

export HERMIT_ENV=/tmp/env with spaces

the shell parses it as 3 arguments:

  1. HERMIT_ENV=/tmp/env
  2. with
  3. spaces

with and spaces match valid shell variable-name syntax ([A-Za-z_][A-Za-z0-9_]*), so export accepts them. No syntax error.

That means:

  • HERMIT_ENV gets the wrong truncated value (/tmp/env)
  • with and spaces are just exported as variables
  • command still exits successfully

But with:

export HERMIT_ENV=/tmp/Application Support/hermit env

one token becomes Support/hermit, which is not a valid variable name (contains /), so export errors. That’s why the Application Support/hermit env shape reliably exposes the bug.

Copy link
Copy Markdown
Contributor

@wsutina wsutina left a comment

Choose a reason for hiding this comment

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

Thank you so much for the fix! I got a few minor comments. Once that's addressed, I will merge and release the PRs :)

Comment thread shell/files/activate.tmpl.fish Outdated
set -gx HERMIT_ENV_OPS $("$HERMIT_ENV/bin/hermit" env --ops)
set -gx ACTIVE_HERMIT "$HERMIT_ENV"
set -gx HERMIT_ENV_OPS "$("$HERMIT_ENV/bin/hermit" env --ops)"
set -gx HERMIT_BIN_CHANGE $(date -r "$HERMIT_ENV/bin" +"%s")
Copy link
Copy Markdown
Contributor

@wsutina wsutina Feb 16, 2026

Choose a reason for hiding this comment

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

For consistency can we add "" to:

Suggested change
set -gx HERMIT_BIN_CHANGE $(date -r "$HERMIT_ENV/bin" +"%s")
set -gx HERMIT_BIN_CHANGE "$(date -r "$HERMIT_ENV/bin" +"%s")"

and

set CURRENT "$(date -r "$HERMIT_ENV/bin" +"%s")"

too?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated in 023c4ee!

@wsutina wsutina force-pushed the fix/quote-hermit-activate-exports-3kjbge branch from 023c4ee to fe806cf Compare February 18, 2026 01:10
@wsutina wsutina merged commit f13c834 into cashapp:master Feb 18, 2026
7 checks passed
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