Skip to content

Commit 298c58f

Browse files
committed
fix: redact working-memory sensitive text
1 parent 0c55f54 commit 298c58f

4 files changed

Lines changed: 249 additions & 14 deletions

File tree

docs/design-docs/working-memory-triage.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ Findings from CodeRabbit review + bug reports. Tracking resolution before merge.
1717
- [ ] **R3 — Don't exclude participant-role facts yet** (`prompts/en/cortex_knowledge_synthesis.md.j2:21`)
1818
Exclusion of "The user is the CEO" drops participant context with nowhere else to live until Phase 6 ships.
1919

20-
- [ ] **R4 — Raw worker task in working memory** (`src/agent/channel_dispatch.rs:596`)
21-
`task` from user input persisted verbatim; could capture secrets/PII. Truncate and scrub.
20+
- [x] **R4 — Raw worker task in working memory** (`src/agent/channel_dispatch.rs:596`)
21+
`task` from user input persisted verbatim; could capture secrets/PII. **Fixed in this slice:** worker-spawn working-memory text now uses shared secret redaction and char-safe truncation.
2222

2323
- [ ] **R5 — Dirty flag only bumps on merges** (`src/agent/cortex.rs:1958`)
2424
Prunes and decays also change the memory set but don't trigger knowledge synthesis re-gen. Add `report.pruned > 0 || report.decayed > 0`. **Partial in PR #570:** prunes and merges now dirty synthesis; decay remains intentionally importance-only and needs a follow-up decision.
@@ -44,8 +44,8 @@ Findings from CodeRabbit review + bug reports. Tracking resolution before merge.
4444
- [ ] **R12 — Silent error swallowing in inspect_prompt** (`src/api/channels.rs:649`)
4545
`unwrap_or_default()` / `.ok()` hides DB/template errors. Log and propagate per coding guidelines.
4646

47-
- [ ] **R13 — Raw error strings in working memory** (`src/cron/scheduler.rs:386`)
48-
Full error text persisted; could contain sensitive internals. Emit redacted summary only.
47+
- [x] **R13 — Raw error strings in working memory** (`src/cron/scheduler.rs:386`)
48+
Full error text persisted; could contain sensitive internals. **Fixed in this slice:** cron error working-memory text now uses the same redacted, bounded summary while tracing keeps the full error.
4949

5050
- [ ] **R14 — Timezone fallback drops valid `cron_timezone`** (`src/main.rs:2559`)
5151
If `user_timezone` is present but unparseable, `cron_timezone` is never tried. Parse each independently.

src/agent/channel_dispatch.rs

Lines changed: 57 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ enum WorkerCompletionKind {
4040
Failed,
4141
}
4242

43+
const WORKING_MEMORY_TASK_MAX_CHARS: usize = 500;
44+
4345
#[derive(Debug, Clone)]
4446
pub(crate) enum WorkerCompletionError {
4547
Cancelled { reason: String },
@@ -99,6 +101,14 @@ pub(crate) fn map_worker_completion_result(
99101
(result_text, notify, success)
100102
}
101103

104+
fn sanitize_worker_memory_task(task: &str, tool_secret_pairs: &[(String, String)]) -> String {
105+
crate::secrets::scrub::scrub_working_memory_text(
106+
task,
107+
tool_secret_pairs,
108+
WORKING_MEMORY_TASK_MAX_CHARS,
109+
)
110+
}
111+
102112
/// Build the worker status text (time + system info) used in worker system prompts.
103113
///
104114
/// Centralises the `SystemInfo` + `TemporalContext` assembly so every worker
@@ -597,9 +607,9 @@ async fn spawn_worker_inner(
597607
let sandbox_write_allowlist = state.deps.sandbox.prompt_write_allowlist();
598608
// Collect tool secret names so the worker template can list available credentials.
599609
let secrets_guard = rc.secrets.load();
600-
let tool_secret_names = match (*secrets_guard).as_ref() {
601-
Some(store) => store.tool_secret_names(),
602-
None => Vec::new(),
610+
let (tool_secret_names, tool_secret_pairs) = match (*secrets_guard).as_ref() {
611+
Some(store) => (store.tool_secret_names(), store.tool_secret_pairs()),
612+
None => (Vec::new(), Vec::new()),
603613
};
604614

605615
let browser_config = (**rc.browser_config.load()).clone();
@@ -793,12 +803,13 @@ async fn spawn_worker_inner(
793803
})
794804
.ok();
795805

806+
let memory_task = sanitize_worker_memory_task(task, &tool_secret_pairs);
796807
state
797808
.deps
798809
.working_memory
799810
.emit(
800811
crate::memory::WorkingMemoryEventType::WorkerSpawned,
801-
format!("Worker spawned: {task}"),
812+
format!("Worker spawned: {memory_task}"),
802813
)
803814
.channel(state.channel_id.to_string())
804815
.importance(0.6)
@@ -872,6 +883,9 @@ async fn spawn_opencode_worker_inner(
872883
let persist_directory = directory.clone();
873884

874885
let oc_secrets_store = state.deps.runtime_config.secrets.load().as_ref().clone();
886+
let oc_tool_secret_pairs = oc_secrets_store
887+
.as_ref()
888+
.map_or_else(Vec::new, |store| store.tool_secret_pairs());
875889

876890
// Build temporal/status context so OpenCode workers get the same system
877891
// info (time, model, context window) as builtin workers.
@@ -996,12 +1010,13 @@ async fn spawn_opencode_worker_inner(
9961010
})
9971011
.ok();
9981012

1013+
let memory_task = sanitize_worker_memory_task(task, &oc_tool_secret_pairs);
9991014
state
10001015
.deps
10011016
.working_memory
10021017
.emit(
10031018
crate::memory::WorkingMemoryEventType::WorkerSpawned,
1004-
format!("Worker spawned (opencode): {task}"),
1019+
format!("Worker spawned (opencode): {memory_task}"),
10051020
)
10061021
.channel(state.channel_id.to_string())
10071022
.importance(0.6)
@@ -1396,7 +1411,10 @@ fn expand_tilde(path: &str) -> std::path::PathBuf {
13961411

13971412
#[cfg(test)]
13981413
mod tests {
1399-
use super::{WorkerCompletionError, map_worker_completion_result, spawn_worker_task};
1414+
use super::{
1415+
WORKING_MEMORY_TASK_MAX_CHARS, WorkerCompletionError, map_worker_completion_result,
1416+
sanitize_worker_memory_task, spawn_worker_task,
1417+
};
14001418
use crate::{ProcessEvent, WorkerId};
14011419
use std::sync::Arc;
14021420
use std::time::Duration;
@@ -1414,6 +1432,39 @@ mod tests {
14141432
assert!(!success);
14151433
}
14161434

1435+
#[test]
1436+
fn worker_spawned_memory_task_redacts_secrets() {
1437+
let tool_secret_pairs = vec![("API_KEY".to_string(), "stored-secret".to_string())];
1438+
let task = "use stored-secret and sk-ant-abc123456789012345678";
1439+
let result = sanitize_worker_memory_task(task, &tool_secret_pairs);
1440+
1441+
assert!(
1442+
!result.contains("stored-secret"),
1443+
"stored secret should be redacted in: {result}"
1444+
);
1445+
assert!(
1446+
!result.contains("sk-ant-"),
1447+
"leak pattern should be redacted in: {result}"
1448+
);
1449+
assert!(
1450+
result.contains("[REDACTED:API_KEY]"),
1451+
"stored secret marker missing in: {result}"
1452+
);
1453+
assert!(
1454+
result.contains("[LEAKED_SECRET_REDACTED]"),
1455+
"leak marker missing in: {result}"
1456+
);
1457+
}
1458+
1459+
#[test]
1460+
fn worker_spawned_memory_task_is_bounded() {
1461+
let task = "a".repeat(WORKING_MEMORY_TASK_MAX_CHARS + 100);
1462+
let result = sanitize_worker_memory_task(&task, &[]);
1463+
1464+
assert_eq!(result.chars().count(), WORKING_MEMORY_TASK_MAX_CHARS);
1465+
assert!(result.ends_with(" ... [truncated]"));
1466+
}
1467+
14171468
#[tokio::test]
14181469
async fn spawn_worker_task_emits_cancelled_completion_event() {
14191470
let (event_tx, mut event_rx) = broadcast::channel(8);

src/cron/scheduler.rs

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ pub struct CronContext {
130130
}
131131

132132
const MAX_CONSECUTIVE_FAILURES: u32 = 3;
133+
const WORKING_MEMORY_CRON_ERROR_MAX_CHARS: usize = 500;
133134

134135
/// RAII guard that clears an `AtomicBool` on drop, ensuring the flag is
135136
/// released even if the holding task panics.
@@ -153,7 +154,12 @@ fn emit_cron_error(
153154
failure_class: &'static str,
154155
error: &crate::error::Error,
155156
) {
156-
let message = format!("Cron {failure_class}: {job_id}: {error}");
157+
let secrets_guard = context.deps.runtime_config.secrets.load();
158+
let tool_secret_pairs = match (*secrets_guard).as_ref() {
159+
Some(store) => store.tool_secret_pairs(),
160+
None => Vec::new(),
161+
};
162+
let message = cron_error_memory_message(job_id, failure_class, error, &tool_secret_pairs);
157163

158164
// Emit to working memory for agent context awareness
159165
context
@@ -167,6 +173,19 @@ fn emit_cron_error(
167173
tracing::error!(cron_id = %job_id, failure_class, %error, "cron job execution failed");
168174
}
169175

176+
fn cron_error_memory_message(
177+
job_id: &str,
178+
failure_class: &'static str,
179+
error: &crate::error::Error,
180+
tool_secret_pairs: &[(String, String)],
181+
) -> String {
182+
crate::secrets::scrub::scrub_working_memory_text(
183+
&format!("Cron {failure_class}: {job_id}: {error}"),
184+
tool_secret_pairs,
185+
WORKING_MEMORY_CRON_ERROR_MAX_CHARS,
186+
)
187+
}
188+
170189
#[derive(Debug)]
171190
enum CronRunError {
172191
Execution(crate::error::Error),
@@ -1730,9 +1749,11 @@ fn normalize_cron_delivery_response(response: OutboundResponse) -> Option<Outbou
17301749
#[cfg(test)]
17311750
mod tests {
17321751
use super::{
1733-
CronConfig, CronJob, CronResponseWaitOutcome, CronRunError, await_cron_delivery_response,
1734-
cron_response_summary, hour_in_active_window, normalize_active_hours,
1735-
normalize_cron_delivery_response, set_job_enabled_state, sync_job_from_store,
1752+
CronConfig, CronJob, CronResponseWaitOutcome, CronRunError,
1753+
WORKING_MEMORY_CRON_ERROR_MAX_CHARS, await_cron_delivery_response,
1754+
cron_error_memory_message, cron_response_summary, hour_in_active_window,
1755+
normalize_active_hours, normalize_cron_delivery_response, set_job_enabled_state,
1756+
sync_job_from_store,
17361757
};
17371758
use crate::cron::store::CronStore;
17381759
use crate::messaging::target::parse_delivery_target;
@@ -1998,6 +2019,43 @@ mod tests {
19982019
assert_eq!(execution_error.failure_class(), "execution_error");
19992020
}
20002021

2022+
#[test]
2023+
fn cron_error_memory_message_redacts_and_bounds_error_text() {
2024+
let tool_secret_pairs = vec![("API_KEY".to_string(), "stored-secret".to_string())];
2025+
let error = crate::error::Error::Other(anyhow::anyhow!(
2026+
"{} {} {}",
2027+
"stored-secret",
2028+
"sk-ant-abc123456789012345678",
2029+
"x".repeat(WORKING_MEMORY_CRON_ERROR_MAX_CHARS)
2030+
));
2031+
2032+
let message = cron_error_memory_message(
2033+
"daily-digest",
2034+
"execution_error",
2035+
&error,
2036+
&tool_secret_pairs,
2037+
);
2038+
2039+
assert!(
2040+
!message.contains("stored-secret"),
2041+
"stored secret should be redacted in: {message}"
2042+
);
2043+
assert!(
2044+
!message.contains("sk-ant-"),
2045+
"leak pattern should be redacted in: {message}"
2046+
);
2047+
assert!(
2048+
message.contains("[REDACTED:API_KEY]"),
2049+
"stored secret marker missing in: {message}"
2050+
);
2051+
assert!(
2052+
message.contains("[LEAKED_SECRET_REDACTED]"),
2053+
"leak marker missing in: {message}"
2054+
);
2055+
assert_eq!(message.chars().count(), WORKING_MEMORY_CRON_ERROR_MAX_CHARS);
2056+
assert!(message.ends_with(" ... [truncated]"));
2057+
}
2058+
20012059
#[tokio::test]
20022060
async fn set_job_enabled_state_updates_in_memory_flag() {
20032061
let jobs = Arc::new(RwLock::new(HashMap::from([(

src/secrets/scrub.rs

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,48 @@ pub fn scrub_leaks(content: &str) -> String {
254254
result
255255
}
256256

257+
/// Scrub and bound text before storing it in working memory.
258+
///
259+
/// Working memory is replayed into future LLM context, so it must not persist
260+
/// exact tool secrets, known plaintext leak patterns, or unbounded user/error
261+
/// payloads.
262+
pub fn scrub_working_memory_text(
263+
text: &str,
264+
tool_secrets: &[(String, String)],
265+
max_chars: usize,
266+
) -> String {
267+
let scrubbed = scrub_secrets(text, tool_secrets);
268+
let scrubbed = scrub_leaks(&scrubbed);
269+
let scrubbed = redact_working_memory_encoded_leaks(&scrubbed);
270+
truncate_for_working_memory(&scrubbed, max_chars)
271+
}
272+
273+
fn redact_working_memory_encoded_leaks(text: &str) -> String {
274+
if scan_for_leaks(text).is_some() {
275+
return "[WORKING_MEMORY_REDACTED:encoded-secret]".to_string();
276+
}
277+
278+
text.to_string()
279+
}
280+
281+
fn truncate_for_working_memory(text: &str, max_chars: usize) -> String {
282+
const TRUNCATED_SUFFIX: &str = " ... [truncated]";
283+
284+
if text.chars().count() <= max_chars {
285+
return text.to_string();
286+
}
287+
288+
let suffix_len = TRUNCATED_SUFFIX.chars().count();
289+
if max_chars <= suffix_len {
290+
return TRUNCATED_SUFFIX.chars().take(max_chars).collect();
291+
}
292+
293+
let kept_chars = max_chars - suffix_len;
294+
let mut result: String = text.chars().take(kept_chars).collect();
295+
result.push_str(TRUNCATED_SUFFIX);
296+
result
297+
}
298+
257299
#[cfg(test)]
258300
mod tests {
259301
use super::*;
@@ -380,4 +422,88 @@ mod tests {
380422
"surrounding text should be preserved in: {result}"
381423
);
382424
}
425+
426+
#[test]
427+
fn scrub_working_memory_text_redacts_exact_and_pattern_secrets() {
428+
let tool_secrets = vec![("API_KEY".to_string(), "stored-secret".to_string())];
429+
let input = "stored-secret and sk-ant-abc123456789012345678";
430+
let result = scrub_working_memory_text(input, &tool_secrets, 200);
431+
432+
assert!(
433+
!result.contains("stored-secret"),
434+
"stored secret should be redacted in: {result}"
435+
);
436+
assert!(
437+
!result.contains("sk-ant-"),
438+
"leak pattern should be redacted in: {result}"
439+
);
440+
assert!(
441+
result.contains("[REDACTED:API_KEY]"),
442+
"exact redaction marker missing in: {result}"
443+
);
444+
assert!(
445+
result.contains("[LEAKED_SECRET_REDACTED]"),
446+
"leak redaction marker missing in: {result}"
447+
);
448+
}
449+
450+
#[test]
451+
fn scrub_working_memory_text_truncates_on_character_boundaries() {
452+
let input = "é".repeat(100);
453+
let result = scrub_working_memory_text(&input, &[], 20);
454+
455+
assert_eq!(result.chars().count(), 20);
456+
assert!(result.ends_with(" ... [truncated]"));
457+
assert!(std::str::from_utf8(result.as_bytes()).is_ok());
458+
}
459+
460+
#[test]
461+
fn scrub_working_memory_text_fails_closed_for_url_encoded_secret() {
462+
let secret = "sk-ant-abc123456789012345678";
463+
let input = "worker task has sk%2Dant%2Dabc123456789012345678 and context";
464+
let result = scrub_working_memory_text(input, &[], 200);
465+
466+
assert_eq!(result, "[WORKING_MEMORY_REDACTED:encoded-secret]");
467+
assert!(
468+
!result.contains(secret),
469+
"decoded secret should not appear in: {result}"
470+
);
471+
assert!(
472+
!result.contains("sk%2Dant"),
473+
"encoded secret should not appear in: {result}"
474+
);
475+
assert!(scan_for_leaks(&result).is_none());
476+
}
477+
478+
#[test]
479+
fn scrub_working_memory_text_fails_closed_for_base64_encoded_secret() {
480+
use base64::Engine as _;
481+
482+
let secret = "sk-ant-abc123456789012345678";
483+
let encoded = base64::engine::general_purpose::STANDARD.encode(secret);
484+
let input = format!("cron error included {encoded}");
485+
let result = scrub_working_memory_text(&input, &[], 200);
486+
487+
assert_eq!(result, "[WORKING_MEMORY_REDACTED:encoded-secret]");
488+
assert!(
489+
!result.contains(&encoded),
490+
"encoded secret should not appear in: {result}"
491+
);
492+
assert!(scan_for_leaks(&result).is_none());
493+
}
494+
495+
#[test]
496+
fn scrub_working_memory_text_fails_closed_for_hex_encoded_secret() {
497+
let secret = "sk-ant-abc123456789012345678";
498+
let encoded = hex::encode(secret);
499+
let input = format!("cron error included {encoded}");
500+
let result = scrub_working_memory_text(&input, &[], 200);
501+
502+
assert_eq!(result, "[WORKING_MEMORY_REDACTED:encoded-secret]");
503+
assert!(
504+
!result.contains(&encoded),
505+
"encoded secret should not appear in: {result}"
506+
);
507+
assert!(scan_for_leaks(&result).is_none());
508+
}
383509
}

0 commit comments

Comments
 (0)