feat(hooks): add session memory policy controls (blockSessionSave, redirectPath) [claude, human developer oversight]#35567
Conversation
…directPath) Allow upstream hooks (e.g. security/provenance plugins) to control session memory behavior: - blockSessionSave: prevent session from being persisted to memory - sessionSaveRedirectPath: redirect session save to alternate path - sessionSaveContent: override the saved content [claude, human developer oversight]
Greptile SummaryThis PR introduces three new context fields (
The code is safe to merge and ready for use by upstream hooks and security plugins. Confidence Score: 5/5
Last reviewed commit: ae873ad |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f6d612032
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
When sessionSaveRedirectPath is set, write to the full redirect path (mkdir -p + appendFile) instead of path.basename() under memoryDir. Fixes the quarantine use case where redirected saves silently landed in the default memory directory.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 029a706af1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Use fs.open with mode 0o600 (owner-only) and mkdir with 0o700 for redirect paths, matching writeFileWithinRoot's confidentiality guarantees. Prevents session data exposure on shared hosts.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee5cf1307c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Instead of bypassing writeFileWithinRoot for redirected session saves, shift the rootDir to the redirect target's parent directory. This preserves path traversal protection while honoring the redirect.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17cca2bb37
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
writeFileWithinRoot requires rootDir to be realpath-resolvable. When redirecting to a fresh quarantine path, the parent directory may not exist yet. Create it before calling writeFileWithinRoot.
3 tests for blockSessionSave, sessionSaveRedirectPath, and sessionSaveContent. Uses existing test infrastructure.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d899f4a8d6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Resolve relative redirect paths against workspaceDir - Reject paths outside workspace with log warning, fall back to normal memory dir - 2 new tests: relative path resolution + workspace escape rejection - Total: 21 tests (16 existing + 5 new)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3adfbe5486
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…t check path.normalize() doesn't resolve symlinks — a symlink inside workspace could point outside it. Now uses fs.realpath() on the parent directory to get the true filesystem path before checking containment.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2f8660304
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… custom content 1. Two-pass validation: textual containment check BEFORE mkdir, then symlink resolution after. Prevents creating directories outside workspace on invalid redirect paths. 2. Skip transcript loading and LLM slug generation when sessionSaveContent is set. Prevents leaking raw session text to the model provider in redaction workflows.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab62f9b470
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Stop doing manual path validation (normalize, realpath, startsWith). Instead, use writeFileWithinRoot with workspaceDir as root for ALL writes. It handles path traversal, symlink resolution, containment, and mkdir internally. Invalid redirect paths fall back to memory dir. This is the canonical pattern used throughout the codebase (fs-safe.ts) and handles all edge cases: symlinks, relative paths, path alias escapes, and workspace-as-symlink.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb6c0182b5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…rors only Operational I/O failures (ENOSPC, EACCES on a valid quarantine path) now fail closed instead of silently falling back to the default memory dir, which could bypass security policy intent in deployments using sessionSaveRedirectPath.
When a redirect path is rejected, log the fallback writePath instead of the rejected writeRelativePath to avoid misleading security audits.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f444fc8d47
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…verride An empty string is a valid redaction signal — hooks may intentionally set sessionSaveContent to '' to persist a blank marker while avoiding transcript retention. Previously this was treated as unset due to the length > 0 check.
…orkflows writeFileWithinRoot handles directory creation internally, so the pre-existing unconditional fs.mkdir(memoryDir) was creating an empty memory/ directory even in redirect-only/quarantine workflows. Removed to avoid unintended artifacts.
…prove redirect fallback warning
- blockSessionSave now reads from the safe 'context' local variable
(with || {} fallback) instead of accessing event.context directly
outside the try block.
- Redirect fallback log.warn now explicitly warns that quarantine
workflows should pair sessionSaveRedirectPath with blockSessionSave.
|
Makes sense — centralizing path security in writeFileWithinRoot is cleaner than per-policy guards. Good approach. |
…emory handler Two new context fields for upstream hooks (e.g. security plugins) to control session memory persistence: - blockSessionSave (boolean): prevent session from being saved to memory - sessionSaveContent (string): override saved content with custom text (empty string is valid — persists a blank marker without transcript) When sessionSaveContent is set, LLM slug generation and session content loading are skipped (unnecessary when content is overridden). Split from openclaw#35567 — sessionSaveRedirectPath follows separately as it requires path canonicalization, symlink resolution, and filesystem write policy review.
…emory handler Two new context fields for upstream hooks (e.g. security plugins) to control session memory persistence: - blockSessionSave (boolean): prevent session from being saved to memory - sessionSaveContent (string): override saved content with custom text (empty string is valid — persists a blank marker without transcript) When sessionSaveContent is set, LLM slug generation and session content loading are skipped (unnecessary when content is overridden). Split from openclaw#35567 — sessionSaveRedirectPath follows separately as it requires path canonicalization, symlink resolution, and filesystem write policy review.
Allows upstream hooks to redirect session memory writes to an alternate path (e.g. quarantine directory for tainted sessions). Security: - canonicalizeViaAncestor() resolves symlinks via nearest existing ancestor - writeFileWithinRoot validates containment within workspace - Fails closed for outside-workspace paths (no fallback to default memory/) - Both workspace and redirect paths canonicalized before comparison Depends on openclaw#38161 (blockSessionSave + sessionSaveContent). Split from openclaw#35567 per review feedback.
Summary
Allows upstream hooks (e.g. security/provenance plugins) to control session memory persistence behavior via three new context fields:
blockSessionSave— prevent the session from being persisted to memory (e.g. for sensitive/classified conversations)sessionSaveRedirectPath— redirect session save to an alternate file path (e.g. quarantine directory)sessionSaveContent— override the saved contentThese fields are read by the bundled
session-memoryhook handler. Upstream hooks (registered by plugins, which load before bundled hooks in FIFO order) can set these fields to control save behavior.Changes
src/hooks/bundled/session-memory/handler.ts— checkblockSessionSave, usesessionSaveRedirectPathfor file path, supportsessionSaveContentoverrideTesting
pnpm checkclean. Deployed locally and in daily use for ~4 weeks.