Move hook session state off the shared /tmp path to a per-user root#36
Merged
Conversation
Summary
Store per-session hook state under a per-user XDG root instead of a
predictable, world-writable `/tmp` path, closing a shared-host tamper
and write-redirection hazard.
Hook state (prompt-cycle markers, once-per-session markers, edit-churn
counters, MCP-failure counters, the session-start "context emitted"
marker) lived at `${TMPDIR:-/tmp}/hook-state/<session>`. The fixed
`/tmp/hook-state` name in a world-writable directory lets another user on
a shared host pre-create it (as a directory they own, or a symlink) and
then tamper with a victim's guard state, deny writes, or redirect the
writes into a victim's tree (CWE-377/CWE-59).
- add `_hook_state_root`, resolving a per-user root: prefer
`XDG_RUNTIME_DIR` (a per-user 0700 directory the system clears on
logout — the canonical home for ephemeral runtime state, so stale
per-session dirs don't accumulate), then `XDG_STATE_HOME`, then
`~/.local/state`, and only a uid-scoped `${TMPDIR:-/tmp}` path when
neither a runtime dir nor `HOME` exists, so hooks never hard-fail
- add `_hook_mkstate`, which creates state dirs under `umask 077` so the
`XDG_STATE_HOME`/`~/.local/state` fallback tiers are 0700 too, matching
the privacy the runtime tier gives for free (session activity metadata
is not readable by other users on a shared host)
- document the new root in both README copies
Out of scope, consciously accepted: the daily plugin-update stamp in
`bin/agent-hook-session-start-claude` (`/tmp/claude-plugins-updated-$(id
-u)`) is already uid-scoped and has a different (daily, persistent)
lifecycle, so it is left as-is rather than folded into the ephemeral
session-state root. A separate, pre-existing test-hermeticity gap
(`XDG_CACHE_HOME` is not scrubbed by the suite, so the protected-bare
classifier cache writes to the real `~/.cache`) is unrelated to this
change and left for its own fix.
Testing
- update the test harness to scrub ambient XDG and re-point
`XDG_RUNTIME_DIR` at the test tmpdir so fixtures and the test process's
own `_hook_*` helpers stay hermetic; document the hermeticity contract
- rewrite the source-time state-dir tests to drive the resolver per tier,
and add cascade tests: `XDG_RUNTIME_DIR` wins, then `XDG_STATE_HOME`,
then `~/.local/state`, then a uid-scoped tmp last resort
- verified the created state dirs are 0700 across tiers
- full suite green (`./test/agentguard-test`: ok, all suites 0 failed);
`checkrun lint` and `shellcheck -x lib/agentguard/hook-helpers.sh` clean
- reviewed by two independent adversarial fresh-eyes passes; their
confirmed items (state-dir privacy on the fallback tiers, hermeticity
contract) are folded into this change
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Store per-session hook state under a per-user XDG root instead of a
predictable, world-writable
/tmppath, closing a shared-host tamperand write-redirection hazard.
Hook state (prompt-cycle markers, once-per-session markers, edit-churn
counters, MCP-failure counters, the session-start "context emitted"
marker) lived at
${TMPDIR:-/tmp}/hook-state/<session>. The fixed/tmp/hook-statename in a world-writable directory lets another user ona shared host pre-create it (as a directory they own, or a symlink) and
then tamper with a victim's guard state, deny writes, or redirect the
writes into a victim's tree (CWE-377/CWE-59).
_hook_state_root, resolving a per-user root: preferXDG_RUNTIME_DIR(a per-user 0700 directory the system clears onlogout — the canonical home for ephemeral runtime state, so stale
per-session dirs don't accumulate), then
XDG_STATE_HOME, then~/.local/state, and only a uid-scoped${TMPDIR:-/tmp}path whenneither a runtime dir nor
HOMEexists, so hooks never hard-fail_hook_mkstate, which creates state dirs underumask 077so theXDG_STATE_HOME/~/.local/statefallback tiers are 0700 too, matchingthe privacy the runtime tier gives for free (session activity metadata
is not readable by other users on a shared host)
Out of scope, consciously accepted: the daily plugin-update stamp in
bin/agent-hook-session-start-claude(/tmp/claude-plugins-updated-$(id -u)) is already uid-scoped and has a different (daily, persistent)lifecycle, so it is left as-is rather than folded into the ephemeral
session-state root. A separate, pre-existing test-hermeticity gap
(
XDG_CACHE_HOMEis not scrubbed by the suite, so the protected-bareclassifier cache writes to the real
~/.cache) is unrelated to thischange and left for its own fix.
Testing
XDG_RUNTIME_DIRat the test tmpdir so fixtures and the test process'sown
_hook_*helpers stay hermetic; document the hermeticity contractand add cascade tests:
XDG_RUNTIME_DIRwins, thenXDG_STATE_HOME,then
~/.local/state, then a uid-scoped tmp last resort./test/agentguard-test: ok, all suites 0 failed);checkrun lintandshellcheck -x lib/agentguard/hook-helpers.shcleanconfirmed items (state-dir privacy on the fallback tiers, hermeticity
contract) are folded into this change