diff --git a/Cargo.lock b/Cargo.lock index 4b586ca..2e6f912 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1953,6 +1953,7 @@ version = "0.12.10" dependencies = [ "anyhow", "chrono", + "libc", "rusqlite", "serde", "serde_json", diff --git a/crates/rewind-store/Cargo.toml b/crates/rewind-store/Cargo.toml index 691d160..5e9a7f9 100644 --- a/crates/rewind-store/Cargo.toml +++ b/crates/rewind-store/Cargo.toml @@ -14,5 +14,8 @@ anyhow = { workspace = true } thiserror = { workspace = true } tracing = { workspace = true } +[target.'cfg(unix)'.dependencies] +libc = "0.2" + [dev-dependencies] tempfile = "3" diff --git a/crates/rewind-store/src/blobs.rs b/crates/rewind-store/src/blobs.rs index 38e9c22..2ee90ae 100644 --- a/crates/rewind-store/src/blobs.rs +++ b/crates/rewind-store/src/blobs.rs @@ -1,6 +1,10 @@ use anyhow::Result; use sha2::{Digest, Sha256}; use std::fs; +#[cfg(unix)] +use std::fs::OpenOptions; +#[cfg(unix)] +use std::io::Write; use std::path::{Path, PathBuf}; /// Content-addressed blob store (like git objects). @@ -12,6 +16,11 @@ pub struct BlobStore { impl BlobStore { pub fn new(root: &Path) -> Result { fs::create_dir_all(root)?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let _ = fs::set_permissions(root, fs::Permissions::from_mode(0o700)); + } Ok(BlobStore { root: root.to_path_buf(), }) @@ -26,6 +35,17 @@ impl BlobStore { if let Some(parent) = path.parent() { fs::create_dir_all(parent)?; } + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + let mut f = OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o600) + .open(&path)?; + f.write_all(data)?; + } + #[cfg(not(unix))] fs::write(&path, data)?; } diff --git a/crates/rewind-store/src/db.rs b/crates/rewind-store/src/db.rs index 8f1add3..ed27a40 100644 --- a/crates/rewind-store/src/db.rs +++ b/crates/rewind-store/src/db.rs @@ -15,6 +15,20 @@ impl Store { pub fn open(root: &Path) -> Result { std::fs::create_dir_all(root)?; + // Harden directory and file permissions. See docs/security-audit.md §HIGH-03. + // ~/.rewind/ contains full LLM conversations that may include API keys, PII, + // and proprietary code — restrict to owner-only access. + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let dir_perms = std::fs::Permissions::from_mode(0o700); + std::fs::set_permissions(root, dir_perms)?; + + // Validate directory ownership: refuse to use a data dir owned by + // another user (guards against REWIND_DATA hijack — see LOW-07). + validate_owner(root)?; + } + let db_path = root.join("rewind.db"); let blobs_path = root.join("objects"); @@ -34,6 +48,15 @@ impl Store { ) })?; + // Tighten DB file to 0600 (owner read/write only). + // WAL/SHM files (-wal, -shm) are created by SQLite at its default umask; + // the parent dir's 0700 is the intended defense for those. + #[cfg(unix)] + if db_path.exists() { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&db_path, std::fs::Permissions::from_mode(0o600))?; + } + let blobs = BlobStore::new(&blobs_path)?; let store = Store { @@ -1740,6 +1763,38 @@ pub struct QueryResult { pub rows: Vec>, } +/// Validate that the data directory is owned by the current user and not +/// world- or group-writable. Guards against `REWIND_DATA` hijack (see +/// docs/security-audit.md §LOW-07). +#[cfg(unix)] +fn validate_owner(path: &Path) -> Result<()> { + use std::os::unix::fs::{MetadataExt, PermissionsExt}; + let meta = std::fs::metadata(path)?; + let dir_uid = meta.uid(); + let my_uid = unsafe { libc::geteuid() }; + if dir_uid != my_uid { + anyhow::bail!( + "Data directory {} is owned by uid {} but current user is uid {}. \ + Refusing to use a directory owned by another user (SSRF/hijack risk). \ + Delete the directory or fix ownership.", + path.display(), + dir_uid, + my_uid, + ); + } + let mode = meta.mode(); + if mode & 0o022 != 0 { + tracing::warn!( + path = %path.display(), + mode = format!("{:o}", mode & 0o777), + "Data directory is group- or world-writable — tightening to 0700" + ); + let perms = std::fs::Permissions::from_mode(0o700); + std::fs::set_permissions(path, perms)?; + } + Ok(()) +} + /// Resolve the Rewind data directory, honoring `REWIND_DATA` then `$HOME/.rewind`. pub fn dirs_path() -> PathBuf { if let Ok(data_dir) = std::env::var("REWIND_DATA") { diff --git a/docs/security-audit.md b/docs/security-audit.md index 36793e8..cbf2cdb 100644 --- a/docs/security-audit.md +++ b/docs/security-audit.md @@ -248,6 +248,7 @@ The `name` value comes from `sqlite_master` so it's trusted here, but the patter ### HIGH-03: Sensitive Data Stored Unencrypted at Rest +**Status:** ✅ **Partially fixed in PR #137** — data directory (`~/.rewind/`) now chmod 0700, DB file chmod 0600, blob store dir chmod 0700 on every open. Owner validation rejects dirs owned by other users. Python SDK mirrors the same. Data is still unencrypted (SQLCipher is a future follow-up) but filesystem ACLs now prevent unauthorized reads. **Severity:** High **Affected component:** `crates/rewind-store/src/blobs.rs`, `crates/rewind-store/src/db.rs` **OWASP:** A02:2021 Cryptographic Failures @@ -636,6 +637,7 @@ This misses: `private_key`, `client_secret`, `aws_secret_access_key`, `bearer`, ### LOW-07: `REWIND_DATA` Environment Variable Allows Data Directory Hijack +**Status:** ✅ **Fixed in PR #137** — `Store::open()` now validates that the data directory is owned by the current user (`geteuid` on unix). Directories owned by other users are rejected with a clear error. Group/world-writable directories are auto-tightened to 0700 with a warning. Python SDK logs a warning on uid mismatch. **Severity:** Low **Affected component:** `crates/rewind-store/src/db.rs:1696` (`dirs_path()`) @@ -751,7 +753,7 @@ Reordered per peer review — fail-closed auth first, then SSRF, then redaction, | **1** | Fail closed on non-loopback bind without `--auth-token`. Generate default token on first run. Apply to HTTP + WebSocket + OTLP ingest routes. | CRITICAL-02, MEDIUM-09 (WS) | Medium | ✅ Shipped (PR #133) | | **2** | Deny private/link-local/loopback in `export/otel` endpoint resolver | CRITICAL-01 | Small | ✅ Shipped (PR #134) | | **3+4** | Blob redaction (request + response), hop-by-hop header denylist, `query_raw` lockdown, `pragma_table_info()` | HIGH-01, HIGH-02, HIGH-06, MEDIUM-06, MEDIUM-08 | Medium | ✅ Shipped (PR #135) | -| **5** | `chmod 0700 ~/.rewind/` and `0600` on files in `Store::open()`; add owner check on `REWIND_DATA` path | HIGH-03, LOW-07 | Small | ⏳ Planned | +| **5** | `chmod 0700 ~/.rewind/` and `0600` on files in `Store::open()`; add owner check on `REWIND_DATA` path | HIGH-03, LOW-07 | Small | ✅ Shipped (PR #137) | ### Next Tier (P2 — ship after the above) diff --git a/python/rewind_agent/store.py b/python/rewind_agent/store.py index 44c90d0..a9b0283 100644 --- a/python/rewind_agent/store.py +++ b/python/rewind_agent/store.py @@ -9,12 +9,49 @@ import hashlib import json +import logging import os import sqlite3 +import stat import threading import uuid from datetime import datetime, timezone +_log = logging.getLogger("rewind") + + +def _harden_dir(path: str) -> None: + """Set directory permissions to 0700 (owner-only). No-op on non-unix.""" + try: + os.chmod(path, stat.S_IRWXU) # 0700 + except OSError as e: + _log.debug("chmod 0700 %s failed: %s", path, e) + + +def _harden_file(path: str) -> None: + """Set file permissions to 0600 (owner read/write). No-op on non-unix.""" + try: + if os.path.exists(path): + os.chmod(path, stat.S_IRUSR | stat.S_IWUSR) # 0600 + except OSError as e: + _log.debug("chmod 0600 %s failed: %s", path, e) + + +def _validate_owner(path: str) -> None: + """Warn if the data directory is owned by a different user.""" + try: + dir_stat = os.stat(path) + if dir_stat.st_uid != os.geteuid(): + _log.warning( + "Data directory %s is owned by uid %d but current user is uid %d. " + "This may indicate a REWIND_DATA hijack.", + path, dir_stat.st_uid, os.geteuid(), + ) + except AttributeError: + pass # non-unix (no os.geteuid) + except OSError as e: + _log.debug("owner check on %s failed: %s", path, e) + # ── Blob Store ──────────────────────────────────────────────── @@ -24,6 +61,7 @@ class BlobStore: def __init__(self, root: str): self._root = root os.makedirs(root, exist_ok=True) + _harden_dir(root) def put(self, data: bytes) -> str: """Store data and return its SHA-256 hex hash.""" @@ -292,9 +330,12 @@ def __init__(self, root: str = None): if root is None: root = os.environ.get("REWIND_DATA") or os.path.join(os.path.expanduser("~"), ".rewind") os.makedirs(root, exist_ok=True) + _harden_dir(root) + _validate_owner(root) db_path = os.path.join(root, "rewind.db") self._conn = sqlite3.connect(db_path, check_same_thread=False) + _harden_file(db_path) self._conn.execute("PRAGMA journal_mode=WAL") self._conn.execute("PRAGMA foreign_keys=ON") self._conn.execute("PRAGMA busy_timeout=5000")