fix(v0.19.2): polish — marker perms 0600, ANSI sanitize, manifest size bound, dead-code cleanup
Closes remaining MEDIUM/LOW audit findings not in v0.19.0 security wave.
M1 — marker file 0600 perms (unix)
config.rs::write() applies chmod 0o600 after write, cfg(unix) gated.
Test marker_file_has_0600_perms_on_unix asserts mode & 0o777 == 0o600.
L9 — ANSI-escape sanitization
New module display.rs (27 LOC) — sanitize_display(&str) replaces
ASCII < 0x20 OR == 0x7F with '?', leaves space + unicode alone.
Applied in status.rs + attach.rs to brain_name / brain_path /
attached_at / client_type / config_path / mcp_path before print.
Test status_sanitizes_control_chars_in_brain_name asserts
sanitize_display('evil\x1b[2Jpayload') → 'evil?[2Jpayload'.
L12 — manifest size bound
brain_validate.rs const MAX_MANIFEST_BYTES = 64 * 1024; metadata
check before read_to_string. New Error::ManifestTooLarge { size, max }
with thiserror Display impl. Test manifest_too_large_rejected
writes 100 KB manifest, asserts error + marker not written.
Dead-code cleanup:
- Error::NotAttached: #[allow(dead_code)] + comment (reserved for
future detach subcommand when no marker exists)
- config::has_client: #[allow(dead_code)] + comment (reserved for
future multi-brain support)
- mount.rs / detach.rs: dropped unused ClientAdapter import
brain.rs module doc-comment expanded — lists all v0.19 invariants:
path confinement, symlink reject, name regex, 64 KiB manifest cap,
schema v1; notes v2 (multi-platform) lands in v0.20.
Tests: 16 existing + 3 new = 19/19 pass.
cargo check -p keisei: zero warnings in keisei crate.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
51715f2045
commit
e43b13335e
12 changed files with 189 additions and 16 deletions
|
|
@ -37,9 +37,15 @@ _primitives/_rust/target/release/kei-changelog \
|
|||
### Changed
|
||||
- Placeholder: plugin / block format refresh targeted for v0.16.0.
|
||||
|
||||
### Security
|
||||
- **primitives/keisei (v0.19.2 audit polish — M1):** `keisei-attached.toml` marker is now `chmod 0o600` on unix (Windows unchanged — no equivalent bit). The marker carries the resolved `brain_path` and every attached client's config path; restricting it to owner-only closes the residual "other local user can enumerate attached brains" surface.
|
||||
- **primitives/keisei (v0.19.2 audit polish — L9):** every manifest-sourced string printed by `status` and `attach` (brain name, brain path, client/config paths) is now scrubbed through `display::sanitize_display`, which replaces every ASCII control byte (`< 0x20` or `== 0x7F`) with `?`. Closes the escape-sequence injection surface from a malicious `brain.name` like `"evil\x1b[2Jpayload"` that would otherwise clear the user's terminal or rewrite already-printed lines.
|
||||
- **primitives/keisei (v0.19.2 audit polish — L12):** `manifest.toml` is now capped at 64 KiB (`Error::ManifestTooLarge { size, max }`). The check runs off `fs::metadata` before `read_to_string` so an attacker-supplied 1 GB file can't exhaust memory inside the toml parser. Legit manifests are ~1 KB; the cap is three orders of magnitude of headroom.
|
||||
|
||||
### Fixed
|
||||
- Placeholder: hook-bypass edge case follow-up to v0.15.1.
|
||||
- **primitives/keisei (v0.19 audit hardening):** close 3 Security HIGH + 3 Critic HIGH + 2 Critic MEDIUM findings. Path-escape guard on `mcp_server` + `memory/artifacts/manifests` (absolute / `..` / canonical-mismatch → `PathEscape`); brain-name regex `^[a-z][a-z0-9_-]{0,63}$` (`InvalidName`); symlink-rooted brain inputs rejected (`BrainIsSymlink` — closes USB → `$HOME` pivot); MCP-entry collision check across all 4 adapters (`NameConflict` instead of silent clobber); dropped unused `rusqlite` dep (no C toolchain tail); `BrainPaths.{memory,artifacts,manifests}` relaxed to `Option<String>`; `$KEISEI_HOME`/`$HOME` resolver deduped into `paths.rs` SSoT; `fsx::write_atomic` rewritten on `tempfile::NamedTempFile` for Windows + cross-fs correctness; 5 adversarial integration tests added (16 total pass).
|
||||
- **primitives/keisei (v0.19.2 polish):** dropped unused `ClientAdapter` imports from `mount.rs` + `detach.rs`; `Error::NotAttached` and `AttachRecord::has_client` now carry explicit `#[allow(dead_code)]` markers documenting that they're reserved for future callers / test-only respectively. `cargo check -p keisei` is warning-clean; integration suite is 19/19 pass (3 new: `marker_file_has_0600_perms_on_unix`, `status_sanitizes_control_chars_in_brain_name`, `manifest_too_large_rejected`). `brain.rs` module-level doc-comment now lists the v0.19 invariants (path confinement / symlink reject / name regex / manifest size cap) and flags schema v2 as v0.20.
|
||||
|
||||
## [0.15.0] — 2026-04-22
|
||||
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@
|
|||
use crate::adapter::{detect_active, ClientAdapter};
|
||||
use crate::brain::Brain;
|
||||
use crate::config::{self, AttachRecord, Attachment};
|
||||
use crate::display::sanitize_display;
|
||||
use crate::error::Result;
|
||||
use std::path::Path;
|
||||
|
||||
|
|
@ -35,9 +36,12 @@ fn build_record(brain: &Brain, adapter: &dyn ClientAdapter) -> AttachRecord {
|
|||
}
|
||||
|
||||
fn print_summary(brain: &Brain, adapter: &dyn ClientAdapter, marker: &std::path::Path) {
|
||||
println!("attached brain '{}' to {}", brain.name(), adapter.name());
|
||||
println!(" brain path: {}", brain.root.display());
|
||||
println!(" mcp server: {}", brain.mcp_server_path().display());
|
||||
let brain_name = sanitize_display(brain.name());
|
||||
let brain_path = sanitize_display(&brain.root.to_string_lossy());
|
||||
let mcp_path = sanitize_display(&brain.mcp_server_path().to_string_lossy());
|
||||
println!("attached brain '{}' to {}", brain_name, adapter.name());
|
||||
println!(" brain path: {}", brain_path);
|
||||
println!(" mcp server: {}", mcp_path);
|
||||
println!(" client cfg: {}", adapter.config_path().display());
|
||||
println!(" marker: {}", marker.display());
|
||||
println!("run /help in Claude Code to verify the MCP server is reachable");
|
||||
|
|
|
|||
|
|
@ -18,11 +18,24 @@
|
|||
//! manifests = "manifests/" # optional
|
||||
//! ```
|
||||
//!
|
||||
//! Every path under `[paths]` MUST be relative AND resolve (after
|
||||
//! canonicalization) inside the brain root. Absolute paths or `..`
|
||||
//! traversal are rejected with `Error::PathEscape`. Symlink roots are
|
||||
//! rejected with `Error::BrainIsSymlink` (user must pass the canonical
|
||||
//! path explicitly to avoid USB→host pivot).
|
||||
//! # v0.19 invariants (audit-hardened)
|
||||
//!
|
||||
//! - **Path confinement** — every path under `[paths]` MUST be relative;
|
||||
//! absolute paths and `..` components are rejected syntactically, and
|
||||
//! the canonical form must remain inside the brain root
|
||||
//! (`Error::PathEscape`).
|
||||
//! - **Symlink reject** — the brain-root input itself cannot be a
|
||||
//! symlink; the user must pass the canonical path to close the
|
||||
//! USB → `$HOME` pivot (`Error::BrainIsSymlink`).
|
||||
//! - **Name regex** — `^[a-z][a-z0-9_-]{0,63}$` on `brain.name`: lowercase
|
||||
//! letter start, up to 64 chars, word chars + hyphen only
|
||||
//! (`Error::InvalidName`).
|
||||
//! - **Manifest size bound** — `manifest.toml` is capped at 64 KiB
|
||||
//! (`brain_validate::MAX_MANIFEST_BYTES`); anything larger returns
|
||||
//! `Error::ManifestTooLarge` before the toml parser sees a byte.
|
||||
//! - **Schema** — only `schema_version = 1` is accepted today. v2
|
||||
//! (multi-platform `mcp_server` per `{os, arch}`) is planned for v0.20
|
||||
//! and will be read side-by-side once landed.
|
||||
//!
|
||||
//! Constructor Pattern: single responsibility — parse + compose the five
|
||||
//! validation primitives from `brain_validate.rs` into the load pipeline.
|
||||
|
|
|
|||
|
|
@ -43,11 +43,23 @@ pub fn canonicalize_root(input: &Path) -> Result<PathBuf> {
|
|||
})
|
||||
}
|
||||
|
||||
/// L12 (v0.19.2 audit): cap manifest.toml at 64 KiB. A well-formed brain
|
||||
/// manifest is ~1 KB; anything larger is either corruption or an attempt
|
||||
/// to exhaust memory by feeding an enormous file through the toml parser.
|
||||
pub const MAX_MANIFEST_BYTES: u64 = 64 * 1024;
|
||||
|
||||
pub fn read_manifest(root: &Path) -> Result<BrainManifest> {
|
||||
let mpath = root.join(MANIFEST_FILENAME);
|
||||
if !mpath.is_file() {
|
||||
return Err(Error::BrainNotFound(mpath));
|
||||
}
|
||||
let meta = std::fs::metadata(&mpath)?;
|
||||
if meta.len() > MAX_MANIFEST_BYTES {
|
||||
return Err(Error::ManifestTooLarge {
|
||||
size: meta.len(),
|
||||
max: MAX_MANIFEST_BYTES,
|
||||
});
|
||||
}
|
||||
let raw = std::fs::read_to_string(&mpath)?;
|
||||
let manifest: BrainManifest = toml::from_str(&raw)?;
|
||||
Ok(manifest)
|
||||
|
|
|
|||
|
|
@ -53,6 +53,11 @@ pub struct AttachRecord {
|
|||
|
||||
impl AttachRecord {
|
||||
/// Convenience: are we attached to the given client?
|
||||
///
|
||||
/// Bin never calls this directly today (detach iterates `attachments`
|
||||
/// instead), but integration tests use it as a semantic assertion and
|
||||
/// it's part of the public shape.
|
||||
#[allow(dead_code)]
|
||||
pub fn has_client(&self, client: &str) -> bool {
|
||||
self.attachments.iter().any(|a| a.client_type == client)
|
||||
}
|
||||
|
|
@ -113,6 +118,15 @@ pub fn write(rec: &AttachRecord) -> Result<PathBuf> {
|
|||
}
|
||||
let text = toml::to_string_pretty(rec)?;
|
||||
std::fs::write(&path, text)?;
|
||||
// M1 (v0.19.2 audit): marker holds the brain_path + attached client list
|
||||
// — restrict to owner-only on unix. Windows has no equivalent bit.
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
let mut perms = std::fs::metadata(&path)?.permissions();
|
||||
perms.set_mode(0o600);
|
||||
std::fs::set_permissions(&path, perms)?;
|
||||
}
|
||||
Ok(path)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@
|
|||
//! failures are collected and reported but do NOT abort the other
|
||||
//! detaches — partial detach is better than stuck state.
|
||||
|
||||
use crate::adapter::{self, ClientAdapter};
|
||||
use crate::adapter;
|
||||
use crate::config::{self, AttachRecord};
|
||||
use crate::error::Result;
|
||||
|
||||
|
|
|
|||
27
_primitives/_rust/keisei/src/display.rs
Normal file
27
_primitives/_rust/keisei/src/display.rs
Normal file
|
|
@ -0,0 +1,27 @@
|
|||
//! Display sanitization — ANSI/control-char stripper for user-facing output.
|
||||
//!
|
||||
//! Constructor Pattern: single responsibility — convert untrusted strings
|
||||
//! (brain names, paths from manifests) into display-safe text by replacing
|
||||
//! every ASCII control character (`< 0x20` or `== 0x7F`) with `?`. Space
|
||||
//! (0x20) is preserved. Characters outside ASCII (emoji / unicode) pass
|
||||
//! through unchanged — their UTF-8 bytes are all `> 0x7F` and never
|
||||
//! collide with the control-byte range.
|
||||
//!
|
||||
//! Closes L9 (v0.19.2 audit): a malicious manifest
|
||||
//! `name = "evil\x1b[2J..."` would clear the terminal or inject escape
|
||||
//! sequences when `status` prints it. Every branch that prints
|
||||
//! manifest-sourced text MUST route through `sanitize_display` first.
|
||||
|
||||
/// Replace every ASCII control character (`< 0x20` or `== 0x7F`) with `?`.
|
||||
/// Space is preserved. Non-ASCII characters pass through unchanged.
|
||||
pub fn sanitize_display(s: &str) -> String {
|
||||
let mut out = String::with_capacity(s.len());
|
||||
for c in s.chars() {
|
||||
if (c as u32) < 0x20 || c == '\x7F' {
|
||||
out.push('?');
|
||||
} else {
|
||||
out.push(c);
|
||||
}
|
||||
}
|
||||
out
|
||||
}
|
||||
|
|
@ -13,13 +13,20 @@ pub enum Error {
|
|||
#[error("brain manifest not found at {0}")]
|
||||
BrainNotFound(PathBuf),
|
||||
|
||||
#[error("manifest too large: {size} bytes (limit {max})")]
|
||||
ManifestTooLarge { size: u64, max: u64 },
|
||||
|
||||
#[error("brain schema version {found} not supported (need 1)")]
|
||||
UnsupportedSchema { found: u32 },
|
||||
|
||||
#[error("no supported client detected in this directory")]
|
||||
NoClientDetected,
|
||||
|
||||
// Reserved for `detach` subcommand when no marker exists — today
|
||||
// that path prints "nothing to detach" and returns Ok(()) instead,
|
||||
// but the variant stays so callers can pattern-match in the future.
|
||||
#[error("no brain currently attached")]
|
||||
#[allow(dead_code)]
|
||||
NotAttached,
|
||||
|
||||
#[error("adapter '{client}' failed: {reason}")]
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ mod brain;
|
|||
mod brain_validate;
|
||||
mod config;
|
||||
mod detach;
|
||||
mod display;
|
||||
mod error;
|
||||
mod fsx;
|
||||
mod list;
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@
|
|||
//! successful list → print summary). No config-schema knowledge beyond
|
||||
//! what the `config` module already owns.
|
||||
|
||||
use crate::adapter::{self, ClientAdapter};
|
||||
use crate::adapter;
|
||||
use crate::brain::Brain;
|
||||
use crate::config::{self, AttachRecord, Attachment};
|
||||
use crate::error::{Error, Result};
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@
|
|||
|
||||
use crate::brain::Brain;
|
||||
use crate::config::{self, AttachRecord};
|
||||
use crate::display::sanitize_display;
|
||||
use crate::error::Result;
|
||||
use std::path::PathBuf;
|
||||
|
||||
|
|
@ -25,20 +26,21 @@ pub fn run() -> Result<()> {
|
|||
}
|
||||
|
||||
fn print_record(rec: &AttachRecord) {
|
||||
println!("brain: {}", rec.brain_name);
|
||||
println!("brain path: {}", rec.brain_path);
|
||||
println!("attached at: {}", rec.attached_at);
|
||||
println!("brain: {}", sanitize_display(&rec.brain_name));
|
||||
println!("brain path: {}", sanitize_display(&rec.brain_path));
|
||||
println!("attached at: {}", sanitize_display(&rec.attached_at));
|
||||
if rec.attachments.is_empty() {
|
||||
println!("clients: (none — marker migrated from v1 without client info)");
|
||||
} else {
|
||||
println!("clients: {}", rec.client_names().join(", "));
|
||||
let names = rec.client_names().join(", ");
|
||||
println!("clients: {}", sanitize_display(&names));
|
||||
for a in &rec.attachments {
|
||||
let cfg = if a.config_path.is_empty() {
|
||||
"(unknown — v1 marker)".to_string()
|
||||
} else {
|
||||
a.config_path.clone()
|
||||
sanitize_display(&a.config_path)
|
||||
};
|
||||
println!(" - {}: {}", a.client_type, cfg);
|
||||
println!(" - {}: {}", sanitize_display(&a.client_type), cfg);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -16,6 +16,8 @@ mod brain;
|
|||
mod brain_validate;
|
||||
#[path = "../src/config.rs"]
|
||||
mod config;
|
||||
#[path = "../src/display.rs"]
|
||||
mod display;
|
||||
#[path = "../src/fsx.rs"]
|
||||
mod fsx;
|
||||
#[path = "../src/adapters/mod.rs"]
|
||||
|
|
@ -482,3 +484,88 @@ attached_at = "2026-04-22T00:00:00Z"
|
|||
assert_eq!(rec.attachments[0].config_path, "");
|
||||
assert!(rec.has_client("claude-code"));
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// v0.19.2 polish tests (M1 perms / L9 sanitize / L12 manifest size).
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn marker_file_has_0600_perms_on_unix() {
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
let _g = setup_home();
|
||||
let brain_dir = tempfile::tempdir().unwrap();
|
||||
write_brain(brain_dir.path(), 1);
|
||||
|
||||
attach::run(brain_dir.path()).expect("attach ok");
|
||||
|
||||
let marker = config::attached_path();
|
||||
let mode = fs::metadata(&marker).unwrap().permissions().mode() & 0o777;
|
||||
assert_eq!(
|
||||
mode, 0o600,
|
||||
"marker perms = {:04o}; expected 0600 (owner r/w only)",
|
||||
mode
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn status_sanitizes_control_chars_in_brain_name() {
|
||||
// Unit test on the sanitize primitive — simpler and tighter than
|
||||
// capturing stdout from `status::run`. L9 wiring in `status.rs` and
|
||||
// `attach.rs` calls through `display::sanitize_display` at every
|
||||
// manifest-sourced printf site, so asserting the primitive is enough.
|
||||
assert_eq!(
|
||||
display::sanitize_display("evil\x1b[2Jpayload"),
|
||||
"evil?[2Jpayload"
|
||||
);
|
||||
// Space / regular ASCII pass through.
|
||||
assert_eq!(display::sanitize_display("my brain 01"), "my brain 01");
|
||||
// DEL (0x7F) is scrubbed too.
|
||||
assert_eq!(display::sanitize_display("a\x7Fb"), "a?b");
|
||||
}
|
||||
|
||||
/// Write a brain with an artificially large manifest.toml for the size-
|
||||
/// bound rejection test. `min_bytes` is the lower bound on the resulting
|
||||
/// file size; the manifest body is padded with a block-comment-like
|
||||
/// filler string (`\n# xxxxxx...`) until the total exceeds `min_bytes`.
|
||||
fn write_brain_with_oversize_manifest(root: &Path, min_bytes: usize) -> PathBuf {
|
||||
fs::create_dir_all(root.join("bin")).unwrap();
|
||||
fs::write(root.join("bin/kei-mcp-server-test"), b"#!/bin/sh\n").unwrap();
|
||||
let mut manifest = String::from(
|
||||
r#"[brain]
|
||||
schema_version = 1
|
||||
name = "test-brain"
|
||||
created = "2026-04-22T00:00:00Z"
|
||||
|
||||
[paths]
|
||||
mcp_server = "bin/kei-mcp-server-test"
|
||||
"#,
|
||||
);
|
||||
// Pad with toml-legal trailing comments to grow past the 64 KiB cap.
|
||||
let filler = format!("# {}\n", "x".repeat(120));
|
||||
while manifest.len() < min_bytes {
|
||||
manifest.push_str(&filler);
|
||||
}
|
||||
fs::write(root.join("manifest.toml"), manifest).unwrap();
|
||||
root.to_path_buf()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn manifest_too_large_rejected() {
|
||||
let _g = setup_home();
|
||||
let brain_dir = tempfile::tempdir().unwrap();
|
||||
// 100 KiB manifest — well above the 64 KiB cap.
|
||||
write_brain_with_oversize_manifest(brain_dir.path(), 100 * 1024);
|
||||
|
||||
let err = attach::run(brain_dir.path()).unwrap_err();
|
||||
assert!(
|
||||
matches!(
|
||||
err,
|
||||
error::Error::ManifestTooLarge { size, max }
|
||||
if size > max && max == 64 * 1024
|
||||
),
|
||||
"expected ManifestTooLarge {{ size > max, max == 65536 }}, got {err:?}"
|
||||
);
|
||||
// Containment: marker MUST NOT be written on rejection.
|
||||
assert!(config::read().unwrap().is_none());
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue