Repository navigation
fix(home): a repository's .env cannot choose Medulla's home or account - #11
Conversation
A `.env` in the working directory is untrusted input, so it may no longer set the variables that choose Medulla's home or the account inside it; those could otherwise point the process at a planted session, config, or credential store. `apply_dotenv` and `load_dotenv_from_cwd` now return the refused keys so `main` can report them, and the variables still work when set from the invoking shell. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The TUI now prints a warning to stderr when a repository's `.env` tries to set variables that select Medulla's home or account, since those must come from the invoking shell. The comment describing the load order was also updated to reflect that home selectors are never applied. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add an end-to-end test asserting that a repository's .env cannot choose the account Medulla uses: MEDULLA_USER from the workspace file is refused with a warning, logout acts on and reports the invoker's own account, and the planted session is left untouched. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The configuration and environment-variable docs now explain that a repository's `.env` cannot set `MEDULLA_HOME`, `MEDULLA_DEV`, `MEDULLA_USER`, `MEDULLA_CONFIG_PATH`, `HOME`, or `USERPROFILE`, and that Medulla warns when it ignores them. This closes a hole where a checked-out repository could redirect Medulla at a session or credentials it planted, so the docs point developers at shell-level settings such as direnv instead. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Document that MEDULLA_HOME, MEDULLA_DEV, MEDULLA_USER, MEDULLA_CONFIG_PATH, HOME, and USERPROFILE are only read from the environment Medulla is started with, so a .env in the current directory cannot set them. The per-row note on MEDULLA_DEV is replaced by this shared explanation. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Brings in #10. Its logout test assumed a cwd .env could re-select the stored session (and so expected no signed_out); with .env home selectors refused here, that premise no longer holds, so it is superseded by a_workspace_env_file_cannot_select_the_account, which asserts the invoker's own account is signed out and reported and the planted one never is. Co-authored-by: Medulla <medulla@tinyhumans.ai>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (6)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe dotenv loader now refuses six home and account selector variables from cwd ChangesDotenv Home Selector Handling
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~15 minutes Change: Bug fix
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely describes the main change: repository-local .env files can no longer select Medulla's home or account. |
| Docstring Coverage | |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 4 files. (2 skipped: 2… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches 💡 1
📝 Generate docstrings
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks 💡
- Commit to this branch
- Create a new PR
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
I’m a rabbit guarding the den,
No .env can choose it again.
Shell-set selectors may enter,
While refused names go to stderr.
I nibble a carrot, then hop away.
Comment @coderabbitai help to get the list of available commands.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a18529eb3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fn is_home_selector(key: &str) -> bool { | ||
| HOME_SELECTORS.contains(&key) |
There was a problem hiding this comment.
On Windows, environment-variable names are case-insensitive, but this comparison is case-sensitive. A repository can therefore use a spelling such as Home or Medulla_Home; it passes this check and is installed into the process environment, where Windows lookups and inherited child environments treat it as HOME or MEDULLA_HOME. This bypasses the new trust boundary for Windows users, so normalize the key (as the existing environment-secret scrubber does) before comparing it with HOME_SELECTORS.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 1 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Changes requested Review snapshot
Completeness: Complete What changedThe review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below. FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. Findings
Pending checks: E2E (docker + tmux + ${{ matrix.harness }}) Before merge
How this fits togetherflowchart LR
n0["apply_dotenv_never_overrides_existing<br/>changed"]:::changed
n1["main<br/>changed"]:::changed
n2["env"]:::impacted
n3["trim"]:::impacted
n4["fs"]:::impacted
n5["write_active_user_id"]:::impacted
n6["...esss_login_does_not_re_home_a_running_one"]:::impacted
n7["iter"]:::impacted
n0 -->|calls| n2
n0 -->|tests| n2
n1 -->|calls| n7
n2 -->|calls| n7
n3 -->|calls| n7
n6 -->|calls| n2
n6 -->|tests| n2
n6 -->|uses| n4
n6 -->|calls| n5
n6 -->|tests| n5
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: 1 lane(s) blocking, worst finding is high.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0056 · 154,808 in / 8,220 out · 17,627 cached (11%) · gpt-5.6-luna, glm-5.3-flash
critique: $0.0020 · 77,664 in / 4,861 out · 10,819 cached (14%) · gpt-5.6-luna, glm-5.3-flash
security: $0.0033 · 45,519 in / 1,649 out · 3,736 cached (8%) · gpt-5.6-luna
tests: $0.0001 · 7,196 in / 249 out · 1,536 cached (21%) · glm-5.3-flash
description: $0.0001 · 7,412 in / 190 out · 1,408 cached (19%) · glm-5.3-flash
e2e: $0.0001 · 10,513 in / 207 out · 0 cached (0%) · glm-5.3-flash
| ]; | ||
|
|
||
| /// Whether `key` is one a `.env` may not set. | ||
| fn is_home_selector(key: &str) -> bool { |
There was a problem hiding this comment.
Reject home selectors regardless of environment-key casing
On Windows, environment variable names are case-insensitive, but this check is exact-case. A repository .env containing medulla_home, medulla_dev, medulla_user, medulla_config_path, home, or userprofile can therefore be inserted into the process environment and later read through the uppercase selector, allowing the repository to redirect Medulla's home or account despite this protection. Compare keys case-insensitively (or use platform-specific normalization) before applying them.
[RULE] case-insensitive-environment-key ·
1 participant
Why
A
.envin the current directory is loaded at startup, and it belongs to whatever repository Medulla is opened in, so it is untrusted input. Until now it could set the variables that decide where Medulla's home is and which account in it is used. A hostile repository could ship a.env, plus a./.medulla/directory forMEDULLA_DEV, that points Medulla at a planted session, config, or credential store. The command then runs as an account the repository chose. #9 and #10 kept telemetry from naming such an account; this closes the underlying hole for every command.What
home::dotenv::HOME_SELECTORSlists the variables a cwd.envmay no longer set:MEDULLA_HOMEsets the root directly.MEDULLA_DEVmakes the root./.medulla, inside the checkout.HOME/USERPROFILEdecide where the default~/.medullais.MEDULLA_USERpicks the account within the root.MEDULLA_CONFIG_PATHnames a config file that can carry an inline token.load_dotenv_from_cwdand its pure counterpartapply_dotenvrefuse those keys and return the refused names.mainprints one stderr line naming them, for examplemedulla: ignored MEDULLA_USER from ./.env — … set it in your shell instead, so a.envMEDULLA_DEV=1that stops working says why. The line goes to stderr because stdout can be themedulla mcpprotocol stream..envkey behaves as before, and none of these variables changes meaning when it comes from the invoking environment.Behaviour change for developers
The gitbook described
.envas "the usual way to setMEDULLA_DEV=1for local development". That no longer works: set it in the shell (export MEDULLA_DEV=1) or with a shell-level tool such as direnv.configuration.mdandenvironment-variables.mdare updated to say so.Interaction with #10
#10's
logout_never_reports_an_account_a_workspace_env_file_selectedassumed a.envMEDULLA_USERcould re-select the session, so it expected nosigned_outat all. With this change that premise is gone. The merge ofmainreplaces it witha_workspace_env_file_cannot_select_the_account, which asserts the stronger outcome: logout succeeds, warns, signs out and reports the invoker's own account (user-42), never reports the planted one, and leaves the planted session untouched. The telemetry trust checks from #9/#10 stay as a second line of defence.Tests
a_dotenv_cannot_choose_the_home_or_the_account: all six keys are refused and reported in order, ordinary keys still apply, and the root resolves exactly as with no.env.a_workspace_env_file_cannot_select_the_accountruns the real binary. With the refusal disabled it fails.cargo fmt --all -- --checkandcargo clippy --locked --all-targets -- -D warningsare clean.cargo test --lockedpassed in two consecutive full runs (70/70 test binaries) on the branch withmainmerged in.Summary by CodeRabbit
.envfiles can no longer change which Medulla home or account is selected. Medulla warns when these settings are ignored and continues using values from the environment that launched it..envsettings continue to apply as before when they are not already set.