#1897 fix: resolve hyde paths without erroring on an unset environment - #1900
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe Lua path module safely resolves missing XDG variables, dynamic configuration loading skips unavailable paths, file probes close handles, and shell tests cover configured, fallback, and unset-variable behavior. ChangesXDG path resolution and configuration loading
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Configs/.local/share/hypr/lua/hyde/path.lua`:
- Around line 64-65: Update is_directory to safely shell-quote the path before
interpolating it into the io.popen test command, preserving correct directory
detection for paths containing apostrophes and preventing injected shell syntax
from executing.
- Around line 51-54: Update the P.runtime initialization in path.lua to convert
an empty XDG_RUNTIME_DIR value to nil while preserving non-empty values and
unset-variable behavior. Add a regression test covering an explicitly empty
XDG_RUNTIME_DIR and verify the unresolved runtime path remains nil.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3caf498c-4548-4d1b-bec8-bf06dcdaa47d
📒 Files selected for processing (4)
CHANGELOG.mdConfigs/.local/share/hypr/lua/dynamic.luaConfigs/.local/share/hypr/lua/hyde/path.luatests/test_paths.sh
| --- @field runtime string|nil Runtime directory. It has no fallback: the spec | ||
| --- requires the session to provide one, and inventing a path would put runtime | ||
| --- state somewhere that outlives the session. | ||
| P.runtime = os.getenv("XDG_RUNTIME_DIR") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/sh
# Statically verify that empty runtime values are normalized and covered.
rg -n -C 2 'P\.runtime|XDG_RUNTIME_DIR' \
Configs/.local/share/hypr/lua/hyde/path.lua tests/test_paths.shRepository: HyDE-Project/HyDE
Length of output: 602
🏁 Script executed:
#!/bin/sh
set -eu
echo "== file excerpt =="
sed -n '1,120p' Configs/.local/share/hyperpr/lua/hyde/path.lua 2>/dev/null || sed -n '1,140p' Configs/.local/share/hypr/lua/hyde/path.lua
echo "== consumers of P.runtime =="
rg -n 'P\.runtime|runtime' Configs/.local/share/hypr/lua/hyde -S || true
rg -n 'P\.runtime|runtime' Configs tests -S || true
echo "== tests around path/env =="
rg -n -C 3 'XDG_RUNTIME_DIR|runtime|path' tests Configs/.local/share/hypr/lua/hyde -S || true
echo "== Lua truthiness probe =="
lua5.1 - <<'LUA' 2>/dev/null || lua - <<'LUA' 2>/dev/null || true
local runtime = ""
print("runtime=" .. runtime)
print("runtime truthy=" .. tostring(runtime and "yes" or "no"))
LUARepository: HyDE-Project/HyDE
Length of output: 29570
🏁 Script executed:
#!/bin/sh
set -eu
echo "== file excerpt =="
sed -n '1,160p' Configs/.local/share/hypr/lua/hyde/path.lua
echo "== consumers of P.runtime =="
rg -n 'P\.runtime|runtime' Configs/.local/share/hypr/lua/hyde -S || true
rg -n 'P\.runtime|runtime' Configs tests -S || true
echo "== Lua truthiness probe =="
lua5.1 - <<'LUA' 2>/dev/null || lua - <<'LUA' 2>/dev/null || true
local runtime = ""
print("runtime=" .. runtime)
print("runtime truthy=" .. tostring(runtime and "yes" or "no"))
LUARepository: HyDE-Project/HyDE
Length of output: 4571
🏁 Script executed:
#!/bin/sh
set -eu
echo "== hyde/runtime-related occurrences across repo =="
rg -n -C 3 'hyde\.path\.runtime|XDG_RUNTIME_DIR|P\.runtime|runtime' Configs tests -S \
-g '!test_screenshot_wrapper.sh' || true
echo "== path module direct runtime tests =="
python3 - <<'PY'
from pathlib import Path
p = Path("tests/test_paths.sh")
text = p.read_text() if p.exists() else ""
for line in text.splitlines():
if "XDG_RUNTIME_DIR" in line or 'P.runtime' in line:
print(line)
PY
echo "== Lua empty-string truthiness probe =="
lua - <<'LUA' 2>/dev/null || true
local runtime = ""
print("runtime=" .. runtime)
print("runtime truthy=" .. tostring(runtime and "yes" or "no"))
LUARepository: HyDE-Project/HyDE
Length of output: 313
🏁 Script executed:
#!/bin/sh
set -eu
echo "== hyde/runtime-related occurrences across repo =="
rg -n -C 3 'hyde\.path\.runtime|XDG_RUNTIME_DIR|P\.runtime|runtime' Configs tests -S \
-g '!test_screenshot_wrapper.sh' || true
echo "== path module direct runtime tests =="
python3 <<'PY'
from pathlib import Path
p = Path("tests/test_paths.sh")
text = p.read_text() if p.exists() else ""
for line in text.splitlines():
if "XDG_RUNTIME_DIR" in line or 'P.runtime' in line:
print(line)
PY
echo "== Lua empty-string truthiness probe =="
lua - <<'LUA' 2>/dev/null || true
local runtime = ""
print("runtime=" .. runtime)
print("runtime truthy=" .. tostring(runtime and "yes" or "no"))
LUARepository: HyDE-Project/HyDE
Length of output: 313
Normalize an empty runtime directory to nil.
P.runtime = os.getenv("XDG_RUNTIME_DIR") leaves an empty XDG_RUNTIME_DIR as "", a truthy Lua value that violates the module’s “unresolved paths are nil” contract for runtime state. Treat an empty runtime as unset, and add a regression case for this input.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Configs/.local/share/hypr/lua/hyde/path.lua` around lines 51 - 54, Update the
P.runtime initialization in path.lua to convert an empty XDG_RUNTIME_DIR value
to nil while preserving non-empty values and unset-variable behavior. Add a
regression test covering an explicitly empty XDG_RUNTIME_DIR and verify the
unresolved runtime path remains nil.
| local function is_directory(path) | ||
| local pipe = io.popen("[ -d '" .. path .. "' ] && echo 1 || echo 0") |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/sh
# Inspect shell command construction without executing repository files.
rg -n -C 3 'io\.popen|shell_quote|is_directory' \
Configs/.local/share/hypr/lua/hyde/path.luaRepository: HyDE-Project/HyDE
Length of output: 901
🏁 Script executed:
#!/bin/sh
set -eu
echo "== path.lua relevant section =="
sed -n '1,130p' Configs/.local/share/hypr/lua/hyde/path.lua
echo
echo "== is_directory call sites / exposed API =="
rg -n -C 2 'is_directory|first_directory|os\.getenv|XDG|HOME' Configs/.local/share/hypr/lua/hyde/path.lua
echo
echo "== shell quoting probe without repository code =="
python3 - <<'PY'
def current(path):
return f"[ -d '{path}' ] && echo 1 || echo 0"
examples = [
"/home/alice",
"/home/alice's project",
"/home/alice; rm -rf /",
"/home/alice'\"; echo PWNED; #",
]
for path in examples:
cmd = current(path)
print(repr(path))
print(cmd)
print()
PYRepository: HyDE-Project/HyDE
Length of output: 7346
Shell-quote paths before probing them.
is_directory() interpolates HOME/XDG values into a shell command without escaping. A path containing ' breaks quoting; if HOME or an XDG directory is set maliciously, it can inject shell syntax while Hyprland reads its configuration.
Proposed fix
+local function shell_quote(value)
+ return "'" .. value:gsub("'", "'\\''") .. "'"
+end
+
local function is_directory(path)
- local pipe = io.popen("[ -d '" .. path .. "' ] && echo 1 || echo 0")
+ local pipe = io.popen("[ -d " .. shell_quote(path) .. " ] && echo 1 || echo 0")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| local function is_directory(path) | |
| local pipe = io.popen("[ -d '" .. path .. "' ] && echo 1 || echo 0") | |
| local function shell_quote(value) | |
| return "'" .. value:gsub("'", "'\\''") .. "'" | |
| end | |
| local function is_directory(path) | |
| local pipe = io.popen("[ -d " .. shell_quote(path) .. " ] && echo 1 || echo 0") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Configs/.local/share/hypr/lua/hyde/path.lua` around lines 64 - 65, Update
is_directory to safely shell-quote the path before interpolating it into the
io.popen test command, preserving correct directory detection for paths
containing apostrophes and preventing injected shell syntax from executing.
2 participants
Refs #1897
Problem
hyde/path.luais the first thing the Hyprland Lua configuration loads, and it resolved every directory by concatenating an environment variable:Two ways that fails on a session the compositor is trying to start:
HOMEabsent — the fallback raisesattempt to concatenate a nil valueand the configuration dies before the first window. Same expression in thelibandsharecandidate lists.os.getenvreturns"", which is truthy in Lua, soP.configbecomes""and every path built from it points at the filesystem root. The XDG specification says an empty value is to be treated as unset; the repository's ownluautils/xdg.luaalready does that.dynamic.luathen repeated the first mistake directly:That is the path a session started from a TTY takes, without uwsm to export the variable. The
io.openprobe also never closed its handle, leaking one on every configuration reload.Change
hyde/path.luaresolves through one helper that treats unset and empty alike and returnsnilwhenHOMEcannot supply a fallback. A field that cannot be resolved staysnilfor the caller to skip — raising in this module takes the whole configuration down, which is the failure being fixed. The directory probe closes its pipe, and thelibandsharelookups drop the user candidate instead of concatenating onto an absentHOME. The module and its helpers are documented.dynamic.luareads the directory fromhyde.pathand closes the handle it opens.XDG_RUNTIME_DIRdeliberately keeps no fallback: the specification puts it on the session to provide, and inventing a path would place runtime state somewhere that outlives the session.Tests
tests/test_paths.sh(new) checks that an emptyXDG_CONFIG_HOMEfalls back toHOME, that a set one is honoured, and that an absentHOMEleaves the field unresolved rather than raising. It also assertsdynamic.luaneither rebuilds the path fromos.getenvnor probes a file without closing it.Verified against the previous revision of each file: restoring the old resolver fails the suite with
attempt to concatenate a nil value, and restoring the olddynamic.luafails both of its assertions.Full suite: 10 cases, 0 failed.
Scope
This is not a fix for the timeout reported in #1897 — the measurements in that thread put the cost in the size of the file being parsed. These are two defects found in the same two lines while reading it.
Type of change
Checklist
Summary by CodeRabbit
Bug Fixes
XDG_CONFIG_HOMEorHOMEis unavailable.Tests
Documentation