Merge security hardening for GHSA-jpcg-w5c5-wcv6

# Conflicts:
#	crates/ai-memory-cli/src/commands/install_mcp.rs
#	docs/lifecycle-ops.md
This commit is contained in:
AkitaOnRails
2026-10-22 19:12:41 -03:00
21 changed files with 1487 additions and 152 deletions
+41
View File
@@ -512,6 +512,47 @@ version in [`docs/changelog/`](docs/changelog/):
and remote `server_url` topologies. `docs/airgapped-install.md`'s
runtime summary was corrected to match.
### Fixed
- Hardened the privacy strip. It now matches credentials across invisible
formatting characters while storing text that matches nothing byte for
byte, so emoji sequences and scripts that need joiners are untouched. It
also redacts PGP private keys and private-key blocks that have no end line,
as long as a key-body line follows the header, so prose that only names a
header is kept. Tag characters outside an emoji flag, a byte-order mark
after the start of the text and the invisible math operators are stripped.
The allowlist no longer exempts private keys, and blank `[sanitize]
allowlist` entries are ignored with a warning at startup. The client scrub
shared by the relay and importer companions follows the same rules.
Reported by @FabioLeitao.
([GHSA-jpcg-w5c5-wcv6](https://github.com/akitaonrails/ai-memory/security/advisories/GHSA-jpcg-w5c5-wcv6))
- Tightened how client commands decide that a server URL is on this machine
before using the install's root token. The check now reads the host the
HTTP client would connect to and refuses any URL that carries user
information. `ai-memory upgrade` uses the same check for its remote-server
note. Reported by @FabioLeitao.
([GHSA-jpcg-w5c5-wcv6](https://github.com/akitaonrails/ai-memory/security/advisories/GHSA-jpcg-w5c5-wcv6))
- `ai-memory restore` no longer replaces an existing live `config.toml` with
the backup's by default, because a config can run commands and redirect
data. The backup's copy is saved beside it as `config.toml.from-backup`
(owner-only) with a notice; pass the new `--with-config` flag to restore it
in place as before. A data dir with no config still gets the backup's. When
the live config is kept and the restored database holds API keys, its pepper
must match them, or the restore stops before changing anything and names
`--with-config`. Reported by @FabioLeitao.
([GHSA-jpcg-w5c5-wcv6](https://github.com/akitaonrails/ai-memory/security/advisories/GHSA-jpcg-w5c5-wcv6))
- `install-mcp --client codex` now makes Codex ask before running ai-memory's
destructive tools. The read tools still run without a prompt; every tool
annotated destructive (deleting or restoring a page, the forget sweep,
accepting or cancelling a handoff, popping or cancelling messages, recall
feedback) gets a per-tool `approval_mode = "prompt"`, and a re-install keeps
per-tool settings you already chose. Re-run `install-mcp --client codex
--apply` to update an existing entry. Reported by @FabioLeitao.
([GHSA-jpcg-w5c5-wcv6](https://github.com/akitaonrails/ai-memory/security/advisories/GHSA-jpcg-w5c5-wcv6))
## [3.2.0] - 2026-10-21
### Added
+17
View File
@@ -35,6 +35,23 @@ what the project is and is not designed to defend against.
not chmodded automatically. Windows uses its filesystem ACLs rather than
POSIX mode bits.
The hook spool (`<data_dir>/hook-spool/`) holds each captured lifecycle
event until the server accepts it. Each file carries the event body as the
harness sent it, before the server's privacy strip runs (the client scrubs
only a field that exceeds its size cap and the opt-in assistant excerpt),
plus a copy of the static hook bearer when the event authenticates with one;
an OIDC event stores no token and resolves it at delivery. Files are `0600`
in a `0700` directory. A drain at session boundaries delivers and deletes
them; an event that cannot be delivered is dropped after 7 days, and the
spool is capped at 10,000 files. `ai-memory backup` does not include the
spool, but a filesystem-level copy of the data directory does.
An `ai-memory backup` archive includes `<data_dir>/config.toml` verbatim,
which may hold the API-key pepper and other inline credentials; protect
the archive as a secret (see
[`docs/lifecycle-ops.md`](docs/lifecycle-ops.md#backup)). `restore` keeps
an existing live config unless `--with-config` is passed.
- **Network exposure when binding to non-loopback addresses.** If you run
`ai-memory serve --bind 0.0.0.0:…` you are exposing the MCP and admin
routes to your local network. Protect this with:
+138 -32
View File
@@ -1,50 +1,147 @@
//! Local credential scrub shared by external capture clients.
//!
//! The server sanitizer (`ai-memory-core::sanitize`) re-scrubs everything
//! these clients send; this copy keeps credentials out of the local spool,
//! queue and preview files written before delivery. It follows the same
//! rules for private keys and invisible characters: key blocks are matched
//! first, including PGP armor and a block cut off before its end line, and
//! invisible formatting characters do not hide a credential from the
//! patterns.
use regex::Regex;
use std::sync::OnceLock;
pub fn sanitize_external_text(input: &str) -> String {
let controls_removed: String = input
.chars()
.map(|ch| {
if ch == '\n'
|| ch == '\r'
|| ch == '\t'
|| !(ch.is_control()
|| matches!(ch, '\u{202a}'..='\u{202e}' | '\u{2066}'..='\u{2069}'))
{
ch
} else {
'\u{fffd}'
}
})
.collect();
let mut offset = 0;
let mut controls_removed = String::with_capacity(input.len());
while let Some(ch) = input[offset..].chars().next() {
let next = offset + ch.len_utf8();
if ch == '\u{1f3f4}' {
let tags = emoji_tag_run_len(&input[next..]);
controls_removed.push_str(&input[offset..next + tags]);
offset = next + tags;
continue;
}
// Removed rather than replaced, before matching: a placeholder left
// inside a credential would split it out of reach of every pattern.
if ch == '\n'
|| ch == '\r'
|| ch == '\t'
|| !(ch.is_control()
|| matches!(ch, '\u{202a}'..='\u{202e}' | '\u{2066}'..='\u{2069}')
|| is_hidden_text_char(offset, ch))
{
controls_removed.push(ch);
}
offset = next;
}
secret_patterns()
.iter()
.fold(controls_removed, |text, rule| {
// Through invisible characters first, so a split credential is
// replaced whole; then the plain pass, which still finds a match
// that needs an invisible character as its word boundary.
let text = if text.contains(is_default_ignorable) {
redact_through_invisibles(&text, rule)
} else {
text
};
rule.regex
.replace_all(&text, |captures: &regex::Captures<'_>| {
let matched = captures.get(0).map_or("", |matched| matched.as_str());
if captures.get(3).is_some_and(|value| {
value.as_str().chars().count() < 6
|| [
"[REDACTED]",
"[REDACTED CREDENTIAL]",
"[REDACTED PRIVATE KEY]",
]
.contains(&value.as_str())
}) {
matched.to_owned()
} else {
let mut replacement = String::new();
captures.expand(rule.replacement, &mut replacement);
replacement
}
replacement(rule, captures)
})
.into_owned()
})
}
fn replacement(rule: &RedactionRule, captures: &regex::Captures<'_>) -> String {
let matched = captures.get(0).map_or("", |matched| matched.as_str());
if captures.get(3).is_some_and(|value| {
value.as_str().chars().count() < 6
|| [
"[REDACTED]",
"[REDACTED CREDENTIAL]",
"[REDACTED PRIVATE KEY]",
]
.contains(&value.as_str())
}) {
matched.to_owned()
} else {
let mut replacement = String::new();
captures.expand(rule.replacement, &mut replacement);
replacement
}
}
/// Run `rule` over `text` with invisible characters removed and replace each
/// match's span in `text`, invisible characters inside it included.
fn redact_through_invisibles(text: &str, rule: &RedactionRule) -> String {
let mut shadow = String::with_capacity(text.len());
let mut origin = Vec::with_capacity(text.len());
for (index, ch) in text.char_indices() {
if !is_default_ignorable(ch) {
shadow.push(ch);
origin.extend(index..index + ch.len_utf8());
}
}
let mut output = String::with_capacity(text.len());
let mut copied = 0;
for captures in rule.regex.captures_iter(&shadow) {
let Some(found) = captures.get(0) else {
continue;
};
let start = origin.get(found.start()).copied().unwrap_or(text.len());
let end = match found.end().checked_sub(1).and_then(|last| origin.get(last)) {
Some(last) if found.end() > found.start() => last + 1,
_ => start,
};
let replaced = replacement(rule, &captures);
// A match the rule keeps stays byte for byte, invisible characters
// and all.
if start < copied || replaced == found.as_str() {
continue;
}
output.push_str(&text[copied..start]);
output.push_str(&replaced);
copied = end;
}
output.push_str(&text[copied..]);
output
}
/// Tag characters outside an emoji flag sequence, the invisible math
/// operators, and a byte-order mark after the start of the text: characters
/// whose only realistic use in captured text is hiding it.
const fn is_hidden_text_char(offset: usize, ch: char) -> bool {
matches!(ch, '\u{2061}'..='\u{2064}' | '\u{e0000}'..='\u{e007f}')
|| (ch == '\u{feff}' && offset > 0)
}
/// Unicode `Default_Ignorable_Code_Point` (UAX #44), the same set the server
/// sanitizer ignores while matching.
const fn is_default_ignorable(ch: char) -> bool {
matches!(ch,
'\u{ad}' | '\u{34f}' | '\u{61c}' | '\u{115f}' | '\u{1160}' | '\u{17b4}' | '\u{17b5}'
| '\u{180b}'..='\u{180f}' | '\u{200b}'..='\u{200f}' | '\u{202a}'..='\u{202e}'
| '\u{2060}'..='\u{206f}' | '\u{3164}' | '\u{fe00}'..='\u{fe0f}' | '\u{feff}'
| '\u{ffa0}' | '\u{fff0}'..='\u{fff8}' | '\u{1bca0}'..='\u{1bca3}'
| '\u{1d173}'..='\u{1d17a}' | '\u{e0000}'..='\u{e0fff}')
}
/// Byte length of the tag run that completes an emoji flag sequence at the
/// start of `rest` (tag letters, then the cancel tag), or 0.
fn emoji_tag_run_len(rest: &str) -> usize {
let mut len = 0;
for ch in rest.chars() {
match ch {
'\u{e0020}'..='\u{e007e}' => len += ch.len_utf8(),
'\u{e007f}' if len > 0 => return len + ch.len_utf8(),
_ => return 0,
}
}
0
}
struct RedactionRule {
regex: Regex,
replacement: &'static str,
@@ -56,11 +153,20 @@ fn secret_patterns() -> &'static [RedactionRule] {
vec![
RedactionRule {
regex: Regex::new(
r"(?s)-----BEGIN [A-Z0-9 ]*PRIVATE KEY-----.*?-----END [A-Z0-9 ]*PRIVATE KEY-----",
r"(?s)-----BEGIN [A-Z0-9 ]*PRIVATE KEY(?: BLOCK)?-----.*?-----END [A-Z0-9 ]*PRIVATE KEY(?: BLOCK)?-----",
)
.unwrap(),
replacement: "[REDACTED PRIVATE KEY]",
},
// A block with no end line, once at least one key-body line
// follows the header; the line end after it is kept.
RedactionRule {
regex: Regex::new(
r#"-----BEGIN [A-Z0-9 ]*PRIVATE KEY(?: BLOCK)?-----(?:(?:\r?\n|(?:\\r)?\\n)[ \t]*(?:(?:Proc-Type|DEK-Info|Version|Comment|Hash):[^\r\n\\]*)?)*?(?:(?:\r?\n|(?:\\r)?\\n)[ \t]*[A-Za-z0-9+/=]{16,}[ \t]*)+(?P<end>\r?(?m:$)|["'\\…]|\.\.\.)"#,
)
.unwrap(),
replacement: "[REDACTED PRIVATE KEY]${end}",
},
RedactionRule {
regex: Regex::new(
r"\b(?:github_pat_[A-Za-z0-9_]{20,}|gh[pousr]_[A-Za-z0-9]{20,}|sk-[A-Za-z0-9_-]{16,}|AKIA[0-9A-Z]{16})\b",
@@ -353,3 +353,64 @@ fn redaction_marker_suffix_never_exempts_a_credential() {
assert_eq!(sanitize_external_text(&control), control);
}
}
/// The vectors the server sanitizer runs too (`ai-memory-core::sanitize`), so
/// the two copies cannot drift: credentials split by an invisible character,
/// private-key blocks with and without an end line, and text that must pass
/// through unchanged.
#[test]
fn shared_sanitizer_vectors_hold() {
let vectors: Value = serde_json::from_str(include_str!(
"../../../../tests/fixtures/sanitizer_vectors.json"
))
.unwrap();
let strings = |key: &str| -> Vec<String> {
vectors[key]
.as_array()
.unwrap()
.iter()
.map(|value| value.as_str().unwrap().to_string())
.collect()
};
for secret in strings("credentials") {
let (head, tail) = secret.split_at(secret.len() / 2);
for invisible in strings("invisibles") {
let out = sanitize_external_text(&format!("key {head}{invisible}{tail} end"));
assert!(
out.contains("[REDACTED") && !out.contains(head) && !out.contains(tail),
"{invisible:?} inside {secret}: {out:?}"
);
assert!(out.starts_with("key ") && out.ends_with(" end"), "{out:?}");
}
}
let body_line = vectors["key_body_line"].as_str().unwrap();
for block in vectors["key_blocks"].as_array().unwrap() {
let input = block["input"].as_str().unwrap();
let out = sanitize_external_text(input);
assert!(
out.contains("[REDACTED PRIVATE KEY]") && !out.contains(body_line),
"{input:?} -> {out:?}"
);
for kept in block["kept"].as_array().unwrap() {
assert!(out.contains(kept.as_str().unwrap()), "{input:?} -> {out:?}");
}
}
for text in strings("unchanged") {
assert_eq!(sanitize_external_text(&text), text);
}
}
#[test]
fn characters_that_hide_text_or_split_a_credential_are_removed() {
let hidden: String = "hidden note"
.chars()
.map(|c| char::from_u32(0xe0000 + u32::from(c)).unwrap())
.collect();
assert_eq!(
sanitize_external_text(&format!("visible{hidden} text")),
"visible text"
);
// A control character inside a credential no longer keeps it whole.
let out = sanitize_external_text("sk-leak-012345\u{0}6789abcdef0123");
assert_eq!(out, "[REDACTED CREDENTIAL]");
}
+6
View File
@@ -2348,6 +2348,12 @@ pub struct RestoreArgs {
/// Overwrite an existing non-empty data dir.
#[arg(long)]
pub force: bool,
/// Replace the live `config.toml` with the backup's. Without it an
/// existing config is kept and the backup's is saved beside it as
/// `config.toml.from-backup` for review: a config can run commands and
/// redirect data, so adopt one only from a backup you trust.
#[arg(long)]
pub with_config: bool,
/// Confirm that the configured token pepper is the original one for a
/// pre-verifier backup that already contains native API keys.
#[arg(long)]
@@ -1806,7 +1806,9 @@ pub async fn drain_with_live_token(
// profile's capture there is the cross-server delivery
// profiles exist to prevent (#992).
let retry = configured.as_deref().and_then(|server| {
if chunk[0].1.profile.is_some() || !is_loopback_url(&chunk[0].1.url) {
if chunk[0].1.profile.is_some()
|| !crate::http_client::is_loopback_url(&chunk[0].1.url)
{
return None;
}
let rerooted = batch_endpoint(&reroot_url(&chunk[0].1.url, server)?);
@@ -2352,22 +2354,6 @@ impl<'a> ProfileTokens<'a> {
}
}
/// Is this URL's host a loopback address?
///
/// A loopback authority is only meaningful on the machine that recorded it, so
/// a stale port in one is unambiguous — nothing else could have been meant. A
/// remote host is left alone: re-pointing it would misdeliver a spool that was
/// deliberately captured against another server.
pub(crate) fn is_loopback_url(url: &str) -> bool {
let rest = match url.split_once("://") {
Some((_, rest)) => rest,
None => url,
};
let authority = rest.split(['/', '?']).next().unwrap_or(rest);
let host = authority.rsplit_once(':').map_or(authority, |(h, _)| h);
host == "127.0.0.1" || host == "localhost" || host == "[::1]" || host == "::1"
}
/// Rewrite `url`'s scheme and authority onto `server_url`, keeping path and
/// query. Used only to retry an endpoint that has already proven unreachable.
fn reroot_url(url: &str, server_url: &str) -> Option<String> {
+127 -12
View File
@@ -2172,20 +2172,27 @@ fn codex_upsert_mcp_server(
// - v3 (this): `[mcp_servers.X.http_headers]` with
// `Authorization = "Bearer ..."`. Codex schema-validates
// and uses it as a static auth header.
// Read before the server table is replaced: per-tool approval
// overrides the user already chose survive a re-install.
let tools = codex_tool_approvals(
doc.get("mcp_servers")
.and_then(|servers| servers.get(&args.name))
.and_then(|server| server.get("tools")),
);
let mut server = Table::new();
server["url"] = value(args.server_url.as_deref().unwrap_or(DEFAULT_MCP_URL));
// Auto-approve ai-memory's tool calls. Without this, Codex
// prompts on EVERY tool invocation ("approve memory_query?"
// "approve memory_briefing?" …) which makes the MCP unusable
// for an auto-capture workflow. The valid TOML values per
// Codex's `AppToolApproval` enum are "auto" / "prompt" /
// "approve" — `approve` means "no prompt, just run it". ai-
// memory's surface is dominantly read-only (query, recent,
// status, briefing, explore); the few writes (consolidate,
// forget_sweep) are tagged `destructiveHint: true` upstream
// so any agent that wants to gate THOSE specifically can
// override per-tool — see Codex's `[mcp_servers.X.tools]`
// map.
// for an auto-capture workflow. The valid values per Codex's
// config reference are "auto" / "prompt" / "writes" /
// "approve" — `approve` means "no prompt, just run it". The
// tools annotated destructive get a per-tool
// `[mcp_servers.X.tools.<tool>] approval_mode = "prompt"`
// override below (`codex_tool_approvals`).
// https://developers.openai.com/codex/config-reference
// (`mcp_servers.<id>.default_tools_approval_mode`,
// `mcp_servers.<id>.tools.<tool>.approval_mode`).
server["default_tools_approval_mode"] = value("approve");
if args.token_env_var {
server["bearer_token_env_var"] = value(PRIME_AGENT_TOKEN_ENV);
@@ -2194,11 +2201,49 @@ fn codex_upsert_mcp_server(
headers["Authorization"] = value(b);
server["http_headers"] = Item::Table(headers);
}
server.insert("tools", Item::Table(tools));
upsert_toml_mcp_server(doc, &args.name, server);
Ok(())
}
/// Codex `[mcp_servers.<name>.tools]`: the user's existing per-tool entries,
/// copied as plain keys, plus `approval_mode = "prompt"` for every tool
/// annotated destructive that has no approval mode yet. The server-wide
/// default stays `approve`, so the read tools never prompt, while every tool
/// annotated destructive asks first.
fn codex_tool_approvals(existing: Option<&toml_edit::Item>) -> toml_edit::Table {
use toml_edit::{Item, Table, value};
let mut tools = Table::new();
tools.set_implicit(true);
if let Some(existing) = existing.and_then(Item::as_table_like) {
for (tool, entry) in existing.iter() {
let Some(entry) = entry.as_table_like() else {
continue;
};
let mut copy = Table::new();
for (key, item) in entry.iter() {
if let Some(value) = item.as_value() {
copy.insert(key, Item::Value(value.clone()));
}
}
tools.insert(tool, Item::Table(copy));
}
}
for tool in ai_memory_mcp::AiMemoryServer::destructive_tool_names() {
if !tools.contains_key(&tool) {
tools.insert(&tool, Item::Table(Table::new()));
}
if let Some(entry) = tools.get_mut(&tool).and_then(Item::as_table_mut)
&& !entry.contains_key("approval_mode")
{
entry.insert("approval_mode", value("prompt"));
}
}
tools
}
/// Insert / replace `[mcp_servers.<name>]` in a Grok `config.toml`.
///
/// Grok's schema (user-guide § MCP Servers) uses:
@@ -2350,9 +2395,8 @@ fn render_codex(args: &InstallMcpArgs) -> Result<String> {
.context("internal: generated Codex TOML table missing")?;
if let Some(mut key) = server.key_mut("default_tools_approval_mode") {
key.leaf_decor_mut().set_prefix(
"# Skip per-call approval prompts on ai-memory's tools.\n\
# ai-memory is read-mostly + writes are auto-capture; the\n\
# approval friction makes it unusable otherwise.\n",
"# Skip per-call approval prompts on ai-memory's read tools;\n\
# the destructive ones below still ask first.\n",
);
}
if let Some(headers) = server
@@ -5950,6 +5994,77 @@ mod tests {
);
}
/// Codex keeps auto-approving the read tools, but every tool annotated
/// destructive gets a per-tool prompt.
#[test]
fn codex_apply_prompts_for_every_destructive_tool() {
let destructive = ai_memory_mcp::AiMemoryServer::destructive_tool_names();
assert!(
destructive.iter().any(|tool| tool == "memory_delete_page"),
"{destructive:?}"
);
let mut doc: toml_edit::DocumentMut = "".parse().unwrap();
codex_upsert_mcp_server(&mut doc, &args_for(McpClient::Codex)).unwrap();
let out = doc.to_string();
assert!(
out.contains("default_tools_approval_mode = \"approve\""),
"{out}"
);
let server = &doc["mcp_servers"]["ai-memory"];
for tool in &destructive {
assert_eq!(
server["tools"][tool.as_str()]["approval_mode"].as_str(),
Some("prompt"),
"{tool} must prompt:\n{out}"
);
}
for read_tool in ["memory_query", "memory_recent", "memory_briefing"] {
assert!(
server["tools"].get(read_tool).is_none(),
"{read_tool} keeps the approve default:\n{out}"
);
}
let rendered = render_codex(&args_for(McpClient::Codex));
for tool in &destructive {
assert!(
rendered.contains(&format!(
"[mcp_servers.ai-memory.tools.{tool}]\napproval_mode = \"prompt\""
)),
"{tool} missing from the printed snippet:\n{rendered}"
);
}
rendered.parse::<toml_edit::DocumentMut>().unwrap();
}
/// A re-install keeps the per-tool choices the user already made.
#[test]
fn codex_apply_keeps_existing_per_tool_overrides() {
let original = "[mcp_servers.ai-memory]\n\
url = \"http://old\"\n\
\n\
[mcp_servers.ai-memory.tools.memory_delete_page]\n\
approval_mode = \"approve\"\n\
\n\
[mcp_servers.ai-memory.tools.memory_query]\n\
approval_mode = \"prompt\"\n";
let mut doc: toml_edit::DocumentMut = original.parse().unwrap();
codex_upsert_mcp_server(&mut doc, &args_for(McpClient::Codex)).unwrap();
let tools = &doc["mcp_servers"]["ai-memory"]["tools"];
assert_eq!(
tools["memory_delete_page"]["approval_mode"].as_str(),
Some("approve")
);
assert_eq!(
tools["memory_query"]["approval_mode"].as_str(),
Some("prompt")
);
assert_eq!(
tools["memory_forget_sweep"]["approval_mode"].as_str(),
Some("prompt")
);
}
/// Sibling `[mcp_servers.<other>]` entries the user has configured
/// (e.g. a different MCP server) must survive an --apply.
#[test]
@@ -318,7 +318,7 @@ fn bridge_bearer(config: &Config, server_url: &str) -> Result<Option<String>> {
}
}
if crate::commands::hook_spool::is_loopback_url(server_url) {
if crate::http_client::is_loopback_url(server_url) {
return Ok(config
.auth
.bearer_token
@@ -781,11 +781,15 @@ mod tests {
discovered: true,
});
// Adversarial: a remote --server-url never receives the root key.
assert_eq!(
bridge_bearer(&config, "https://remote.example/mcp").unwrap(),
None
);
// Adversarial: a remote --server-url never receives the root key,
// including one whose user information spells a loopback address.
for remote in [
"https://remote.example/mcp",
"http://localhost:80@remote.example/mcp",
"http://127.0.0.1:49374@remote.example/mcp",
] {
assert_eq!(bridge_bearer(&config, remote).unwrap(), None, "{remote}");
}
// Control: a loopback server the config does not name (a packaged
// install whose config lives outside the data dir) still gets it.
assert_eq!(
+303 -26
View File
@@ -13,6 +13,14 @@
//! — the moment a restore fails is the moment the operator has no other
//! copy, so the previous state must survive it.
//!
//! A backup's `config.toml` is code-bearing: it can name commands the server
//! runs (`[auto_improve.eval]`), where consolidated memory is sent (provider
//! URLs and headers, admission webhooks) and how much is redacted
//! (`[sanitize]`). A restore therefore keeps an existing live config and saves
//! the archived one beside it as `config.toml.from-backup` for review;
//! `--with-config` restores it in place, and a data dir with no config gets
//! the archived one as before.
//!
//! # Exception to invariant §16
//!
//! `restore` is one of the documented exceptions to the rule that the CLI
@@ -25,7 +33,7 @@
use ai_memory_store::Store;
use anyhow::{Context, Result, bail};
use flate2::read::GzDecoder;
use std::path::{Component, Path};
use std::path::{Component, Path, PathBuf};
use tracing::{info, warn};
use crate::cli::RestoreArgs;
@@ -38,10 +46,33 @@ use crate::process_guard::{busy_message, sibling_processes};
/// `models/`, `raw/` and anything else beside them are never touched.
const REPLACED_DIRS: &[&str] = &["wiki", "db"];
/// The one file a restore replaces, and only when the archive carries it;
/// otherwise the live copy stays.
/// The one file a restore can replace: only when the archive carries it, and
/// only with `--with-config` or when the data dir has none.
const CONFIG_FILE: &str = "config.toml";
/// Where an archived config goes when the live one is kept.
const CONFIG_FROM_BACKUP_FILE: &str = "config.toml.from-backup";
/// What a restore did with the archive's `config.toml`.
#[derive(Debug, PartialEq, Eq)]
enum ConfigOutcome {
/// The archive carried no config; the live one, if any, is untouched.
NotInArchive,
/// The archived config is now the live one.
Restored,
/// The live config was kept and the archived one saved at this path.
SavedBeside(PathBuf),
}
/// The archived config's place: live when asked for or when there is none,
/// otherwise beside the live one.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum ConfigPlacement {
Absent,
Install,
Divert,
}
/// Run the `restore` subcommand.
///
/// # Errors
@@ -59,10 +90,11 @@ pub async fn run(config: &Config, args: RestoreArgs) -> Result<()> {
bail!("source tarball {} not found", args.from.display());
}
restore_data_dir_with_legacy_pepper(
let outcome = restore_data_dir_with_legacy_pepper(
&config.data_dir,
&args.from,
args.force,
args.with_config,
config.auth.token_pepper.as_deref(),
args.accept_legacy_api_key_pepper,
)
@@ -73,26 +105,42 @@ pub async fn run(config: &Config, args: RestoreArgs) -> Result<()> {
args.from.display(),
config.data_dir.display()
);
match outcome {
ConfigOutcome::NotInArchive => {}
ConfigOutcome::Restored => println!("restored the backup's config.toml"),
ConfigOutcome::SavedBeside(path) => println!(
"kept the live config.toml; the backup's copy is at {}. A config can run \
commands and redirect data: review it before adopting it, or rerun with \
--with-config to restore it in place.",
path.display()
),
}
Ok(())
}
/// Stage, validate, then swap. Nothing under `data_dir` changes until the
/// archive has been fully extracted into a staging directory and the
/// staged store has opened; the live `wiki/`, `db/` and (when the archive
/// carries one) `config.toml` are then exchanged for the staged copies by
/// staged store has opened; the live `wiki/`, `db/` and (when it is being
/// restored) `config.toml` are then exchanged for the staged copies by
/// rename and rolled back if any step of the exchange fails.
#[cfg(test)]
async fn restore_data_dir(data_dir: &Path, from: &Path, force: bool) -> Result<()> {
restore_data_dir_with_legacy_pepper(data_dir, from, force, None, false).await
async fn restore_data_dir(
data_dir: &Path,
from: &Path,
force: bool,
with_config: bool,
) -> Result<ConfigOutcome> {
restore_data_dir_with_legacy_pepper(data_dir, from, force, with_config, None, false).await
}
async fn restore_data_dir_with_legacy_pepper(
data_dir: &Path,
from: &Path,
force: bool,
with_config: bool,
token_pepper: Option<&str>,
accept_legacy_api_key_pepper: bool,
) -> Result<()> {
) -> Result<ConfigOutcome> {
let wiki = data_dir.join("wiki");
let db = data_dir.join("db").join("memory.sqlite");
let occupied = (wiki.is_dir() && std::fs::read_dir(&wiki)?.next().is_some()) || db.is_file();
@@ -119,6 +167,7 @@ async fn restore_data_dir_with_legacy_pepper(
from,
&staging,
&previous,
with_config,
token_pepper,
accept_legacy_api_key_pepper,
)
@@ -142,9 +191,10 @@ async fn stage_then_swap(
from: &Path,
staging: &Path,
previous: &Path,
with_config: bool,
token_pepper: Option<&str>,
accept_legacy_api_key_pepper: bool,
) -> Result<()> {
) -> Result<ConfigOutcome> {
std::fs::create_dir(staging)
.with_context(|| format!("creating staging dir {}", staging.display()))?;
@@ -158,6 +208,14 @@ async fn stage_then_swap(
.with_context(|| format!("extracting {} into {}", from.display(), staging.display()))?;
info!(from = %from.display(), into = %staging.display(), "tarball extracted into staging");
let placement = if !staging.join(CONFIG_FILE).is_file() {
ConfigPlacement::Absent
} else if with_config || !data_dir.join(CONFIG_FILE).exists() {
ConfigPlacement::Install
} else {
ConfigPlacement::Divert
};
// 2. Open + drop the staged store so refinery applies any pending
// migrations and the SQLite file is validated — still before anything
// live is touched. Dropping the store joins the writer thread and
@@ -192,19 +250,79 @@ async fn stage_then_swap(
bail!("the restored API-key pepper verifier could not be bound");
}
}
// Keeping the live config instead of the archived one must not leave a
// server that refuses to start: the pepper this command loaded has to be
// the one the restored API keys were hashed with.
if placement == ConfigPlacement::Divert && has_api_credentials {
let matches = match (
token_pepper.filter(|value| !value.trim().is_empty()),
store.reader.api_credential_pepper_verifier().await?,
) {
(Some(pepper), Some(verifier)) => {
ai_memory_store::password::verify_password(pepper.to_owned(), verifier)
.await
.context("verifying the live API-key pepper against the backup")?
}
_ => false,
};
if !matches {
bail!(
"the backup's API keys were hashed with a different [auth].token_pepper than \
the live config's; rerun with --with-config to restore the backup's \
config.toml, or set the live config's token_pepper to the original (the live \
data was not changed)"
);
}
}
drop(store);
// 3. Exchange the live directories for the staged ones.
swap_into_place(data_dir, staging, previous)?;
swap_into_place(
data_dir,
staging,
previous,
placement == ConfigPlacement::Install,
)?;
info!(into = %data_dir.display(), "restored data swapped into place");
Ok(())
Ok(match placement {
ConfigPlacement::Absent => ConfigOutcome::NotInArchive,
ConfigPlacement::Install => ConfigOutcome::Restored,
ConfigPlacement::Divert => {
let beside = data_dir.join(CONFIG_FROM_BACKUP_FILE);
save_archived_config(&staging.join(CONFIG_FILE), &beside).with_context(|| {
format!(
"the restore completed and kept the live config.toml, but saving the \
backup's config.toml to {} failed",
beside.display()
)
})?;
ConfigOutcome::SavedBeside(beside)
}
})
}
/// Move the staged config beside the live one, owner-only: it can hold the
/// API-key pepper and other credentials.
fn save_archived_config(staged: &Path, beside: &Path) -> std::io::Result<()> {
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt as _;
std::fs::set_permissions(staged, std::fs::Permissions::from_mode(0o600))?;
}
std::fs::rename(staged, beside)
}
/// Move the live entries aside into `previous`, move the staged entries
/// into place, then discard `previous`. Every step is a same-filesystem
/// rename; if one fails, the moves already made are reversed so the data
/// dir ends up as it started.
fn swap_into_place(data_dir: &Path, staging: &Path, previous: &Path) -> Result<()> {
fn swap_into_place(
data_dir: &Path,
staging: &Path,
previous: &Path,
install_config: bool,
) -> Result<()> {
std::fs::create_dir(previous).with_context(|| format!("creating {}", previous.display()))?;
// Entries moved from the data dir into `previous`, and staged entries
@@ -222,12 +340,13 @@ fn swap_into_place(data_dir: &Path, staging: &Path, previous: &Path) -> Result<(
}
}
let live_config = data_dir.join(CONFIG_FILE);
if staging.join(CONFIG_FILE).is_file() && live_config.exists() {
if install_config && live_config.exists() {
std::fs::rename(&live_config, previous.join(CONFIG_FILE))
.with_context(|| format!("moving {} aside", live_config.display()))?;
moved_aside.push(CONFIG_FILE);
}
for name in REPLACED_DIRS.iter().chain(std::iter::once(&CONFIG_FILE)) {
let config = install_config.then_some(&CONFIG_FILE);
for name in REPLACED_DIRS.iter().chain(config) {
let staged = staging.join(name);
if staged.exists() {
std::fs::rename(&staged, data_dir.join(name))
@@ -630,7 +749,9 @@ mod tests {
std::fs::read(tmp.path().join("db/memory.sqlite")).unwrap()
}
async fn legacy_api_key_sqlite_bytes() -> Vec<u8> {
/// A snapshot holding one API key, with the pepper verifier bound to
/// `pepper`, or none (a pre-verifier backup) when it is `None`.
async fn api_key_sqlite_bytes(pepper: Option<&str>) -> Vec<u8> {
let tmp = tempfile::TempDir::new().unwrap();
let store = Store::open(tmp.path()).unwrap();
let user_id = store
@@ -666,6 +787,18 @@ mod tests {
.unwrap()
.is_none()
);
if let Some(pepper) = pepper {
let verifier = ai_memory_store::password::hash_password(pepper.to_owned())
.await
.unwrap();
assert!(
store
.writer
.bind_api_credential_pepper_verifier(verifier)
.await
.unwrap()
);
}
let snapshot = tmp.path().join("legacy-snapshot.sqlite");
store.reader.snapshot_to(snapshot.clone()).await.unwrap();
drop(store);
@@ -716,7 +849,7 @@ mod tests {
let scratch = tempfile::TempDir::new().unwrap();
let from = write_tarball(scratch.path(), "bad.tar.gz", b"this is not a gzip stream");
let err = restore_data_dir(data.path(), &from, true)
let err = restore_data_dir(data.path(), &from, true, false)
.await
.unwrap_err();
@@ -735,7 +868,7 @@ mod tests {
let whole = good_archive();
let from = write_tarball(scratch.path(), "cut.tar.gz", &whole[..whole.len() / 2]);
let err = restore_data_dir(data.path(), &from, true)
let err = restore_data_dir(data.path(), &from, true, false)
.await
.unwrap_err();
@@ -764,7 +897,7 @@ mod tests {
]),
);
let err = restore_data_dir(data.path(), &from, true)
let err = restore_data_dir(data.path(), &from, true, false)
.await
.unwrap_err();
@@ -789,7 +922,7 @@ mod tests {
]),
);
let err = restore_data_dir(data.path(), &from, true)
let err = restore_data_dir(data.path(), &from, true, false)
.await
.unwrap_err();
@@ -807,7 +940,7 @@ mod tests {
let scratch = tempfile::TempDir::new().unwrap();
let from = write_tarball(scratch.path(), "good.tar.gz", &good_archive());
let err = restore_data_dir(data.path(), &from, false)
let err = restore_data_dir(data.path(), &from, false, false)
.await
.unwrap_err();
@@ -819,13 +952,17 @@ mod tests {
}
#[tokio::test]
async fn restore_force_replaces_the_live_data_with_the_archive() {
async fn restore_force_with_config_replaces_the_live_data_and_config() {
let data = tempfile::TempDir::new().unwrap();
seed_live_data(data.path());
let scratch = tempfile::TempDir::new().unwrap();
let from = write_tarball(scratch.path(), "good.tar.gz", &good_archive());
restore_data_dir(data.path(), &from, true).await.unwrap();
let outcome = restore_data_dir(data.path(), &from, true, true)
.await
.unwrap();
assert_eq!(outcome, ConfigOutcome::Restored);
assert!(!data.path().join(CONFIG_FROM_BACKUP_FILE).exists());
assert_eq!(
std::fs::read(data.path().join("wiki/default/project/notes/new.md")).unwrap(),
@@ -859,6 +996,51 @@ mod tests {
drop(Store::open(data.path()).unwrap());
}
/// A backup's config can run commands, so by default the live one stays
/// and the archived one is saved beside it, owner-only, for review.
#[tokio::test]
async fn restore_keeps_a_live_config_and_saves_the_archived_one_beside_it() {
let data = tempfile::TempDir::new().unwrap();
seed_live_data(data.path());
let scratch = tempfile::TempDir::new().unwrap();
let archived = b"[auto_improve.eval]\nenabled = true\ncommand = [\"/bin/false\"]\n";
let db = migrated_sqlite_bytes();
let from = write_tarball(
scratch.path(),
"code.tar.gz",
&gz_tarball(&[
("wiki/default/project/notes/new.md", b"new page"),
("db/memory.sqlite", db.as_slice()),
("config.toml", archived),
]),
);
let outcome = restore_data_dir(data.path(), &from, true, false)
.await
.unwrap();
let beside = data.path().join(CONFIG_FROM_BACKUP_FILE);
assert_eq!(outcome, ConfigOutcome::SavedBeside(beside.clone()));
assert_eq!(
std::fs::read(data.path().join("config.toml")).unwrap(),
b"# old config\n",
"the live config must not be replaced without --with-config"
);
assert_eq!(std::fs::read(&beside).unwrap(), archived);
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt as _;
let mode = std::fs::metadata(&beside).unwrap().permissions().mode() & 0o777;
assert_eq!(mode, 0o600, "the archived config can hold credentials");
}
// The data itself is restored.
assert_eq!(
std::fs::read(data.path().join("wiki/default/project/notes/new.md")).unwrap(),
b"new page"
);
assert!(restore_scratch_dirs(data.path()).is_empty());
}
#[tokio::test]
async fn restore_keeps_the_live_config_when_the_archive_carries_none() {
let data = tempfile::TempDir::new().unwrap();
@@ -874,8 +1056,12 @@ mod tests {
]),
);
restore_data_dir(data.path(), &from, true).await.unwrap();
let outcome = restore_data_dir(data.path(), &from, true, true)
.await
.unwrap();
assert_eq!(outcome, ConfigOutcome::NotInArchive);
assert!(!data.path().join(CONFIG_FROM_BACKUP_FILE).exists());
assert_eq!(
std::fs::read(data.path().join("config.toml")).unwrap(),
b"# old config\n"
@@ -893,13 +1079,22 @@ mod tests {
let scratch = tempfile::TempDir::new().unwrap();
let from = write_tarball(scratch.path(), "good.tar.gz", &good_archive());
restore_data_dir(data.path(), &from, false).await.unwrap();
let outcome = restore_data_dir(data.path(), &from, false, false)
.await
.unwrap();
assert_eq!(
std::fs::read(data.path().join("wiki/default/project/notes/new.md")).unwrap(),
b"new page"
);
assert!(data.path().join("db/memory.sqlite").is_file());
// With no live config to keep, the archived one is restored in place.
assert_eq!(outcome, ConfigOutcome::Restored);
assert_eq!(
std::fs::read(data.path().join("config.toml")).unwrap(),
b"# restored config\n"
);
assert!(!data.path().join(CONFIG_FROM_BACKUP_FILE).exists());
assert!(restore_scratch_dirs(data.path()).is_empty());
}
@@ -908,7 +1103,7 @@ mod tests {
let data = tempfile::TempDir::new().unwrap();
seed_live_data(data.path());
let scratch = tempfile::TempDir::new().unwrap();
let db = legacy_api_key_sqlite_bytes().await;
let db = api_key_sqlite_bytes(None).await;
let from = write_tarball(
scratch.path(),
"legacy.tar.gz",
@@ -919,6 +1114,7 @@ mod tests {
data.path(),
&from,
true,
false,
Some("original-pepper"),
false,
)
@@ -936,6 +1132,7 @@ mod tests {
data.path(),
&from,
true,
false,
Some("original-pepper"),
true,
)
@@ -954,4 +1151,84 @@ mod tests {
.unwrap()
);
}
/// Keeping the live config must not leave a server that refuses to start
/// on the restored API keys. A pepper mismatch is refused before anything
/// changes and names `--with-config`; with the flag, or with the
/// original pepper live, the restore goes through.
#[tokio::test]
async fn keeping_the_live_config_requires_its_pepper_to_match_the_restored_keys() {
let scratch = tempfile::TempDir::new().unwrap();
let db = api_key_sqlite_bytes(Some("archive-pepper")).await;
let archived = b"[auth]\ntoken_pepper = \"archive-pepper\"\n";
let from = write_tarball(
scratch.path(),
"keys.tar.gz",
&gz_tarball(&[
("wiki/default/project/notes/new.md", b"new page"),
("db/memory.sqlite", db.as_slice()),
("config.toml", archived),
]),
);
for live_pepper in [Some("live-pepper"), None] {
let data = tempfile::TempDir::new().unwrap();
seed_live_data(data.path());
let refused = restore_data_dir_with_legacy_pepper(
data.path(),
&from,
true,
false,
live_pepper,
false,
)
.await
.unwrap_err();
assert!(
refused.to_string().contains("--with-config"),
"unexpected error: {refused:#}"
);
assert_live_data_untouched(data.path());
assert!(!data.path().join(CONFIG_FROM_BACKUP_FILE).exists());
}
let data = tempfile::TempDir::new().unwrap();
seed_live_data(data.path());
let outcome = restore_data_dir_with_legacy_pepper(
data.path(),
&from,
true,
true,
Some("live-pepper"),
false,
)
.await
.unwrap();
assert_eq!(outcome, ConfigOutcome::Restored);
assert_eq!(
std::fs::read(data.path().join("config.toml")).unwrap(),
archived
);
let data = tempfile::TempDir::new().unwrap();
seed_live_data(data.path());
let outcome = restore_data_dir_with_legacy_pepper(
data.path(),
&from,
true,
false,
Some("archive-pepper"),
false,
)
.await
.unwrap();
assert_eq!(
outcome,
ConfigOutcome::SavedBeside(data.path().join(CONFIG_FROM_BACKUP_FILE))
);
assert_eq!(
std::fs::read(data.path().join("config.toml")).unwrap(),
b"# old config\n"
);
}
}
+1 -1
View File
@@ -515,7 +515,7 @@ fn plan_service(
.to_owned(),
);
};
if !super::hook_spool::is_loopback_url(server_url) {
if !crate::http_client::is_loopback_url(server_url) {
return ServicePlan::Skip(format!(
"the configured server {server_url} is not on this machine; no local service"
));
+14 -11
View File
@@ -936,7 +936,7 @@ fn warn_remote_server(config: &Config) {
if !config.server_url_configured() {
return;
}
if is_loopback_url(&config.server_url) {
if is_local_server_url(&config.server_url) {
return;
}
eprintln!(
@@ -946,12 +946,11 @@ fn warn_remote_server(config: &Config) {
);
}
fn is_loopback_url(url: &str) -> bool {
let lower = url.to_ascii_lowercase();
lower.contains("127.0.0.1")
|| lower.contains("localhost")
|| lower.contains("[::1]")
|| lower.contains("0.0.0.0")
/// Loopback, or the `0.0.0.0` bind address a local server is often reached
/// through. Only decides whether to print the remote-server note.
fn is_local_server_url(url: &str) -> bool {
crate::http_client::is_loopback_url(url)
|| reqwest::Url::parse(url).is_ok_and(|parsed| parsed.host_str() == Some("0.0.0.0"))
}
struct ReqwestFetcher {
@@ -1782,10 +1781,14 @@ mod tests {
}
#[test]
fn is_loopback_detects_common_forms() {
assert!(is_loopback_url("http://127.0.0.1:49374"));
assert!(is_loopback_url("http://localhost:49374"));
assert!(!is_loopback_url("http://192.168.1.10:49374"));
fn local_server_url_detects_common_forms() {
assert!(is_local_server_url("http://127.0.0.1:49374"));
assert!(is_local_server_url("http://localhost:49374"));
assert!(is_local_server_url("http://0.0.0.0:49374"));
assert!(!is_local_server_url("http://192.168.1.10:49374"));
// A host that merely contains a loopback name is remote.
assert!(!is_local_server_url("http://localhost.example.com:49374"));
assert!(!is_local_server_url("http://127.0.0.1.example.com"));
}
#[test]
+58 -1
View File
@@ -1260,7 +1260,7 @@ fn apply_bearer_token_file(auth: &mut AuthSettings, ctx: &TokenFileContext<'_>)
fn discover_root_token(auth: &mut AuthSettings, ctx: &TokenFileContext<'_>) {
if non_empty(auth.bearer_token.as_deref()).is_some()
|| ctx.config_file_exists
|| !crate::commands::hook_spool::is_loopback_url(ctx.server_url)
|| !crate::http_client::is_loopback_url(ctx.server_url)
{
return;
}
@@ -1484,6 +1484,22 @@ pub struct RunProfile {
pub env: BTreeMap<String, String>,
}
/// A blank `[sanitize] allowlist` entry is a substring of every match, so it
/// would exempt everything from redaction. The sanitizer ignores such entries
/// too; dropping them here is where the operator hears about it.
fn drop_blank_sanitize_allowlist_entries(sanitize: &mut ai_memory_core::SanitizeConfig) {
let before = sanitize.allowlist.len();
sanitize.allowlist.retain(|entry| !entry.trim().is_empty());
let dropped = before - sanitize.allowlist.len();
if dropped > 0 {
tracing::warn!(
dropped,
"ignoring blank [sanitize] allowlist entries: an empty entry would \
exempt every match from redaction"
);
}
}
fn validate_run_profiles(run: &RunSettings) -> Result<()> {
const MAX_PROFILES: usize = 128;
const MAX_PROFILE_NAME: usize = 64;
@@ -2450,6 +2466,7 @@ impl Config {
config.runtime_env = runtime_env;
validate_run_profiles(&config.run)?;
drop_blank_sanitize_allowlist_entries(&mut config.sanitize);
// The idle auto-end tick turns `idle_end_after_hours` into a
// microsecond cutoff: a value above a year is a configuration
@@ -3746,6 +3763,28 @@ mod tests {
/// adopt it, or `serve` on the same data dir would start requiring that
/// key and accepting it as its bearer. Control: a key named by
/// `[auth].bearer_token_file` is adopted.
/// A blank allowlist entry would turn redaction off; loading drops it and
/// keeps the real entries.
#[test]
fn config_load_drops_blank_sanitize_allowlist_entries() {
let tmp = TempDir::new().unwrap();
let dd = tmp.path().join("data");
std::fs::create_dir_all(&dd).unwrap();
let config_path = dd.join("config.toml");
std::fs::write(
&config_path,
"[sanitize]\nallowlist = [\"\", \" \", \"KEEP_ME\"]\n",
)
.unwrap();
let cfg = Config::load_with_runtime_env(
Some(&config_path),
Some(dd.clone()),
RuntimeEnv::default(),
)
.unwrap();
assert_eq!(cfg.sanitize.allowlist, vec!["KEEP_ME".to_string()]);
}
#[test]
fn config_load_never_adopts_the_stored_hook_key() {
let tmp = TempDir::new().unwrap();
@@ -4529,6 +4568,24 @@ mod tests {
);
assert!(remote.auth.bearer_token.is_none());
// Nor a remote server whose URL starts with a loopback name: the text
// before `@` is user information, not the host.
for url in [
"http://localhost:80@attacker.example",
"http://127.0.0.1:49374@attacker.example/",
] {
let tmp = TempDir::new().unwrap();
plant_root_token(tmp.path());
let disguised = load_without_config(
tmp.path(),
RuntimeEnv {
server_url: Some(url.into()),
..RuntimeEnv::default()
},
);
assert!(disguised.auth.bearer_token.is_none(), "{url}");
}
// A registered profile named `default` owns that file (#992).
let tmp = TempDir::new().unwrap();
let name = crate::server_profiles::ProfileName::parse("default").unwrap();
+72
View File
@@ -271,6 +271,39 @@ pub(crate) fn split_origin_and_path(raw: &str) -> (String, String) {
(trimmed.to_string(), String::new())
}
/// Whether `url` names a server on this machine: an `http`/`https` URL whose
/// host is `localhost`, an address in `127.0.0.0/8`, or `[::1]`, carrying no
/// user information.
///
/// Parsed the way the HTTP client parses it, so the host checked here is the
/// host a request would reach. Text before an `@` is user information, not
/// the host, and a URL that has any is refused outright rather than
/// interpreted. `localhost.` (trailing dot) is refused too: a fully qualified
/// name can be answered by a DNS server instead of the local resolver. This
/// gates the install's own root token (`config.rs`, `mcp-bridge`), so an
/// unparsable or ambiguous URL is never treated as local.
pub(crate) fn is_loopback_url(url: &str) -> bool {
let Ok(parsed) = reqwest::Url::parse(url) else {
return false;
};
if !matches!(parsed.scheme(), "http" | "https")
|| !parsed.username().is_empty()
|| parsed.password().is_some()
{
return false;
}
let Some(host) = parsed.host_str() else {
return false;
};
if let Some(v6) = host.strip_prefix('[').and_then(|h| h.strip_suffix(']')) {
return v6
.parse::<std::net::Ipv6Addr>()
.is_ok_and(|ip| ip.is_loopback());
}
host.parse::<std::net::Ipv4Addr>()
.map_or(host == "localhost", |ip| ip.is_loopback())
}
/// GET `<endpoint>{path}` with optional query params, deserialise JSON.
///
/// # Errors
@@ -638,6 +671,45 @@ mod tests {
use super::*;
/// The host checked must be the host a request reaches: user
/// information before an `@` is refused, never read as the host.
#[test]
fn loopback_url_is_decided_by_the_parsed_host() {
for url in [
"http://127.0.0.1:49374",
"http://localhost:49374/mcp",
"http://LOCALHOST:1",
"https://localhost",
"http://[::1]:49374",
"http://[::1]",
"http://127.0.0.2:1",
"http://127.255.255.254/",
] {
assert!(is_loopback_url(url), "{url} is loopback");
}
for url in [
"http://localhost:80@attacker.example/",
"http://127.0.0.1:1@evil.example/",
"http://u:p@[::1]:49374@evil.example/",
"http://user@localhost:49374",
"http://user:pw@127.0.0.1:49374",
"http://evil.example#@localhost:1",
"http://evil.example/?@localhost:1",
"http://localhost.evil.example",
"http://127.0.0.1.evil.example",
"http://localhost.:49374",
"http://0.0.0.0:49374",
"http://192.168.1.10:49374",
"http://[::ffff:127.0.0.1]:49374",
"ftp://localhost/",
"localhost:49374",
"not a url",
"",
] {
assert!(!is_loopback_url(url), "{url} is not loopback");
}
}
#[test]
fn only_an_unanswered_request_is_unreachable() {
let refused =
+1 -4
View File
@@ -107,10 +107,7 @@ impl AuthPosture {
/// `server_open` is whether the server is known to answer without a
/// credential.
pub(crate) fn classify(server_open: bool, server_url: &str) -> Self {
match (
server_open,
crate::commands::hook_spool::is_loopback_url(server_url),
) {
match (server_open, crate::http_client::is_loopback_url(server_url)) {
(false, _) => Self::Token,
(true, true) => Self::NoneLoopback,
(true, false) => Self::None,
+435 -31
View File
@@ -27,9 +27,27 @@
//! `[sanitize].allowlist` — the allowlist is checked *per match*, so a
//! pattern still runs but an allowlisted span survives unchanged.
//!
//! Private keys come first: complete PEM and PGP armor blocks, and a block
//! cut off before its end line (`head` of a key file, a truncated tool
//! result) as long as at least one key-body line follows the header. A
//! sentence that only names a header has no body line and survives. The
//! allowlist never exempts a private key.
//!
//! Invisible formatting characters (Unicode `Default_Ignorable_Code_Point`:
//! zero-width spaces and joiners, soft hyphens, variation selectors, …) do
//! not hide a credential from the patterns. When the text carries any, each
//! pattern also runs over a copy with them removed and the match is mapped
//! back to the original span, invisible characters included. Text that
//! matches nothing is stored byte for byte, so emoji sequences and scripts
//! that need joiners (Persian, the Indic scripts) are never altered.
//!
//! Terminal escape sequences, NUL and bidi overrides are stripped rather
//! than redacted: they are not secrets, but stored text is replayed to a
//! terminal by the CLI, and a NUL makes the markdown file binary.
//! terminal by the CLI, and a NUL makes the markdown file binary. Tag
//! characters outside an emoji flag sequence, a byte-order mark after the
//! start of the text, and the invisible math operators are stripped too:
//! captured prose has no use for them, and they can carry text a reader
//! never sees.
//!
//! ## What we deliberately do not catch
//!
@@ -37,6 +55,9 @@
//! be safely redacted without knowing their structure — too many false
//! positives. Operators who care about that level of paranoia should
//! add a custom pattern via `extra_patterns`.
//!
//! Look-alike characters (full-width `sk-…`, other confusables) are not
//! folded: matching does not apply NFKC normalization.
use std::sync::Arc;
@@ -64,10 +85,107 @@ const fn is_stripped_control(c: char) -> bool {
| '\u{7f}'..='\u{9f}' | '\u{202a}'..='\u{202e}' | '\u{2066}'..='\u{2069}')
}
/// Characters whose only realistic use in captured text is hiding it: the
/// tag block (also used by emoji flag sequences, which [`emoji_tag_run_len`]
/// keeps), the invisible math operators, and a byte-order mark anywhere but
/// the start of the text (`U+2060` replaced it as a zero-width no-break
/// space).
const fn is_hidden_text_char(offset: usize, c: char) -> bool {
matches!(c, '\u{2061}'..='\u{2064}' | '\u{e0000}'..='\u{e007f}')
|| (c == '\u{feff}' && offset > 0)
}
/// Unicode `Default_Ignorable_Code_Point` (UAX #44, `DerivedCoreProperties`):
/// characters that render as nothing. Matching ignores them; storage keeps
/// them unless [`is_stripped_control`] or [`is_hidden_text_char`] says
/// otherwise.
const fn is_default_ignorable(c: char) -> bool {
matches!(c,
'\u{ad}' | '\u{34f}' | '\u{61c}' | '\u{115f}' | '\u{1160}' | '\u{17b4}' | '\u{17b5}'
| '\u{180b}'..='\u{180f}' | '\u{200b}'..='\u{200f}' | '\u{202a}'..='\u{202e}'
| '\u{2060}'..='\u{206f}' | '\u{3164}' | '\u{fe00}'..='\u{fe0f}' | '\u{feff}'
| '\u{ffa0}' | '\u{fff0}'..='\u{fff8}' | '\u{1bca0}'..='\u{1bca3}'
| '\u{1d173}'..='\u{1d17a}' | '\u{e0000}'..='\u{e0fff}')
}
/// Byte length of the tag characters that close an emoji flag sequence
/// (`U+1F3F4`, tag letters, `U+E007F` cancel tag) at the start of `rest`, or
/// 0 when `rest` does not start with a well-formed run.
fn emoji_tag_run_len(rest: &str) -> usize {
let mut len = 0;
for c in rest.chars() {
match c {
'\u{e0020}'..='\u{e007e}' => len += c.len_utf8(),
'\u{e007f}' if len > 0 => return len + c.len_utf8(),
_ => return 0,
}
}
0
}
/// `text` with every [`is_default_ignorable`] character removed, plus the
/// original byte offset of each byte kept, so a match found in the shadow
/// maps back to the span it covers in `text`.
struct Shadow {
text: String,
origin: Vec<usize>,
}
impl Shadow {
fn new(text: &str) -> Self {
let mut shadow = String::with_capacity(text.len());
let mut origin = Vec::with_capacity(text.len());
for (index, c) in text.char_indices() {
if is_default_ignorable(c) {
continue;
}
shadow.push(c);
origin.extend(index..index + c.len_utf8());
}
Self {
text: shadow,
origin,
}
}
/// The span of `text` a shadow match `start..end` covers: from its first
/// visible character through its last, invisible characters between them
/// included.
fn original_span(&self, text_len: usize, start: usize, end: usize) -> (usize, usize) {
let from = self.origin.get(start).copied().unwrap_or(text_len);
let to = match end.checked_sub(1).and_then(|last| self.origin.get(last)) {
Some(last) if end > start => last + 1,
_ => from,
};
(from, to)
}
}
/// Compile-time list of redaction patterns. Order is intentional:
/// more-specific patterns first. False positives are acceptable —
/// better to redact a stray hash than to leak a credential.
const BUILTIN_PATTERNS: &[(&str, &str)] = &[
// Private keys run first, while their base64 body lines are still intact
// for the token rules below to leave alone.
//
// A complete PEM or PGP armor block (`… PRIVATE KEY BLOCK-----` is the PGP
// form) — multi-line, lazy match.
(
r"(?s)-----BEGIN [A-Z0-9 ]*PRIVATE KEY(?: BLOCK)?-----.*?-----END [A-Z0-9 ]*PRIVATE KEY(?: BLOCK)?-----",
"private_key",
),
// A block with no end line: the head of a key file, or a tool result cut
// short before the hook saw it. It needs at least one key-body line (16+
// base64 characters filling the rest of the line) after the header,
// optionally behind armor headers (`Proc-Type:`, `Version:`, …) and blank
// lines, so prose that only names the header keeps it. Lines break on a
// real newline or a JSON-escaped one. The line end after the last body
// line sits outside the `secret` group: it has to be there, but it is not
// redacted.
(
r#"(?P<secret>-----BEGIN [A-Z0-9 ]*PRIVATE KEY(?: BLOCK)?-----(?:(?:\r?\n|(?:\\r)?\\n)[ \t]*(?:(?:Proc-Type|DEK-Info|Version|Comment|Hash):[^\r\n\\]*)?)*?(?:(?:\r?\n|(?:\\r)?\\n)[ \t]*[A-Za-z0-9+/=]{16,}[ \t]*)+)(?:\r?(?m:$)|["'\\…]|\.\.\.)"#,
"private_key",
),
// Bearer-style tokens.
(r#"(?i)bearer\s+[A-Za-z0-9._\-+/=]{16,}"#, "bearer_token"),
// Vendor-prefixed API keys.
@@ -148,11 +266,6 @@ const BUILTIN_PATTERNS: &[(&str, &str)] = &[
// credential from a short encoded fragment; config that long is pasted as
// JSON, not base64.
(r"eyJ[A-Za-z0-9_\-+/]{40,}={0,2}", "base64_json_token"),
// PEM private key blocks — multi-line, lazy match.
(
r"(?s)-----BEGIN [A-Z ]*PRIVATE KEY-----.*?-----END [A-Z ]*PRIVATE KEY-----",
"private_key",
),
// URL-embedded credentials: scheme://user:pass@host.
(
r"[a-zA-Z][a-zA-Z0-9+\-.]*://[^:/\s]+:[^@\s]+@[^\s]+",
@@ -238,11 +351,23 @@ pub struct Sanitizer {
}
struct SanitizerInner {
patterns: Vec<(Regex, &'static str)>,
patterns: Vec<Pattern>,
allowlist: Vec<String>,
escapes: Regex,
}
struct Pattern {
re: Regex,
label: &'static str,
/// A built-in rule whose match must include context that is not itself
/// secret (a line end) redacts only its `secret` capture group.
secret_group: bool,
}
/// The one label the allowlist never exempts: no project codename belongs
/// inside a private key, and a short entry would otherwise match key bodies.
const PRIVATE_KEY_LABEL: &str = "private_key";
impl std::fmt::Debug for Sanitizer {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("Sanitizer")
@@ -277,15 +402,32 @@ impl Sanitizer {
pub fn new(cfg: &SanitizeConfig) -> Result<Self, regex::Error> {
let mut patterns = Vec::with_capacity(BUILTIN_PATTERNS.len() + cfg.extra_patterns.len());
for (p, label) in BUILTIN_PATTERNS {
patterns.push((Regex::new(p)?, *label));
let re = Regex::new(p)?;
let secret_group = re.capture_names().any(|name| name == Some("secret"));
patterns.push(Pattern {
re,
label: *label,
secret_group,
});
}
for p in &cfg.extra_patterns {
patterns.push((Regex::new(p)?, "custom"));
patterns.push(Pattern {
re: Regex::new(p)?,
label: "custom",
secret_group: false,
});
}
Ok(Self {
inner: Arc::new(SanitizerInner {
patterns,
allowlist: cfg.allowlist.clone(),
// A blank entry is a substring of every match and would turn
// redaction off entirely. `Config::load` warns about it.
allowlist: cfg
.allowlist
.iter()
.filter(|entry| !entry.trim().is_empty())
.cloned()
.collect(),
escapes: Regex::new(ESCAPE_SEQUENCES)?,
}),
})
@@ -306,39 +448,118 @@ impl Sanitizer {
// Before the redaction passes: an escape inside a secret would
// otherwise split it out of reach of every pattern below.
let mut out = self.strip_control(input);
for (re, label) in &self.inner.patterns {
out = re
.replace_all(&out, |caps: &regex::Captures<'_>| {
let m = caps.get(0).map(|m| m.as_str()).unwrap_or_default();
if self.inner.allowlist.iter().any(|a| m.contains(a)) {
m.to_string()
} else {
debug!(pattern = re.as_str(), "sanitize: redacted match");
format!("[REDACTED:{label}]")
}
})
.into_owned();
for pattern in &self.inner.patterns {
// When invisible characters are present, the pass that looks
// through them goes first, so a split secret is redacted whole
// rather than losing only the part before the split. The plain
// pass then still finds a match that needs an invisible character
// as its word boundary.
if out.contains(is_default_ignorable) {
let shadow = Shadow::new(&out);
out = self.redact(&out, Some(&shadow), pattern);
}
out = self.redact(&out, None, pattern);
}
out
}
/// Drop terminal escape sequences and the control characters that have
/// no place in a page.
/// One pattern over `text`, or over `shadow` with each match mapped back
/// to the span of `text` it covers.
fn redact(&self, text: &str, shadow: Option<&Shadow>, pattern: &Pattern) -> String {
let view = shadow.map_or(text, |shadow| shadow.text.as_str());
let spans: Vec<regex::Match<'_>> = if pattern.secret_group {
pattern
.re
.captures_iter(view)
.filter_map(|caps| caps.name("secret").or_else(|| caps.get(0)))
.collect()
} else {
pattern.re.find_iter(view).collect()
};
let mut out = String::new();
let mut copied = 0;
let mut redacted = false;
for m in spans {
if self.allowlisted(pattern.label, m.as_str()) {
continue;
}
let (start, end) = match shadow {
Some(shadow) => shadow.original_span(text.len(), m.start(), m.end()),
None => (m.start(), m.end()),
};
if start < copied {
continue;
}
debug!(pattern = pattern.re.as_str(), "sanitize: redacted match");
out.push_str(&text[copied..start]);
out.push_str("[REDACTED:");
out.push_str(pattern.label);
out.push(']');
copied = end;
redacted = true;
}
if !redacted {
return text.to_string();
}
out.push_str(&text[copied..]);
out
}
/// Whether an allowlist entry exempts `matched`. Compared with invisible
/// characters removed, so they neither break nor fake an entry.
fn allowlisted(&self, label: &str, matched: &str) -> bool {
if label == PRIVATE_KEY_LABEL || self.inner.allowlist.is_empty() {
return false;
}
let visible: String;
let matched = if matched.contains(is_default_ignorable) {
visible = matched
.chars()
.filter(|c| !is_default_ignorable(*c))
.collect();
visible.as_str()
} else {
matched
};
self.inner
.allowlist
.iter()
.any(|entry| matched.contains(entry.as_str()))
}
/// Drop terminal escape sequences, the control characters that have no
/// place in a page, and characters that hide text.
///
/// Captured text is replayed to a terminal by `ai-memory read-page` and
/// `search`, where an escape rewrites the screen or the window title, and
/// a bidi override reverses what the reader sees. A NUL additionally
/// makes the markdown file binary, which costs it `grep` and git diffs.
/// Tag characters spell out text no reader sees; the ones that finish an
/// emoji flag (England, Scotland, Wales) stay.
fn strip_control(&self, input: &str) -> String {
let stripped = self.inner.escapes.replace_all(input, "");
if stripped.contains(is_stripped_control) {
stripped
.chars()
.filter(|c| !is_stripped_control(*c))
.collect()
} else {
stripped.into_owned()
if !stripped
.char_indices()
.any(|(offset, c)| is_stripped_control(c) || is_hidden_text_char(offset, c))
{
return stripped.into_owned();
}
let mut out = String::with_capacity(stripped.len());
let mut offset = 0;
while let Some(c) = stripped[offset..].chars().next() {
let next = offset + c.len_utf8();
if c == '\u{1f3f4}' {
let tags = emoji_tag_run_len(&stripped[next..]);
out.push_str(&stripped[offset..next + tags]);
offset = next + tags;
continue;
}
if !is_stripped_control(c) && !is_hidden_text_char(offset, c) {
out.push(c);
}
offset = next;
}
out
}
}
@@ -1195,6 +1416,189 @@ mod tests {
);
}
/// One line of a fake key body. Short on purpose: a complete fake block
/// stays under the length the secret scanner's private-key rule needs.
const FAKE_KEY_LINE: &str = "b3BlbnNzaC1rZXktdjEAAAAA";
/// The vectors the client companion's scrub runs too
/// (`companions/ai-memory-client`), so the two copies cannot drift.
#[test]
fn shared_sanitizer_vectors_hold() {
let vectors: serde_json::Value = serde_json::from_str(include_str!(
"../../../tests/fixtures/sanitizer_vectors.json"
))
.unwrap();
let strings = |key: &str| -> Vec<String> {
vectors[key]
.as_array()
.unwrap()
.iter()
.map(|value| value.as_str().unwrap().to_string())
.collect()
};
for secret in strings("credentials") {
let (head, tail) = secret.split_at(secret.len() / 2);
for invisible in strings("invisibles") {
let out = s().scrub(&format!("key {head}{invisible}{tail} end"));
assert!(
out.contains("[REDACTED:") && !out.contains(head) && !out.contains(tail),
"{invisible:?} inside {secret}: {out:?}"
);
assert!(out.starts_with("key ") && out.ends_with(" end"), "{out:?}");
}
}
let body_line = vectors["key_body_line"].as_str().unwrap();
for block in vectors["key_blocks"].as_array().unwrap() {
let input = block["input"].as_str().unwrap();
let out = s().scrub(input);
assert!(
out.contains("[REDACTED:private_key]") && !out.contains(body_line),
"{input:?} -> {out:?}"
);
for kept in block["kept"].as_array().unwrap() {
assert!(out.contains(kept.as_str().unwrap()), "{input:?} -> {out:?}");
}
}
for text in strings("unchanged") {
assert_eq!(s().scrub(&text), text);
}
}
#[test]
fn invisible_character_inside_an_operator_pattern_match_is_redacted() {
let sn = Sanitizer::new(&SanitizeConfig {
extra_patterns: vec![r"CANARY-[0-9]{6}".to_string()],
allowlist: vec![],
})
.unwrap();
assert_eq!(
sn.scrub("found CANARY-12\u{200b}3456 here"),
"found [REDACTED:custom] here"
);
}
/// An invisible character neither breaks an allowlist entry (the span is
/// still exempt) nor lets a secret that is not allowlisted through.
#[test]
fn invisible_characters_neither_break_nor_bypass_the_allowlist() {
let sn = Sanitizer::new(&SanitizeConfig {
extra_patterns: vec![],
allowlist: vec!["KEEP_ME".to_string()],
})
.unwrap();
for kept in [
"SERVICE_TOKEN=KEEP_ME_visible_value",
"SERVICE_TOKEN=KEEP\u{200b}_ME_visible_value",
] {
assert_eq!(sn.scrub(kept), kept, "allowlisted span must survive");
}
let out = sn.scrub("SERVICE_TOKEN=s3cretva\u{200b}lue0123");
assert!(
out.contains("[REDACTED:env_secret]") && !out.contains("lue0123"),
"a secret without the allowlist entry must still be redacted: {out:?}"
);
}
#[test]
fn characters_that_hide_text_are_stripped() {
let hidden: String = "hidden note"
.chars()
.map(|c| char::from_u32(0xe0000 + u32::from(c)).unwrap())
.collect();
let out = s().scrub(&format!("visible{hidden} text"));
assert_eq!(out, "visible text");
assert_eq!(s().scrub("a\u{2062}b\u{feff}c"), "abc");
// A flag start with no cancel tag is not a flag: its tags go.
assert_eq!(s().scrub("\u{1f3f4}\u{e0067}\u{e0062} x"), "\u{1f3f4} x");
}
#[test]
fn pgp_private_key_block_is_redacted() {
let block = format!(
"before\n-----BEGIN PGP PRIVATE KEY BLOCK-----\nVersion: 1\n\n\
{FAKE_KEY_LINE}\n-----END PGP PRIVATE KEY BLOCK-----\nafter"
);
assert_eq!(s().scrub(&block), "before\n[REDACTED:private_key]\nafter");
}
#[test]
fn private_key_block_without_an_end_line_is_redacted() {
let line = FAKE_KEY_LINE;
for (input, expected) in [
(
format!("$ head -3 key.pem\n-----BEGIN PRIVATE KEY-----\n{line}\n{line}\n$ ls"),
"$ head -3 key.pem\n[REDACTED:private_key]\n$ ls".to_string(),
),
(
format!("-----BEGIN RSA PRIVATE KEY-----\r\n{line}\r\n{line}\r\n"),
"[REDACTED:private_key]\r\n".to_string(),
),
(
format!(r#"{{"stdout":"-----BEGIN OPENSSH PRIVATE KEY-----\n{line}\n{line}"}}"#),
r#"{"stdout":"[REDACTED:private_key]"}"#.to_string(),
),
(
format!(
" -----BEGIN EC PRIVATE KEY-----\n {line}\n {}",
&line[..20]
),
" [REDACTED:private_key]".to_string(),
),
(
format!(
"-----BEGIN RSA PRIVATE KEY-----\nProc-Type: 4,ENCRYPTED\n\
DEK-Info: AES-128-CBC,0123456789ABCDEF\n\n{line}\n"
),
"[REDACTED:private_key]\n".to_string(),
),
] {
assert_eq!(s().scrub(&input), expected, "input: {input:?}");
}
}
/// A key body line can contain `eyJ…`, which a token rule would redact
/// in part, leaving the rest of the key unrecognizable to the key rules.
#[test]
fn key_rules_run_before_token_rules_can_split_a_key_body() {
let first = format!("MIIEeyJ{}", "A".repeat(50));
let input = format!("-----BEGIN PRIVATE KEY-----\n{first}\n{FAKE_KEY_LINE}\n");
assert_eq!(s().scrub(&input), "[REDACTED:private_key]\n");
}
#[test]
fn allowlist_never_exempts_a_private_key() {
let sn = Sanitizer::new(&SanitizeConfig {
extra_patterns: vec![],
allowlist: vec!["b3Bl".to_string()],
})
.unwrap();
let complete =
format!("-----BEGIN PRIVATE KEY-----\n{FAKE_KEY_LINE}\n-----END PRIVATE KEY-----");
assert_eq!(sn.scrub(&complete), "[REDACTED:private_key]");
let unterminated = format!("-----BEGIN PRIVATE KEY-----\n{FAKE_KEY_LINE}\n");
assert_eq!(sn.scrub(&unterminated), "[REDACTED:private_key]\n");
}
#[test]
fn blank_allowlist_entries_are_ignored() {
let input = "token=ghp_FAKEfakeFAKEfakeFAKEfake012345678";
let sn = Sanitizer::new(&SanitizeConfig {
extra_patterns: vec![],
allowlist: vec![String::new(), " ".to_string()],
})
.unwrap();
assert_eq!(sn.scrub(input), "token=[REDACTED:github_token]");
// Control: a real entry still exempts its span.
let sn = Sanitizer::new(&SanitizeConfig {
extra_patterns: vec![],
allowlist: vec!["ghp_FAKE".to_string()],
})
.unwrap();
assert_eq!(sn.scrub(input), input);
}
/// JSON is the shape most captured tool payloads arrive in, and the
/// quote before the value used to put it outside the value class, so the
/// same secret was redacted in YAML and stored verbatim in JSON.
+42
View File
@@ -21015,6 +21015,48 @@ pub(crate) mod tests {
);
}
/// End to end through `/hook`: a credential with an invisible formatting
/// character inside it reaches the store redacted, in the title and the
/// body, and the prose around it is kept.
#[tokio::test]
async fn user_prompt_secret_split_by_an_invisible_character_is_redacted() {
let tmp = TempDir::new().unwrap();
let state = make_state(&tmp).await;
// FAKE shape: `sk-` plus 26 characters, split after the first 10.
let (head, tail) = ("sk-FAKEfakeFA", "KEfake0123456789");
let prompt = format!("use {head}\u{200B}{tail} for the call");
let sid = SessionId::new();
let env = HookEnvelope::from_query_and_body(
HookQuery {
event: "user-prompt".into(),
agent: Some("claude-code".into()),
..Default::default()
},
serde_json::json!({
"session_id": sid.to_string(),
"cwd": "/repo",
"prompt": prompt,
}),
);
process(&state, env, None, Vec::new()).await.unwrap();
let observations = state.reader.observations_for_session(sid).await.unwrap();
let obs = observations
.iter()
.find(|o| o.kind == ObservationKind::UserPrompt)
.expect("user prompt observation was recorded");
for stored in [&obs.title, &obs.body] {
assert!(
!stored.contains(head) && !stored.contains(tail),
"a fragment of the split key was stored: {stored:?}"
);
assert!(
stored.contains("[REDACTED:api_key]") && stored.contains("for the call"),
"the key must be redacted and the prose kept: {stored:?}"
);
}
}
/// Shared shape for the #1114 body-side regressions: a secret straddling
/// a per-event byte cap used to be cut in half at excerpt extraction,
/// before the sanitizer ever ran, so the surviving prefix was too short
+36
View File
@@ -7346,6 +7346,27 @@ fn select_bumpable(
}
impl AiMemoryServer {
/// Names of the tools annotated `destructive_hint = true`, sorted.
/// `install-mcp` gives each a per-tool approval prompt in clients that
/// support one, so a new destructive tool inherits it from its
/// annotation.
#[must_use]
pub fn destructive_tool_names() -> Vec<String> {
let mut names: Vec<String> = Self::tool_router()
.list_all()
.into_iter()
.filter(|tool| {
tool.annotations
.as_ref()
.and_then(|annotations| annotations.destructive_hint)
== Some(true)
})
.map(|tool| tool.name.to_string())
.collect();
names.sort();
names
}
/// The latest version id of a page in an already-authorized scope.
async fn latest_page_id(
&self,
@@ -8790,6 +8811,21 @@ mod tests {
let (_tmp, _store, _server, _ws, _pj) = setup_server().await;
}
#[test]
fn destructive_tool_names_follow_the_annotations() {
let names = AiMemoryServer::destructive_tool_names();
for destructive in [
"memory_delete_page",
"memory_forget_sweep",
"memory_message_pop",
] {
assert!(names.iter().any(|name| name == destructive), "{names:?}");
}
for read_only in ["memory_query", "memory_recent", "memory_status"] {
assert!(!names.iter().any(|name| name == read_only), "{names:?}");
}
}
#[tokio::test]
async fn prompts_cover_every_registered_mcp_tool() {
let (_tmp, _store, server, _ws, _pj) = setup_server().await;
+9
View File
@@ -1333,6 +1333,15 @@ docker run --rm akitaonrails/ai-memory:latest \
--auth-token "$TOKEN"
```
The Codex entry `install-mcp` writes sets `default_tools_approval_mode =
"approve"`, so ai-memory's read tools run without a prompt, and adds
`[mcp_servers.<name>.tools.<tool>]` with `approval_mode = "prompt"` for every
tool annotated destructive (`memory_delete_page`, `memory_forget_sweep`,
`memory_restore_page`, the handoff accept and cancel tools, the message pop and
cancel tools, and `memory_feedback`), so Codex asks before running them. A
re-install keeps any per-tool `approval_mode` you set yourself. Both keys are in
Codex's [configuration reference](https://developers.openai.com/codex/config-reference).
Native Codex tool hooks use top-level `tool_name`, `tool_input`, `tool_response`,
and `tool_use_id` fields (verified against CLI 0.154.0). ai-memory records the
tool family and call ID on `PreToolUse` and `PostToolUse`; recognized tools such
+27 -9
View File
@@ -24,7 +24,7 @@ on a homelab box where mistakes are harder to undo.
| `restore-page --path --from` | ✅ yes | overwrites one markdown page version | yes (restore another checkpoint) | Restores one page from wiki git history, reindexes it into SQLite, and writes a post-restore checkpoint. Does not restore DB-only state. |
| MCP `memory_restore_page` | ✅ yes | no (writes a new current version; the replaced one stays reachable) | yes (restore another checkpoint) | Lists a page's checkpoints, or writes one back through the `memory_write_page` path. Admin only, like `restore-page`. See [below](#mcp-memory_restore_page). |
| `backup-agents --to` / `restore-agents --from <tarball>` | ✅ yes (client-side; never touches the server or its data dir) | no | yes (`restore-agents --apply` keeps timestamped copies of overwritten files) | Agent-config assets, not wiki/db: MCP configs (redacted unless `--include-secrets`), hook configs, instructions, skills, plugins. `restore-agents` is a dry run by default; `--apply` writes to disk and `--force` overwrites existing or differing targets. Non-MCP assets are restored verbatim and may contain secrets, so only `--apply` an archive whose source you trust. |
| `restore --from <tarball>` | ❌ **stop the server first** | overwrites the data dir | no (without prior backup) | Refuses if any sibling `ai-memory` process is alive (sysinfo guard). Stages and verifies the archive before swapping it in, so a failed restore leaves `wiki/` and `db/` as they were. |
| `restore --from <tarball>` | ❌ **stop the server first** | overwrites the data dir | no (without prior backup) | Refuses if any sibling `ai-memory` process is alive (sysinfo guard). Stages and verifies the archive before swapping it in, so a failed restore leaves `wiki/` and `db/` as they were. Keeps an existing `config.toml` and saves the backup's as `config.toml.from-backup` unless `--with-config` is passed. |
| `reset --confirm` | ❌ **stop the server first** | yes, all data | no | Refuses if any sibling `ai-memory` process is alive (sysinfo guard). |
| `reindex` | ❌ **stop the server first** | no wiki wipe; requires a clean DB | only with prior DB backup | Rebuilds pages/links/FTS from `wiki/` using `_meta.md` manifests. Refuses if SQLite already has rows so stale DB-only state cannot survive silently. |
@@ -1146,7 +1146,11 @@ intentionally excludes `auth-tokens/`, the hook-side `auth-token` and
data dir with `--config`. The included `<data_dir>/config.toml` is archived
verbatim and may itself contain inline root/proxy/recovery credentials, the
API-key pepper, or sensitive LLM headers. Always protect the snapshot as a
secret-bearing artifact. Excluding the separate credential files limits the
secret-bearing artifact. It is also code-bearing: a config can name commands
the server runs (`[auto_improve.eval]`), send consolidated memory to another
host (LLM provider URLs and headers, admission webhooks) and weaken redaction
(`[sanitize]`), so `restore` adopts it only when asked (see below). Excluding
the separate credential files limits the
copied auth material; omitting `raw/` avoids racing the actively appended
managed-workstream ledger. The SQLite snapshot still contains the searchable
managed-workstream events, but loses their immutable source copy.
@@ -1194,7 +1198,7 @@ HTTP admin API).
### `restore`
```bash
ai-memory restore --from <tarball> --data-dir <path> --force
ai-memory restore --from <tarball> --data-dir <path> --force [--with-config]
```
Direct-disk operation. Refuses if any other `ai-memory` process is
@@ -1208,12 +1212,22 @@ Order of operations:
3. Open the staged store so pending migrations run and the SQLite
snapshot is verified, still without touching the live data. A legacy
database with API keys but no pepper verifier stops here unless the original
pepper is configured and `--accept-legacy-api-key-pepper` was passed.
4. Swap: rename the live `wiki/` and `db/` (and `config.toml`, when the
archive carries one) aside, rename the staged copies into place, then
delete the previous data. Each move is a same-filesystem rename; if
one fails, the moves already made are reversed.
5. Print a one-line summary.
pepper is configured and `--accept-legacy-api-key-pepper` was passed. When
the live config is being kept (step 4) and the database holds API keys, the
configured pepper must match the one the keys were hashed with, or the
restore stops here too.
4. Swap: rename the live `wiki/` and `db/` aside, rename the staged copies
into place, then delete the previous data. Each move is a same-filesystem
rename; if one fails, the moves already made are reversed. The archive's
`config.toml`, when it carries one, replaces the live config only with
`--with-config` or when the data dir has none. Otherwise the live config
stays and the archived one is saved as `<data_dir>/config.toml.from-backup`
(`0600` on Unix), replacing an earlier copy there.
5. Print a summary, including where the backup's config went.
A backup's `config.toml` can run commands and redirect data. Restore it, with
`--with-config` or by adopting `config.toml.from-backup`, only from an archive
you trust; review it first otherwise.
The live data is therefore untouched until the archive has proven usable.
A restore that fails in steps 2–3 leaves `wiki/` and `db/` exactly as they
@@ -1234,6 +1248,10 @@ Failure modes:
although a later one was applied. The staged store is refused, just as at
normal startup. Keep the backup and report the history for review;
`--force` does not bypass this check or repair the missing migration.
- **The live config's pepper does not match the backup's API keys** (live
config kept) → rerun with `--with-config` to restore the backup's config, or
set the live `[auth].token_pepper` to the original. The live data is left as
it was.
- **Legacy backup has API keys but no pepper verifier** → restore the original
`[auth].token_pepper` through the current config/environment and rerun with
`--accept-legacy-api-key-pepper`. This flag confirms provenance; it cannot
File diff suppressed because one or more lines are too long
+83
View File
@@ -0,0 +1,83 @@
{
"_comment": "Shared by the server sanitizer (crates/ai-memory-core/src/sanitize.rs) and the client scrub (companions/ai-memory-client/src/scrub.rs) so the two copies cannot drift. Every value is a FAKE shape.",
"credentials": [
"sk-leak-0123456789abcdef0123",
"ghp_FAKE0123456789ABCDEFGHIJ0123",
"AKIAFAKEFAKEFAKEFAKE"
],
"invisibles": [
"\u200b",
"\u200c",
"\u200d",
"\u200e",
"\u2060",
"\u2063",
"\ufeff",
"\u00ad",
"\u034f",
"\u115f",
"\u17b4",
"\u180e",
"\u3164",
"\ufe0f",
"\uffa0",
"\udb40\udc01",
"\udb40\udd00"
],
"key_body_line": "b3BlbnNzaC1rZXktdjEAAAAA",
"key_blocks": [
{
"input": "before\n-----BEGIN PRIVATE KEY-----\nb3BlbnNzaC1rZXktdjEAAAAA\n-----END PRIVATE KEY-----\nafter",
"kept": [
"before\n",
"\nafter"
]
},
{
"input": "before\n-----BEGIN PGP PRIVATE KEY BLOCK-----\nVersion: 1\n\nb3BlbnNzaC1rZXktdjEAAAAA\n-----END PGP PRIVATE KEY BLOCK-----\nafter",
"kept": [
"before\n",
"\nafter"
]
},
{
"input": "$ head -3 key.pem\n-----BEGIN PRIVATE KEY-----\nb3BlbnNzaC1rZXktdjEAAAAA\nb3BlbnNzaC1rZXktdjEAAAAA\n$ ls",
"kept": [
"$ head -3 key.pem\n",
"\n$ ls"
]
},
{
"input": "-----BEGIN RSA PRIVATE KEY-----\r\nb3BlbnNzaC1rZXktdjEAAAAA\r\nb3BlbnNzaC1rZXktdjEAAAAA\r\n",
"kept": [
"\r\n"
]
},
{
"input": "{\"stdout\":\"-----BEGIN OPENSSH PRIVATE KEY-----\\nb3BlbnNzaC1rZXktdjEAAAAA\\nb3BlbnNzaC1rZXktdjEAAAAA\"}",
"kept": [
"{\"stdout\":\"",
"\"}"
]
},
{
"input": "-----BEGIN RSA PRIVATE KEY-----\nProc-Type: 4,ENCRYPTED\nDEK-Info: AES-128-CBC,0123456789ABCDEF\n\nb3BlbnNzaC1rZXktdjEAAAAA\n",
"kept": []
}
],
"unchanged": [
"family \ud83d\udc68\u200d\ud83d\udc69\u200d\ud83d\udc67 emoji",
"keycaps 1\ufe0f\u20e3 #\ufe0f\u20e3",
"flag \ud83c\udff4\udb40\udc67\udb40\udc62\udb40\udc73\udb40\udc63\udb40\udc74\udb40\udc7f Scotland",
"\u0645\u06cc\u200c\u062e\u0648\u0627\u0647\u0645",
"\u0915\u094d\u200d\u0937",
"\u0e2a\u0e27\u0e31\u0e2a\u0e14\u0e35\u200b\u0e04\u0e23\u0e31\u0e1a",
"\u05e9\u05dc\u05d5\u05dd\u200f world",
"co\u00adoperate",
"\ufeffstarts with a byte-order mark",
"-----BEGIN PGP PUBLIC KEY BLOCK-----\n\nmQENBFakeFakeFakeFakeFakeFakeFake\n-----END PGP PUBLIC KEY BLOCK-----",
"The file starts with -----BEGIN OPENSSH PRIVATE KEY----- and that is expected.",
"-----BEGIN RSA PRIVATE KEY-----\nThat header marks a PKCS#1 key.",
"const HEADER: &str = \"-----BEGIN RSA PRIVATE KEY-----\";\nparseHeaderLineFromInput(x)"
]
}