fix(security): fail-closed XDG_RUNTIME_DIR and harden state file writes

Remove predictable /tmp/omarchy-key-visualizer.json fallback that exposed
captured key combinations and allowed symlink redirection when
XDG_RUNTIME_DIR is absent. Introduce fail-closed behaviour: disable
capture if XDG_RUNTIME_DIR is missing, equals /tmp, is not absolute,
not owned or not 0700.

Add secure_write with O_NOFOLLOW mitigation (test ! -L), atomic
tmp+rename and chmod 600 to prevent world-readable leaks and TOCTOU
races. Apply same guard to QML statePath/superPath and Panel nudge
writes (umask 077, chmod 600, symlink check).

Fixes security review at 790b07c0c9:
key-visualizer.lua:15-17,141-145
This commit is contained in:
felixzsh
2026-08-24 14:00:12 -05:00
parent 790b07c0c9
commit 89cd794708
3 changed files with 78 additions and 19 deletions
+6 -2
View File
@@ -105,7 +105,9 @@ Item {
readonly property string statePath: {
var runtime = Quickshell.env("XDG_RUNTIME_DIR")
return (runtime && runtime.length > 0 ? runtime : "/tmp") + "/omarchy-key-visualizer.json"
if (!runtime || runtime.length === 0) return ""
if (runtime === "/tmp") return ""
return runtime + "/omarchy-key-visualizer.json"
}
// Super-held flag written by the Lua capture hook. While Super is down the
@@ -114,7 +116,9 @@ Item {
property bool superHeld: false
readonly property string superPath: {
var runtime = Quickshell.env("XDG_RUNTIME_DIR")
return (runtime && runtime.length > 0 ? runtime : "/tmp") + "/omarchy-key-visualizer-super"
if (!runtime || runtime.length === 0) return ""
if (runtime === "/tmp") return ""
return runtime + "/omarchy-key-visualizer-super"
}
// True while the cursor hovers the card with Super held: the moment when the
// compositor's SUPER+mouse move/resize binds are temporarily unbound so the
+9 -3
View File
@@ -40,7 +40,9 @@ Panel {
// the visualizer renders the move like a real keypress.
readonly property string statePath: {
var runtime = Quickshell.env("XDG_RUNTIME_DIR")
return (runtime && runtime.length > 0 ? runtime : "/tmp") + "/omarchy-key-visualizer.json"
if (!runtime || runtime.length === 0) return ""
if (runtime === "/tmp") return ""
return runtime + "/omarchy-key-visualizer.json"
}
// Config lives outside the plugin folder (the shell reloads all plugin
@@ -125,10 +127,12 @@ Panel {
var nx = Math.max(-2000, Math.min(2000, root.offsetX + dx * root.nudgeStep))
var ny = Math.max(-2000, Math.min(2000, root.offsetY + dy * root.nudgeStep))
root.writeConfig({ offsetX: nx, offsetY: ny })
if (!root.statePath || root.statePath.length === 0) return
if (root.statePath === "/tmp/omarchy-key-visualizer.json") return
var arrow = dx < 0 ? "←" : dx > 0 ? "→" : dy < 0 ? "↑" : "↓"
var now = Math.floor(Date.now() / 1000)
nudgeKeyProc.command = ["sh", "-c",
"printf '%s' '" + JSON.stringify({ keys: [arrow], t: now }) + "' > " + Util.shellQuote(root.statePath)]
"if [ ! -L " + Util.shellQuote(root.statePath) + " ]; then umask 077; printf '%s' '" + JSON.stringify({ keys: [arrow], t: now }) + "' > " + Util.shellQuote(root.statePath) + " && chmod 600 " + Util.shellQuote(root.statePath) + "; fi"]
nudgeKeyProc.running = true
nudgeReleaseTimer.interval = 120
nudgeReleaseTimer.restart()
@@ -161,9 +165,11 @@ Panel {
interval: 120
repeat: false
onTriggered: {
if (!root.statePath || root.statePath.length === 0) return
if (root.statePath === "/tmp/omarchy-key-visualizer.json") return
var now = Math.floor(Date.now() / 1000)
nudgeKeyProc.command = ["sh", "-c",
"printf '%s' '" + JSON.stringify({ keys: [], t: now }) + "' > " + Util.shellQuote(root.statePath)]
"if [ ! -L " + Util.shellQuote(root.statePath) + " ]; then umask 077; printf '%s' '" + JSON.stringify({ keys: [], t: now }) + "' > " + Util.shellQuote(root.statePath) + " && chmod 600 " + Util.shellQuote(root.statePath) + "; fi"]
nudgeKeyProc.running = true
}
}
+63 -14
View File
@@ -12,9 +12,64 @@
-- then reload Hyprland (`hyprctl reload`). Requires Hyprland with Lua
-- config support (0.56+).
local runtime = os.getenv("XDG_RUNTIME_DIR") or ""
if runtime == "" then runtime = "/tmp" end
local STATE_FILE = runtime .. "/omarchy-key-visualizer.json"
local function shell_quote(s) return "'" .. s:gsub("'", "'\\''") .. "'" end
local function is_runtime_secure(r)
if not r or r == "" then return false end
if r == "/tmp" then return false end
if r:sub(1, 1) ~= "/" then return false end
local q = shell_quote(r)
-- Must be a directory owned by the current user and, if stat is
-- available, mode 0700 (systemd's XDG_RUNTIME_DIR default). The
-- stat check is skipped when stat is missing so we don't fail-closed
-- on minimal containers.
local cmd = "test -d " .. q .. " && test -O " .. q .. " && { p=$(stat -c %a " .. q .. " 2>/dev/null); [ -z \"$p\" ] || [ \"$p\" = \"700\" ]; }"
local res = os.execute(cmd)
return res == 0 or res == true
end
local runtime = os.getenv("XDG_RUNTIME_DIR")
if not is_runtime_secure(runtime) then
if runtime and runtime ~= "" then
print("[key-visualizer] insecure XDG_RUNTIME_DIR, disabling capture: " .. tostring(runtime))
else
print("[key-visualizer] XDG_RUNTIME_DIR not set, disabling capture")
end
runtime = nil
end
local STATE_FILE = runtime and (runtime .. "/omarchy-key-visualizer.json") or nil
local function secure_write(path, content)
if not path then return false end
-- Reject the exact predictable fallback that the security review flagged.
-- A valid XDG_RUNTIME_DIR under /tmp with a random suffix and 0700 is
-- allowed because it passed is_runtime_secure().
if path == "/tmp/omarchy-key-visualizer.json" or path == "/tmp/omarchy-key-visualizer-super" then return false end
local q = shell_quote(path)
-- Refuse to follow a symlink at the destination (O_NOFOLLOW mitigation).
-- `test ! -L` succeeds when the file does not exist or is not a symlink;
-- it fails only when the destination is a symlink, which we must not follow.
local not_symlink = os.execute("test ! -L " .. q)
if not (not_symlink == 0 or not_symlink == true) then
print("[key-visualizer] refusing to write symlink: " .. path)
return false
end
local tmp = path .. ".tmp"
local f = io.open(tmp, "w")
if not f then return false end
f:write(content)
f:close()
os.execute("chmod 600 " .. shell_quote(tmp) .. " 2>/dev/null")
-- Atomic replace; avoids truncating a file that may have been swapped
-- between the symlink check and the open (TOCTOU mitigation).
local ok = os.rename(tmp, path)
if not ok then
os.execute("rm -f " .. shell_quote(tmp) .. " 2>/dev/null")
return false
end
os.execute("chmod 600 " .. q .. " 2>/dev/null")
return true
end
-- Modifier names, keyed by xkb keycode (evdev + 8).
local MODS = {
@@ -130,6 +185,7 @@ end
local last_payload = ""
local function emit()
if not STATE_FILE then return end
local parts = labels()
local payload = '{"keys":['
if #parts > 0 then
@@ -138,30 +194,23 @@ local function emit()
payload = payload .. '],"t":' .. os.time() .. '}'
if payload == last_payload then return end
last_payload = payload
local f = io.open(STATE_FILE, "w")
if f then
f:write(payload)
f:close()
end
secure_write(STATE_FILE, payload)
end
-- Super-held flag: the panel/display watches this to know when to capture the
-- SUPER+drag on the overlay (instead of a window underneath). Written only on
-- transitions so it does not spam the filesystem on every key.
local SUPER_FLAG = runtime .. "/omarchy-key-visualizer-super"
local SUPER_FLAG = runtime and (runtime .. "/omarchy-key-visualizer-super") or nil
local last_super = nil
local function super_down()
return pressed[133] or pressed[134]
end
local function emit_super()
if not SUPER_FLAG then return end
local down = super_down()
if down == last_super then return end
last_super = down
local f = io.open(SUPER_FLAG, "w")
if f then
f:write(down and "1" or "0")
f:close()
end
secure_write(SUPER_FLAG, down and "1" or "0")
end
-- Combos: a combination of keys is treated as a unit. The display only