actions: keep a running card's claim; forget only the value it was about

crates/cli/skill/SKILL.md

@@ -95,7 +95,7 @@ Some commands act for the user. Run them only when the user asks, and pass `--wh
 | Replace a key that still works | `tokenstash rotate NAME` |
 | Delete a stored key | `tokenstash forget NAME` |
 | Always use another identity for a key in this project | `tokenstash bind NAME --identity work` |
-| Ask again for a key they declined | `tokenstash need NAME --force` (once per key until the "no" expires) |
+| Ask again for a key they declined | `tokenstash need NAME --force` (once per key and identity until the "no" expires) |
 | Stop agents loading this skill on their own, or let them again | `tokenstash init --mode explicit`, `tokenstash init --mode auto` |
 | Register or remove the MCP server | `tokenstash init --mcp`, `tokenstash init --no-mcp` |
 | Take tokenstash out of their agents | `tokenstash init --undo` |

crates/cli/skill/reference.md

@@ -30,7 +30,7 @@ Writes each key to the project's env file, or files a card for it. The project i
 | `--blocking` | Wait for the user instead of returning at once |
 | `--timeout SECONDS` | With `--blocking`, how long to wait (default 600) |
 | `--agent NAME` | Your name on the card and in the audit log (detected otherwise) |
-| `--force` | Ask again after the user declined. Once per key and project until the "no" expires; the card says it is a second ask |
+| `--force` | Ask again after the user declined. Once per key, identity and project until the "no" expires; the card says it is a second ask, and the same command run again while that card waits returns it |
 | `--json` | One JSON object instead of lines |
 
 Generated secrets (`AUTH_SECRET`, `JWT_SECRET`, `SESSION_SECRET`, `NEXTAUTH_SECRET`, `ENCRYPTION_KEY`) are never asked for: tokenstash generates one per project, or keeps the value the env file already holds.
@@ -95,7 +95,9 @@ Runs COMMAND with the env file loaded into its environment. Its output reaches y
 
 ### `tokenstash doctor`
 
-Checks the setup, one line each: config file, stash backend (and how long keys last in it), database, provider registry, inbox, agent mode, the skill and any MCP registration per agent, this directory (refused, paired, or not yet), and the binary path. Exit 0 when nothing is wrong, 1 otherwise. Safe to run at any time.
+Checks the setup, one line each: config file, stash backend (and how long keys last in it), older copies (Linux kernel keyring only), database, provider registry, inbox, agent mode, the skill and any MCP registration per agent, this directory (refused, paired, or not yet), and the binary path. Exit 0 when nothing is wrong, 1 otherwise. Safe to run at any time.
+
+The older copies line looks at every key this home's index lists and every key the kernel keyring holds for this home, for copies a tokenstash older than 0.4 keeps. It fails when a key's user keyring and persistent keyring hold different values, because a read can then return the old key. It passes with a note when only another login session holds a copy, which matters only while an older tokenstash runs in that session. When `/proc/keys` cannot be read, it says so and looks only at the keys the index lists.
 
 ### `tokenstash remote [tailscale [--login LOGIN] | off]`
 

crates/cli/skill/troubleshooting.md

@@ -47,7 +47,7 @@ Unless remote access is on, the inbox listens on `127.0.0.1` of the machine toke
 
 Follow "When a provider rejects a key" in `SKILL.md`: `tokenstash report-bad NAME --status 401`, then `tokenstash need NAME`. If the second `need` writes the key again, the provider accepted it on re-check: compare your request with the provider's documentation (auth header name and format, base URL, project or organization header).
 
-If the user pasted a new key and the old, rejected one keeps coming back, run `tokenstash doctor` and look at the stash backend line. With `keyutils` (the Linux kernel keyring, used when no Secret Service runs), versions before 0.4 kept a separate copy per login session, so a key pasted from one session could be shadowed in another; `doctor`'s last line shows which binary runs. Tell the user what you found.
+If the user pasted a new key and the old, rejected one keeps coming back, run `tokenstash doctor` and look at the stash backend line. With `keyutils` (the Linux kernel keyring, used when no Secret Service runs), versions before 0.4 kept a separate copy per login session, so a key pasted from one session could be shadowed in another; `doctor`'s last line shows which binary runs. An older tokenstash still running in another login session (an inbox or `tokenstash mcp` started before the upgrade) keeps doing this, and this version cannot fully prevent it. No other session can write the copy that process holds. When the process links its copy into the shared keyring, the kernel does not record whether it read an old value or wrote a new one, so this version keeps the value it had, and a key pasted through the old process can be passed over. The `older copies` line of `doctor` then names the keys. It fails when a key's user keyring and persistent keyring hold different values, which is when a read can return the old key. When another login session only holds a copy, the line passes, since the copy matters only while an older tokenstash runs in that session. Tell the user what you found. They stop the older process, and if the old key is still the one in use, replace it with `tokenstash rotate NAME`.
 
 ## `need` exits 1
 

crates/cli/src/cmd/actions.rs

@@ -7,7 +7,7 @@ use crate::notify;
 use crate::util::{self, App};
 use anyhow::{bail, Result};
 use std::path::Path;
-use tokenstash_core::actions::{self, Action};
+use tokenstash_core::actions::{self, Action, Forgot};
 use tokenstash_core::config::AgentMode;
 use tokenstash_core::db::Task;
 
@@ -32,9 +32,10 @@ pub fn print_pending(app: &App, project: &Path, agent: &str, t: &Task) -> Result
 
 /// Person side, from the inbox's full session or the person's terminal: claim the card, do
 /// what it says, then close it. The claim means a second confirm (another tab, a double click)
-/// runs nothing: an old forget card confirmed again would delete a key stored since. The card
-/// stays pending until the action has run, so if this process stops half way the claim runs
-/// out and the person can confirm it again; if the action fails, the claim is given back.
+/// runs nothing: an old forget card confirmed again would delete a key stored since. The claim
+/// is renewed while the action runs, so a slow one keeps its card however long it takes. The
+/// card stays pending until the action has run, so if this process stops half way the claim
+/// runs out and the person can confirm it again; if the action fails, the claim is given back.
 pub fn confirm(app: &App, task: &Task, action: &Action) -> Result<String> {
     use tokenstash_core::db::TaskStatus;
     let Some(claim) = app.db.claim_action(&task.id)? else {
@@ -43,11 +44,16 @@ pub fn confirm(app: &App, task: &Task, action: &Action) -> Result<String> {
             _ => bail!("this card was already answered"),
         }
     };
-    match perform(app, task, action) {
+    // A keeper that cannot start gives the claim back before it returns the error.
+    let kept = app.db.keep_claim(&task.id, &claim)?;
+    let ran = perform(app, task, action, &claim);
+    drop(kept);
+    match ran {
         Ok(done) => {
             if !app.db.finish_action(&task.id, &claim, &done)? {
-                // Ran past the claim's time and another confirm took over: the change is made,
-                // and the card is the other confirm's to close.
+                // The claim went unrenewed for its whole time while this ran (the machine
+                // slept, or the index stayed locked) and another confirm took over: the change
+                // is made, and the card is the other confirm's to close.
                 return Ok(format!("{done} (another confirm took this card over meanwhile)"));
             }
             app.db.audit(Some(&task.project), Some(&task.agent), "action.confirmed", None, None, Some(&task.expects))?;
@@ -60,12 +66,14 @@ pub fn confirm(app: &App, task: &Task, action: &Action) -> Result<String> {
     }
 }
 
-fn perform(app: &App, task: &Task, action: &Action) -> Result<String> {
+fn perform(app: &App, task: &Task, action: &Action, claim: &str) -> Result<String> {
     match action {
-        Action::Forget { name, identity } => Ok(if forget_key(app, name, identity)? {
-            format!("Forgot {name}@{identity}")
-        } else {
-            format!("Nothing was stored for {name}@{identity}")
+        Action::Forget { name, identity } => Ok(match actions::forget(&app.ctx(), name, identity, Some((&task.id, claim)))? {
+            Forgot::Deleted => format!("Forgot {name}@{identity}"),
+            Forgot::NothingStored => format!("Nothing was stored for {name}@{identity}"),
+            Forgot::Gone { kept: false } => format!("Forgot {name}@{identity} (an earlier confirm of this card had deleted it)"),
+            Forgot::Gone { kept: true } => format!("Kept {name}@{identity}: it was stored again after this card was first confirmed, so it is not the key this card was about"),
+            Forgot::KeptUnrecorded => format!("A value is still stored under {name}@{identity}. This card cannot tell it from one stored after the card was first confirmed, so it was kept. A new forget card removes it"),
         }),
         Action::Bind { name, identity } => {
             let project = Path::new(&task.project);
@@ -99,10 +107,7 @@ fn perform(app: &App, task: &Task, action: &Action) -> Result<String> {
     }
 }
 
-/// Delete a stored key and its index row. True if either existed.
+/// Delete a stored key and its index row, for a person at a terminal. True if either existed.
 pub fn forget_key(app: &App, name: &str, identity: &str) -> Result<bool> {
-    let had = app.stash.delete(&tokenstash_core::stash::stash_key(name, identity))?;
-    let meta = app.db.delete_secret(name, identity)?;
-    app.db.audit(None, None, "forget", Some(name), Some(identity), None)?;
-    Ok(had || meta)
+    Ok(actions::forget(&app.ctx(), name, identity, None)? == Forgot::Deleted)
 }

crates/cli/src/cmd/admin.rs

@@ -296,7 +296,7 @@ pub fn audit(a: AuditArgs) -> Result<i32> {
     } else {
         let project = tokenstash_core::project::current();
         match app.db.find_workspace(&project)? {
-            Some(ws) => app.db.recent_audit_for(&project.to_string_lossy(), &ws.created, a.limit)?,
+            Some(ws) => app.db.recent_audit_for(&project.to_string_lossy(), &ws, a.limit)?,
             None => vec![],
         }
     };

crates/cli/src/cmd/doctor.rs

@@ -25,6 +25,40 @@ pub fn doctor() -> Result<i32> {
                 (Ok(()), None) => String::new(),
             };
             ok &= check("stash backend", probe.is_ok(), format!("{}{note}", s.backend()));
+            // With the kernel keyring, an older tokenstash keeps its own copy of a key in each
+            // login session it ran in, and while it runs it can put an old value back.
+            let keys: Vec<String> = Db::open_default()
+                .and_then(|db| db.list_secrets())
+                .map(|v| v.iter().map(|m| tokenstash_core::stash::stash_key(&m.name, &m.identity)).collect())
+                .unwrap_or_default();
+            // One line whenever the backend has copies to look for, so a missing line never
+            // stands for a check that found nothing.
+            if let Some(found) = s.stray_copies(&keys) {
+                let mut detail = vec![];
+                // Two keyrings that disagree are a fault, because a read can return the old
+                // key now.
+                let differ: Vec<&str> = found.copies.iter().filter(|c| c.differs).map(|c| c.key.as_str()).collect();
+                if !differ.is_empty() {
+                    detail.push(format!(
+                        "The user keyring and the persistent keyring hold different values for {}, so a read can return the old key. tokenstash 0.3.0 or earlier, still running in another login session (an inbox or `tokenstash mcp` started before the upgrade), put the other value there. Stop it, then replace the key with `tokenstash rotate NAME` if the old one is in use.",
+                        differ.join(", ")
+                    ));
+                }
+                // A copy only another session holds is a note. It does nothing until an older
+                // tokenstash runs there, and an upgrade leaves idle sessions holding them.
+                let held: Vec<&str> = found.copies.iter().filter(|c| c.elsewhere > 0).map(|c| c.key.as_str()).collect();
+                if !held.is_empty() {
+                    detail.push(format!(
+                        "Other login sessions hold their own copy of {}, left by tokenstash 0.3.0 or earlier. It matters only while an older tokenstash still runs in one of them (an inbox or `tokenstash mcp` started before the upgrade), because it could put the old key back. Stop it, or end that session.",
+                        held.join(", ")
+                    ));
+                }
+                if detail.is_empty() {
+                    detail.push(format!("None among the {} key(s) checked.", found.checked));
+                }
+                detail.extend(found.limited);
+                ok &= check("older copies", differ.is_empty(), detail.join(" "));
+            }
         }
         Err(e) => { ok &= check("stash backend", false, e.to_string()); }
     }

crates/cli/src/cmd/inbox.rs

@@ -337,13 +337,8 @@ fn handle(app: &App, req: Request, tokens: &inbox_auth::Tokens) -> Result<()> {
                 // send it to them the one way the agent cannot read, the desktop. Anyone with
                 // the link can press this; all it does is notify the person again.
                 (_, "notify") => send_full_link(app, &task),
-                (kind, "deny") => {
-                    // An approval card is the human's decision either way: closing it from
-                    // the agent's link would let the agent bury its own pairing card for a
-                    // day, or another agent's in the same directory.
-                    if kind == TaskKind::Approval && scope != Scope::Full {
-                        anyhow::bail!("closing an approval card needs your own inbox link: use the one in the desktop notification (the button below sends it again) and decide on this card there");
-                    }
+                // An approval card's Deny is decided below, with Allow.
+                (kind, "deny") if kind != TaskKind::Approval => {
                     tasks::deny(&ctx, &task, form.get("note").map(|s| s.as_str()))?;
                     Ok(format!("Denied {}", task.title))
                 }
@@ -371,19 +366,28 @@ fn handle(app: &App, req: Request, tokens: &inbox_auth::Tokens) -> Result<()> {
                 (TaskKind::Approval, _) => {
                     // Approving is the one thing a card session must not do: it is the
                     // human's yes to "this project may use my key", and the agent's link
-                    // must not be able to give it. Refuse with no state change.
+                    // must not be able to give it. Refuse with no state change. Closing the card
+                    // is the human's decision too: from the agent's link it would let the agent
+                    // bury its own pairing card for a day, or another agent's in the same
+                    // directory.
                     if scope != Scope::Full {
+                        if action == "deny" {
+                            anyhow::bail!("closing an approval card needs your own inbox link: use the one in the desktop notification (the button below sends it again) and decide on this card there");
+                        }
                         anyhow::bail!("approving needs your own inbox link: use the one in the desktop notification (the button below sends it again) and approve this card there");
                     }
                     let decision = match action.as_str() { "allow" => tasks::Decision::Allow, "allow_broad" => tasks::Decision::AllowBroad, _ => tasks::Decision::Deny };
                     // What the page listed when it was rendered: a card that grew since
-                    // (an agent asked for more) is refused and re-read.
+                    // (an agent asked for more) is refused and re-read. That holds for Deny
+                    // too, which closes the card and every key on it: a key added after the
+                    // page loaded is not declined unseen. The comparison and the close happen
+                    // under the index write lock in `answer_approval`.
                     // The browser form always carries `seen`; a POST without it (a
                     // person scripting `curl`) is judged on the card as it is now.
                     let seen: Option<Vec<String>> = form.get("seen").map(|s| s.split(',').filter(|x| !x.is_empty()).map(String::from).collect());
                     match tasks::answer_approval(&ctx, &task, decision, seen.as_deref())? {
                         AnswerResult::Approved { injected, replaced } => Ok(format!("Approved; injected {}{}", if injected.is_empty() { "nothing new".into() } else { injected.join(", ") }, if replaced.is_empty() { String::new() } else { format!(". {} rejected by the provider at delivery — a Replace card is waiting", replaced.join(", ")) })),
-                        _ => Ok("Denied".into()),
+                        _ => Ok(format!("Denied {}", task.title)),
                     }
                 }
                 (TaskKind::Human, _) => match tokenstash_core::actions::Action::of(&task) {