Skip to content

fix: set XDG_CONFIG_DIRS and XDG_DATA_DIRS defaults in tryGetPamEnvVars#3121

Merged
sawka merged 1 commit intowavetermdev:mainfrom
majiayu000:fix/issue-2970-xdg-config-dirs-snap-default
Mar 26, 2026
Merged

fix: set XDG_CONFIG_DIRS and XDG_DATA_DIRS defaults in tryGetPamEnvVars#3121
sawka merged 1 commit intowavetermdev:mainfrom
majiayu000:fix/issue-2970-xdg-config-dirs-snap-default

Conversation

@majiayu000
Copy link
Copy Markdown
Contributor

Summary

  • Fixes [Bug]: Not Loading the XDG_CONFIG_DIRS Environment Variable #2970: WaveTerm does not inherit XDG_CONFIG_DIRS (and XDG_DATA_DIRS) when snap or other environments strip these variables and PAM env files do not define them
  • In tryGetPamEnvVars(), after the existing XDG_RUNTIME_DIR fallback, adds identical fallback logic for XDG_CONFIG_DIRS (default: /etc/xdg) and XDG_DATA_DIRS (default: /usr/local/share:/usr/share) per the XDG Base Directory Specification
  • No behavior change when these vars are already set by PAM env files

Root Cause

Snap confinement strips several XDG environment variables. tryGetPamEnvVars() already handles XDG_RUNTIME_DIR with a sensible default, but XDG_CONFIG_DIRS and XDG_DATA_DIRS were left unhandled, causing child shells to receive empty/unset values.

Test plan

  • gofmt -l ./pkg/shellexec/ — no output (clean)
  • go build ./... — succeeds
  • On Linux with snap, verify that child shells receive XDG_CONFIG_DIRS=/etc/xdg and XDG_DATA_DIRS=/usr/local/share:/usr/share when the variables are not set by the desktop environment

When snap or other environments strip XDG_CONFIG_DIRS and XDG_DATA_DIRS,
and PAM env files do not define them, child shells receive no value for
these vars. Apply XDG Base Directory spec defaults (/etc/xdg and
/usr/local/share:/usr/share) following the same pattern already used
for XDG_RUNTIME_DIR.

Fixes wavetermdev#2970

Signed-off-by: majiayu000 <1835304752@qq.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 25, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 63a24ce6-fcb6-4df6-91da-f4e252337232

📥 Commits

Reviewing files that changed from the base of the PR and between 0b29c49 and 329e5fe.

📒 Files selected for processing (1)
  • pkg/shellexec/shellexec.go

Walkthrough

The tryGetPamEnvVars() function in pkg/shellexec/shellexec.go was modified to set default values for additional XDG environment variables when they are missing or empty. Specifically, XDG_CONFIG_DIRS is now defaulted to /etc/xdg and XDG_DATA_DIRS is now defaulted to /usr/local/share:/usr/share. Previously, only XDG_RUNTIME_DIR received a default value. The function's overall control flow and return behavior remain unchanged.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: setting XDG_CONFIG_DIRS and XDG_DATA_DIRS defaults in the tryGetPamEnvVars function.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, explaining the fix, root cause, and test plan for the XDG environment variables issue.
Linked Issues check ✅ Passed The PR addresses issue #2970 by implementing fallback defaults for XDG_CONFIG_DIRS and XDG_DATA_DIRS in tryGetPamEnvVars, which resolves the reported problem of missing XDG environment variables.
Out of Scope Changes check ✅ Passed All changes are scoped to the tryGetPamEnvVars function to add XDG variable defaults, directly addressing issue #2970 with no extraneous modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@kilo-code-bot
Copy link
Copy Markdown
Contributor

kilo-code-bot bot commented Mar 25, 2026

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (1 file)
  • pkg/shellexec/shellexec.go - No issues

The fix follows the existing pattern used for XDG_RUNTIME_DIR fallback and uses the correct default values per the XDG Base Directory Specification. The code correctly handles both the case where the key is missing and where it exists but is empty.


Reviewed by minimax-m2.5-20260211 · 119,577 tokens

@sawka
Copy link
Copy Markdown
Member

sawka commented Mar 26, 2026

@majiayu000 thanks again for the submission. looks good!

@sawka sawka merged commit b26eb69 into wavetermdev:main Mar 26, 2026
3 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.

[Bug]: Not Loading the XDG_CONFIG_DIRS Environment Variable

2 participants