Skip to content

fix(desktop): prevent env var leak in persistent terminal subprocess#785

Merged
Kitenite merged 1 commit intomainfrom
fix/terminal-env-leak
Jan 17, 2026
Merged

fix(desktop): prevent env var leak in persistent terminal subprocess#785
Kitenite merged 1 commit intomainfrom
fix/terminal-env-leak

Conversation

@Kitenite
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite commented Jan 17, 2026

Summary

Fixes environment variable leak in persistent terminal sessions that was causing bun dev to fail with NODE_ENV=production.

Change

The subprocess (pty-subprocess.js) was spawned with raw process.env, which includes NODE_ENV and other vars that should be filtered. Now uses the same filtered processEnv for both the subprocess and PTY shell.

-env: { ...process.env, ELECTRON_RUN_AS_NODE: "1" }
+env: { ...processEnv, ELECTRON_RUN_AS_NODE: "1" }

Test Plan

  • Rebuild desktop app
  • Kill existing daemon or restart app
  • Open persistent terminal
  • Verify echo $NODE_ENV returns empty
  • Run bun dev - should work without env validation errors

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 17, 2026

Warning

Rate limit exceeded

@Kitenite has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 6 minutes and 49 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between c42368c and 3e7ea8f.

📒 Files selected for processing (1)
  • apps/desktop/src/main/terminal-host/session.ts
📝 Walkthrough

Walkthrough

Modified PTY spawn environment construction to use only the provided environment (plus TERM) instead of merging with host process.env. Creates sanitized subprocess environment from host process.env. Forces TERM to 'xterm-256color'. Prevents NODE_ENV leakage from parent process.

Changes

Cohort / File(s) Summary
PTY/Session Environment Handling
apps/desktop/src/main/terminal-host/session.ts
Modified environment construction for spawned PTY processes. Changed from merging host process.env with provided env to using only provided env. Introduces sanitized subprocess environment from host process.env. Forces TERM override to 'xterm-256color'. Comments added explaining daemon-managed environment filtering and NODE_ENV leak prevention.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Poem

🐰 A bunny hops through variables with care,
No NODE_ENV shall leak into the air!
Sanitized paths and xterm so bright,
Environment chaos? Not on my watch tonight! 🌟

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: preventing environment variable leakage in terminal subprocess, which is the primary objective of this PR.
Description check ✅ Passed The description provides comprehensive coverage of required sections including clear summary, detailed changes, root cause analysis, and test plan. However, it does not include related issues links or formal type of change selection from the template.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

The subprocess (pty-subprocess.js) was spawned with raw process.env,
which includes NODE_ENV and other vars that should be filtered.
Now uses the same filtered processEnv for both subprocess and PTY shell.
@Kitenite Kitenite force-pushed the fix/terminal-env-leak branch from c42368c to 3e7ea8f Compare January 17, 2026 03:46
@Kitenite Kitenite merged commit f0c44a1 into main Jan 17, 2026
3 of 4 checks passed
@Kitenite Kitenite deleted the fix/terminal-env-leak branch January 17, 2026 03:48
@github-actions
Copy link
Copy Markdown
Contributor

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ⚠️ Neon database branch
  • ⚠️ Electric Fly.io app

Thank you for your contribution! 🎉

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.

1 participant