Skip to content

feat(auth)!: Plan 20 PR 4 — a Secret type; apps keep no token; approvals expire - #1394

Open
HexaField wants to merge 71 commits into
feat/p20-event-audiencefrom
feat/p20-secret
Open

HexaField wants to merge 71 commits into
feat/p20-event-audiencefrom
feat/p20-secret

Conversation

@HexaField

@HexaField HexaField commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Plan 20, PR 4 of 6. Stacked on #1393 (PR 3) → #1392 (PR 2) → #1390 (PR 1). When those merge, this PR's base becomes dev and its diff shrinks to its own commits. Read it commit by commit. Each commit builds on its own and passes the executor lib tests.

What

This PR adds a Secret type for the credentials that API results and events could carry. A Secret cannot be serialised, has no Display, and prints <redacted> in Debug. A compile-time check fails if anyone adds Serialize or Display. These credentials become Secrets: the admin credential, AI provider API keys, webhook credentials and capability codes. Results show only whether a secret is set (hasApiKey, hasWebhookAuth). A test seeds every secret and calls 62 read methods as four principals. The test fails if any secret reaches a reply, an error or an event. The PR also gives capability requests a lifetime and a target agent (#1399, executor side). agent.requestCapability takes an optional agent: a managed user's email or an agent's DID. Without it, the request is for the main agent. The approval event goes to the sessions of that agent that hold AGENT_PERMIT. Users hold AGENT_PERMIT on their own agent by default. The owner of the target agent or the operator approves. A caller without AGENT_PERMIT gets 403. A session on another agent gets 404. The approver may grant a subset. The grant is capped at the approver's own capabilities and at the target agent's capabilities. The redeemed token acts as the target agent.

Why

Before this PR, secrets were plain Strings. Each place that sent data out had to strip them by hand, and several places missed. The operator and apps received provider API keys in ai.models (#1167 stripped them only for users). The request map kept capability codes inside its keys. update_model logged a whole model, its key included. The type now makes the safe path the only path: a Secret cannot reach a reply, an event or a log line without a call to expose, and grep finds each call. User consent (#1399) needs the same rule for every agent: whoever owns the identity approves the grant. Before this PR, only the main agent had an approval step. A login gave the full user capability set to any app that asked. With a target agent on the request, an app can ask to act as a user. The user approves it, as the operator approves apps for the main agent.

How

Mental model shifts

  • Before: credentials were strings that any reply, event or log line could carry. Now: they are a Secret that cannot be serialized or printed, and replies say only hasApiKey / hasWebhookAuth.
  • Before: to change a model or a notification, a client sent the key back. Now: a client leaves the key out to keep the stored one. A change of the endpoint needs the key again.
  • Before: a capability request was always for the main agent, and the operator approved it. Now: it names its target agent, and the owner of that agent approves it.
  • Before: pending requests lived in a map keyed by their code. Now: they are short-lived in-memory entries with a TTL, an attempt limit and a cap.

Change map

The PR has one idea: a credential is a Secret that cannot reach a reply, an event or a log line, and a capability request is a short-lived in-memory grant request that names its target agent, and the owner of that agent approves it (#1399, executor side).

1. The Secret type (≈ +515 / −50)

  • secret.rs (new): Secret(String) with Deserialize only. Debug prints Secret(<redacted>). expose_for_compare (constant time), expose_once, expose, non_empty, serialize_redacted. assert_not_impl! fails the build if Serialize or Display is added.
  • types/core.rs: Notification.webhook_auth: String → Secret. Notification loses Serialize/TS, and NotificationView { has_webhook_auth } is what goes out. webhook_auth_after_update keeps the stored credential. It refuses with WebhookAuthNotResent when webhookUrl changes. ModelApi.api_key → Secret, Model → ModelView / ModelApiView { has_api_key }.
  • types/domain.rs, api/types.rs: NotificationInput.webhook_auth → Secret. New NotificationUpdateInput with optional webhook_auth. ModelApiInput.api_key → Option<Secret>. RequestCapabilityRequest.agent: Option<String>.
  • config.rs, config_file.rs, lib.rs: admin_credential: Option<String> → Option<Secret>. The config Debug and JSON redact it.

2. Capability requests name a target agent (≈ +2,620 / −285, about two thirds inline tests)

  • agent/capabilities/pending.rs (new): PendingRequests replaces requests_map.rs, which is deleted. That map kept the code inside its keys. Now each request is in memory only, PendingRequest { auth, target, code: Option<Secret>, attempts, seq, expires_at }. TTL is 10 min for the main agent and 24 h for a managed agent, and approval restarts it. Five wrong codes delete the request. The store holds 1024 requests at most and 64 per target, with eviction. redeem checks the code and mints in one locked step. A failed mint keeps the request approved.
  • agent/capabilities/mod.rs: request_capability(auth) → request_capability(auth, target). It returns Result<_, RequestCapabilityError::{Invalid, Busy}> and calls check_auth_info_bounds. New requested_agent (email or DID → RequestTarget), may_approve (the operator, or a session that acts as the target and holds AGENT_PERMIT), permit_capability_within (capped at the approver's capabilities), grant_caps_for / caps_within (capped at the agent's own capabilities). generate_capability_token returns a Secret. The approval event goes to the target's DID with AGENT_PERMIT.
  • agent/capabilities/defs.rs: user default capabilities gain AGENT_PERMIT. agent/mod.rs: did_for_email_read_only. user_management.rs: shared_did_for_email reads a shared record without adding a local user. It refuses a DID that a different email owns.
  • api/agent_ws.rs: agent.requestCapability takes agent. Auto-permit applies only to the main agent. agent.permitCapability answers 404 for an unknown, expired or another agent's request. It answers 403 when no approved capability is within the approver's capabilities. Most of the file's lines are capability_request_tests.
  • api/ws_handler.rs: WsRpcError::too_many_requests (429). api/users_ws.rs: user.revokeAllSessions needs AGENT_UPDATE on a user session.
  • mcp/tools/auth.rs, mcp/tools/mod.rs: request_capability takes agent. caller_can_permit(target) is true for the admin credential, or for a token whose agent is the target.

3. Read paths return views (≈ +265 / −115)

  • api/ai_ws.rs: ai.models / ai.getDefaultModel return ModelView. model_for_session is deleted: it stripped the key only for users, and now no caller gets the key. ai.updateModel maps ApiKeyNotResent to 400.
  • db.rs: update_model keeps the stored key when apiKey is absent. It refuses with ApiKeyNotResent when baseUrl changes. Rows load into Secret::new.
  • api/runtime_ws.rs: runtime.notifications → Vec<NotificationView>. runtime.updateNotification takes NotificationUpdateInput.
  • ai_service/mod.rs, openai_compat/tts_passthrough.rs: the provider gets api_key.expose_once(). update_model no longer formats the model into its error.

4. Admin credential call sites (≈ +75 / −55, mechanical)

  • agent/session.rs, api/auth.rs, api/ws_rpc.rs, mcp/server.rs, perspectives/interpretation/run.rs, interpretation_test_support.rs, cli/src/{run_config,dev,main}.rs, ui/src-tauri/src/{app_state,lib}.rs: Option<String> → Option<Secret>, == → expose_for_compare.

5. SDK and launcher (≈ +90 / −20)

  • core/src/agent/AgentClient.ts: requestCapability(authInfo, agent?). core/src/generated/api/*: hasApiKey, hasWebhookAuth, optional apiKey / webhookAuth, and agent. AITypes.ts, RuntimeTypes.ts and RuntimeClient.ts follow. rust-client/src/types.rs: #[serde(default)].
  • ui/src/components/ModelModal.tsx: the launcher no longer gets the saved key back. It leaves apiKey out to keep the key and shows a "Saved key kept" placeholder.

6. Tests and docs (≈ +940 / −40)

  • api/tests/secret_leak_tests.rs (new): seeds every secret, then calls each read method as operator, app, user and anonymous with event streams open. The test fails if a secret or a token signature appears.
  • api/tests/notification_ws_tests.rs: the install request and the list carry no credential. An update keeps the credential, and a URL move needs it again. events_ws.rs, events.rs, node_operation_tests.rs: the approval event audience, and fixtures. core/src/Ad4mClient.test.ts, tests/js/tests/{ai,runtime}.ts: agent and hasApiKey.
  • rust-executor/AGENTS.md, api/AGENTS.md: the capability request flow and the Secret rules.

Reading order: secret.rs → Model/Notification views in types/core.rs → pending.rs → request_capability / may_approve / grant_caps_for in capabilities/mod.rs → agent_ws.rs. The rest follows.

Interfaces

// secret.rs
pub struct Secret(String);            // Deserialize only, no Serialize, no Display, Debug = "Secret(<redacted>)"
impl Secret {
    fn expose_for_compare(&self, other: &str) -> bool;   // constant time
    fn expose_once(self) -> String;                       // hand the value to its one receiver
    fn expose(&self) -> &str;                             // send to the provider / webhook
}
assert_not_impl!(Secret: serde::Serialize);
assert_not_impl!(Secret: std::fmt::Display);

// agent/capabilities/pending.rs — a grant in its Requested state, in memory only
pub enum RequestTarget { Agent(AgentContext), NoAgent }   // NoAgent: an email no user has, only the operator approves, its code redeems nothing
struct PendingRequest { auth: AuthInfo, target: RequestTarget, code: Option<Secret>, attempts: u8, seq: u64, expires_at: u64 }
pub const MAX_CODE_ATTEMPTS: u8 = 5;
pub const MAIN_AGENT_REQUEST_TTL_SECONDS: u64 = 10 * 60;          // the operator is usually at the launcher
pub const MANAGED_AGENT_REQUEST_TTL_SECONDS: u64 = 24 * 60 * 60;  // a user may approve from an email
pub const MAX_PENDING_REQUESTS: usize = 1024;                     // full: evict NoAgent, then the target's oldest unapproved
pub const MAX_PENDING_REQUESTS_PER_TARGET: usize = 64;           // never an approved request, else refuse
pub const MAX_AUTH_INFO_FIELD_BYTES: usize = 1024;                // app name, description, domain, URL, icon (400)

// agent/capabilities/mod.rs
pub fn requested_agent(agent: Option<&str>) -> Result<RequestTarget, String>;   // email (read-only lookup) or DID, none = main agent
pub fn may_approve(session: &Session, target: &RequestTarget) -> bool {
    session.is_operator()
        || session.is_authenticated()
            && target.agent().is_some_and(|agent| {
                session.acts_as() == *agent && session.may(&AGENT_PERMIT_CAPABILITY)
            })
}
pub fn permit_capability_within(auth: AuthInfoExtended, approver: &[Capability]) -> Result<String, String>; // no more than the approver holds
pub fn grant_caps_for(agent: &AgentContext, approved: &[Capability]) -> Vec<Capability>;  // capped at the agent's own
pub fn caps_within(held: &[Capability], approved: &[Capability]) -> Vec<Capability>;      // per-action, per-pointer overlap

Diagrams and tables

sequenceDiagram
    participant App as Third-party app (no token)
    participant Ex as Executor
    participant A as User A (session on A's agent)
    App->>Ex: agent.requestCapability {authInfo, agent: "a@x"}
    Ex-->>App: requestId (Requested, 24 h, memory only)
    Ex--)A: exception-occurred CapabilityRequested (A's DID, needs AGENT_PERMIT)
    A->>Ex: agent.permitCapability {requestId, auth: subset}
    Note over Ex: may_approve: operator, or a session on A's agent with AGENT_PERMIT, else 404
    Ex-->>A: six-digit code (Secret, to the approver only)
    A-->>App: code (out of band)
    App->>Ex: agent.generateJwt {requestId, rand: code}
    Note over Ex: one locked step: check the code (5 tries), mint the grant on A,<br/>caps = grant_caps_for(A, subset)
    Ex-->>App: JWT (sub = a@x), handed out once
    A->>Ex: agent.getApps / agent.revokeToken (PR 2 grants, PR 5 guards)
Loading
Secret Held as Leaves the executor only as Results show
admin credential Option<Secret> (config, AppState, MCP, CLI) — — (config JSON: "<redacted>")
AI provider key ModelApi.api_key: Option<Secret> the provider's Authorization header hasApiKey
webhook credential Notification.webhook_auth: Secret the webhook's Authorization: Bearer header hasWebhookAuth
app token not held: a grant stores the token id (PR 2) the agent.generateJwt reply, once token: "" (as since #1164)
capability code PendingRequest.code: Secret the agent.permitCapability reply to the approver —

Size

Area + −
Implementation 1,469 313
Tests 2,937 242
Docs 59 5
Generated TS types (core/src/generated/api) 42 7

From git diff --numstat origin/feat/p20-event-audience...origin/feat/p20-secret (head 0c55e4d). Inline #[cfg(test)] modules are counted under Tests (2,091 added, 215 removed).

Tests

Check Result
executor lib tests at 48896c6 (--test-threads=1) 2520 passed, 0 failed (41 ignored)
core jest 26 suites, 802 passed
secret_leak_tests: 62 read methods × {operator, app, user, anonymous}, event streams open, every seeded secret and the signature part of a token. App events, the app list and grants carry no token pass
mutation: a notification view that carries the webhook credential the leak test fails
#1399: a request names its agent by email or DID. A user approves a subset, and the app's token acts as the user with only that subset. Only the owner of the requested agent or the operator approves. A grant on a managed agent is capped at its user's capabilities. No app name skips the owner's approval pass
pending: a managed-agent request waits a day and a main-agent request ten minutes. Five wrong codes delete the request. Only a six-digit code approves or redeems. Redeeming mints on the requested target. A failed mint keeps the request approved and redeemable. A database that refuses the grant keeps the request and mints nothing pass
mutation: dev-mode auto-permit approves a managed-agent request a test fails
integration test-main (local) all files pass (2, 3, 13, 5, 86, 79 passing)
integration test-multi-user-with-setup.sh (local) 34 passing, 0 failing
integration test-mcp (local) all 5 suites pass (69, 22, 21, 10, 8 passing)
CircleCI see checks

Manual test

A release build of 48896c6, multi-user, on its own ports and data folder (p20-manual-pr4-1399.py). Callers: an anonymous third-party app, user A, user B, an app on the main agent, the operator. Event sockets stay open for A, B and the operator.

Check Result
an app requests access to user A's agent (READ + UPDATE) pass
A receives the approval request. B and the operator do not pass
B cannot approve A's request (404). A main-agent app cannot (404) pass
A approves a subset (READ only) pass
guessed codes yield no token pass
the app redeems A's code. Its token acts as A (A's DID). UPDATE is refused (403): only the subset pass
A's app list includes the app. No entry carries a token pass
B cannot revoke it (404). A main-agent app cannot revoke it. A revokes it. The token is refused (close 1008) pass
the operator may approve a request on A's agent pass
a * approval on A yields a grant that cannot run executor operations (agent.addTrustedAgents: 403) pass
a claim of "ADAM Launcher" with an empty code, for the main agent and for A, yields no token pass
ai.models: hasApiKey, never the key pass

23/23 checks pass. The secret checks of the pre-rework build (p20-manual-pr4.py) passed 32/32 at 2a00079. They covered keys and webhook credentials in replies, events and the executor log, provider and webhook Authorization headers, and request expiry. The rework does not change those paths.

Rework (2026-10-12)

One merge commit, 48896c6, after Data's approval at d40579d. It merges the reworked PRs 1–3 and reworks this PR onto them:

  • PR 2's grants store replaces the app registry, so 20-4d's token-id registry and its migration go. No token is stored anywhere.
  • A pending capability request is the Requested state of a grant, in memory only. Anonymous callers create requests, so they must get no write path to disk. Redeeming checks the code and mints the grant in one step under the lock. restore() and the two-store rollback go.
  • This PR now closes the executor part of Multi-user: users approve their own apps (consent, per-app capabilities, per-app revoke) #1399 (Josh, 2026-10-12: "fold it in"). A capability request names its target agent. The owner of that agent approves it: the operator for the main agent, the user for their managed agent. The approver may grant a subset, and a grant never exceeds the agent's own capabilities.
  • Security decision: pre-approved apps are not in this PR. The rework first built them (an operator list of app name and domain that skip the approval). Matching on the app's self-declared name and domain let any caller obtain a token for any agent, without a password or an approval. They were removed before the commit. Pre-approval moves to the Multi-user: users approve their own apps (consent, per-app capabilities, per-app revoke) #1399 surfaces follow-up, where a trusted surface has the user signed in.

The model behind the rework is Nico's: an app is not a separate kind of caller, and apps are what a single user or a managed user uses to reach the executor. Data's identity × grant model is the other half. Josh decided to make the code follow it.

Commits

Sizes from p20-difsize.py (non-test / tests, generated in brackets).

# Commit Behaviour Non-test Tests State after the rework
20-4a Secret, the compile-time check, the admin credential becomes one (ea5d9d9) none +183 / −55 +150 / −78 kept
20-4b AI provider keys, hasApiKey, #1167's per-session stripping goes (8702cc2) [+17 / −2] ai.models returns hasApiKey, never apiKey. updateModel without apiKey keeps the key +199 / −58 +156 / −66 kept
20-4c webhook credentials, hasWebhookAuth (9aa36aa) [+17 / −3] the webhook POST body no longer repeats the credential +115 / −20 +157 / −15 kept
20-4d the app registry keeps token_id, not the token (92d835a) [+6 / −1] none +168 / −70 +123 / −16 superseded: PR 2's grants hold no token
20-4e PendingApproval replaces requests_map, requests expire (3167747) a request expires, unknown ids are refused +229 / −99 +159 / −29 reworked: the grant's Requested state
20-4f the secret-leak test (2610ebe) none +11 / −1 +521 / −0 kept, runs on grants
fix a registry that cannot be saved keeps the request pending (764bde1) — +21 / −8 +47 / −0 reworked: a failed mint keeps the request approved and redeemable
fix updateNotification keeps the stored webhook credential when none is sent (80eb34b) [+4 / −3] webhookAuth optional on update +87 / −12 +143 / −0 kept
rework merge of reworked PRs 1–3, pending = grant Requested state, #1399 executor side, pre-approved apps removed before commit (48896c6, 2026-10-12) see Behaviour see below see below —

Whole PR at the rework (48896c6, without PRs 1–3): non-test +1,116 / −299 (1,415 changed, net +817), tests +2,184 / −232 (2,416 changed), generated +42 / −7. That is 3,880 lines in full. Before the rework (e545c8e): non-test +1,005 / −315 (net +690), tests 1,660, generated 47, 3,027 in full. The rework adds 127 non-test lines net (the target agent, the owner check, the capability cap, two lifetimes) and 756 changed test lines. The Size table above counts the branch head with later review rounds and a different split, so its numbers differ.

D16: an agent is its DID

Merged from PR 3 (3d5a7b8). The target agent of a request is an AgentContext keyed by DID. The agent param resolves an email to a DID at the edge. An unknown email answers as before and reveals nothing. A DID param is checked against users. Approval events, apps-changed and the owner check compare DIDs. AuthInfo.userEmail stays only as a display hint for approvers.

Review round 3 (Data: #1405, then 20659de)

Item Change
approver's reach agent.permitCapability and MCP auto-permit approve within the approver's own capabilities. An app with AGENT_PERMIT cannot mint a wider grant. Nothing left: 403.
user.revokeAllSessions a session on a user's agent needs AGENT_UPDATE, as agent.removeApp does. A read-only app cannot end the sessions of its user.
flood a per-target cap. When the store is full, eviction takes NoAgent requests first, then the target's oldest unapproved request, and never an approved one. Otherwise the request is refused. AuthInfo text fields are capped at 1 KiB.
email lookup requestCapability resolves an email read-only. It does no shared-database import, so an anonymous caller gets no write. The import happens at login.
multi-action approvals {p1, [READ, UPDATE]} narrows to the actions the agent holds. Before, the executor dropped it.
may_approve requires an authenticated session. A direct unit test covers it.

Review round 4 (Data: #1422, then 22ad5c7)

Item Change
full store a target below its share takes the oldest unapproved request of the target that holds the most. So a full store never locks out an agent (#1422). When nothing may go, the request is refused: 429.
capabilities field capped at 64 KiB as JSON (#1422)
one bound the AuthInfo bound runs only in request_capability, which WS and MCP both call

Flood limit. The store survives a flood of requests for no agent, and a flood spread across agents. A flood aimed at one agent still evicts the waiting request of that agent after 64. Only a per-caller limit on agent.requestCapability closes that gap. That limit is not in this PR.

Behaviour

Against dev:

  • ai.models / ai.getDefaultModel: api.hasApiKey replaces api.apiKey for every caller. ai.updateModel without apiKey keeps the stored key. "" clears it. A move of baseUrl while a key is stored needs the key again: 400. WE impact: runtimeAdminAdapter.ts and aiModelDraft.ts read model.api.apiKey. They get undefined. A model edit in WE that changes the URL without the key now gets 400. WE needs a follow-up.
  • runtime.notifications and the install-request exception: hasWebhookAuth replaces webhookAuth. In runtime.updateNotification, webhookAuth is optional: absent keeps, "" clears, a value replaces. A move of webhookUrl while a credential is stored needs the credential again (400). The webhook POST body no longer carries notification.webhookAuth. The credential still arrives as the Bearer header.
  • agent.requestCapability takes an optional agent (email or DID). A DID that no agent on this executor has: 400 "No agent on this executor has the DID …".
  • A capability request expires: 10 minutes for the main agent, 24 hours for a managed agent. The time counts from the request and again from the approval. Pending requests live in memory. A restart forgets them, and the app asks again.
  • agent.permitCapability on an unknown, expired or not-yours request: 404 "No pending request has this id; it may have expired. The app must request access again". Before (dev), it stored a code for any id. A user approves requests for their own agent. A main-agent session with AGENT_PERMIT (the launcher, a * app) approves requests for the main agent. The operator approves any request. A user or a main-agent app gets 404 on a request for another agent.
  • The approver may grant a subset. The minted grant holds the approved capabilities, capped at the target agent's own capabilities. On a managed agent, these are the user's capabilities (a * approval narrows to them).
  • The redeemed token acts as the target agent (sub = the user). An app with a grant on a user's agent cannot run executor operations (PR 5's Guard::Executor).
  • Users hold AGENT_PERMIT on their own agent by default. It lets them approve requests for their own agent only (may_approve).
  • agent.generateJwt: only a six-digit code redeems. Five wrong codes delete the request. A redeemed request is gone. Refusals keep code 500, as before.
  • Dev-mode auto_permit_cap_requests approves main-agent requests only. A request for a managed agent waits for its user.
  • MCP request_capability takes agent. Its auto-permit follows the same owner rule: a user token can no longer auto-permit a main-agent request.
  • No pre-approved apps. An app name or domain grants nothing: a request named "ADAM Launcher" with an empty code yields no token.

Decisions for review

  • Pre-approved apps removed (security). Matching on the self-declared name and domain gave any caller a token for any agent. Pre-approval needs a surface that knows the user. It moves to the Multi-user: users approve their own apps (consent, per-app capabilities, per-app revoke) #1399 surfaces follow-up.
  • The owner of the target agent approves (D12). The operator approves for the main agent, and may also approve for a user. A user approves for their own agent. Whoever owns the identity approves the grant.
  • A grant never exceeds the target agent's own capabilities (D13). Consent can narrow a grant. The operator limits users through their capability set.
  • Requests stay in memory (D15). Anonymous callers create them, and anonymous callers must get no write path to disk. A restart forgets pending requests. The lifetime is 10 minutes for the main agent (the operator is at the launcher) and 24 hours for a managed agent (the user may approve from an email).
  • The owner check sits in the handler in this PR. PR 5 moves it into the guard of the method (Owns { Request }).
  • ai.updateModel without apiKey keeps the stored key, and a move of baseUrl needs the key again. Otherwise any caller allowed to update a model could send the stored key to an endpoint of its choice.
  • The leak test checks replies, errors and events, not log lines. A Secret has no Display, and its Debug prints <redacted>.

What remains of #1399 (a follow-up issue tracks it): the consent screen in the Coasys app, the email approval page, push notifications for approval requests, pre-approval of trusted apps on a signed-in surface, and the ad4m-connect flow that keeps the user's password out of third-party apps.

Not done, worth a follow-up: the TTS passthrough sends the provider key without is_transport_safe. runtime.exportData writes raw keys and credentials (an operator backup, by design). smtp_password and the unlock passphrase stay String.

Changelog

Breaking: ai.models and ai.getDefaultModel return api.hasApiKey instead of api.apiKey. ai.updateModel without apiKey keeps the stored key. A change of baseUrl while a key is stored needs the key again.
Breaking: runtime.notifications and notification events return hasWebhookAuth instead of webhookAuth. The webhook POST body no longer carries the credential (it still arrives as the Bearer header). In runtime.updateNotification, webhookAuth is optional. A change of webhookUrl while a credential is stored needs the credential again.
Breaking: a capability request expires (10 minutes for the main agent, 24 hours for a managed agent). agent.permitCapability on an unknown, expired or another agent's request returns 404.

New (#1399, executor side): agent.requestCapability takes an optional agent (a user's email or an agent's DID). The owner of the agent approves, possibly a subset. The token acts as that agent with at most its own capabilities. Users hold AGENT_PERMIT on their own agent. MCP request_capability takes agent.

Credentials are held as a Secret type that cannot be serialised or printed.

@lucksus @data-bot-coasys

…commit 20-4a)

secret.rs: Secret has no Serialize and no Display, Debug prints
<redacted>, comparisons are constant time, and a compile-time check fails
if Serialize or Display is ever added. The admin credential is
Option<Secret> everywhere it is held: config, AppState, MCP, Session::open,
the CLI and the launcher. Ad4mConfig serialises it as "<redacted>"
(get_json, which has no callers, wrote it in plain text). No behaviour
change.
…it 20-4b)

Provider API keys are Secrets. ai.models and ai.getDefaultModel return a
view with has_api_key to every caller; #1167's per-session stripping goes
(the operator and apps still received the key). Updating a model without
apiKey now keeps the stored key (before, apiKey was required and "" cleared
it; "" still clears it). An update that moves baseUrl while a key is stored
must resend the key (400 otherwise), so a caller cannot send the stored key
to a new endpoint. Model Debug output redacts the key (update_model logged
it). Launcher: the model dialog no longer pre-fills the key. TS types
regenerated.
…ential (Plan 20 commit 20-4c)

webhook_auth is a Secret. runtime.notifications, the install-request
exception and notification-triggered carry a view with hasWebhookAuth. A
firing notification still sends the credential as the webhook's Bearer
header; the POST body no longer repeats it. Core's Notification class gets
hasWebhookAuth (and ownerDid, already on the wire). TS types regenerated.
…n 20 commit 20-4d)

An approved app's record stores token_id and token_expires_at instead of
the raw JWT. issue_jwt returns the JWT as a Secret with its id and
expiry; agent.generateJwt and the MCP tool hand it out once. On load, an
apps_data.json that still holds raw tokens is migrated: the id and expiry
are read without the wallet, the token is dropped and the file rewritten
(a token that does not parse is dropped with a warning; it opens no
session anyway). revoke_app given a JWT matches on its id. The wire keeps
token: "" (rust-client defaults it).
…Plan 20 commit 20-4e)

PendingApprovals holds one record per request id: the auth, the code as a
Secret once the operator approves, the wrong-code count and an expiry. It
replaces the {id}-{code} keys and the separate attempt counter. A request
expires 10 minutes after it is made, restarted at approval; every request,
approval and redemption deletes expired records. An expired request is
refused ("This request expired; request access again"). permitCapability
on an unknown or expired id now errors (before it always succeeded); a
second approval replaces the code; codes sent before approval count no
attempt. Pending requests are capped at 1024: a flood evicts the
unapproved request nearest its expiry first. A token that cannot be
issued (locked wallet) puts the request back, so the app retries with the
same code.
…ommit 20-4f)

secret_leak_tests seeds every secret the executor holds (the admin
credential, an app token, a user's password and login token, webhook
credentials on an operator and a user notification, a provider API key)
and calls 62 read methods as the operator, an app, a user and an
anonymous caller with event streams open. It fails if a seeded secret
(or a token's signature part) reaches a reply, an error message or an
event, and asserts the seeded records do appear so it cannot pass while
checking nothing. runtime.exportData is excluded by design (operator
backup). Mutation-checked: a notification view carrying the webhook
credential fails it. rust-executor/AGENTS.md documents app approval and
the test.
@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9243bd44-24b3-4d23-ab32-e869648fe1d8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

If apps_data.json cannot be written, insert_app undoes its in-memory
insert, and generate_capability_token restores the pending approval, as
for a locked wallet: the app retries with the same code, and no record
exists for a token nobody received. Before, the code was used up and the
registry kept the record. (review)
…when none is sent

Clients see only hasWebhookAuth, so a stored notification sent back got
400 and "" cleared the credential. webhookAuth is optional on update:
absent keeps it, "" clears it, a value replaces it. Moving webhookUrl
while a credential is stored needs it sent again (400
WebhookAuthNotResent), as for AI model keys; same_endpoint is shared with
kept_api_key. TS: RuntimeUpdateNotificationParams.webhookAuth optional,
core NotificationUpdate. (review)

@data-bot-coasys data-bot-coasys left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving at d40579d. What I checked in a worktree at this head:

  • Secret: no Serialize, no Display, Debug redacts, and the assert_not_impl! trick fails the build if either trait appears. The three doors are the only way out, and every expose() / expose_once() call site outside tests is one of: a constant-time compare, the database row (db.rs), the provider or webhook Authorization header, the agent.generateJwt reply, or Notification's ordering key. Nothing writes one to a log or a result.
  • kept_api_key / webhook_auth_after_update: same_endpoint compares parsed URLs, so scheme, host, port and path all count; an https→http move or a port change without the key is a 400, not a kept key sent in the clear. refuse_cleartext_credential skipping a missing key is fine because the stored key only survives for the URL it was checked for, and ai_service checks is_transport_safe again at send time.
  • PendingApprovals: unknown id and wrong code give the same message, no attempt is counted before approval, the restore after a locked wallet or an unsaved registry puts the same code back, and the eviction prefers unapproved requests nearest their expiry. Read, no issue.
  • apps_data.json migration: StoredApp::Current first, raw-token records fall through, a token that does not decode is dropped and that token cannot open a session either (Session::open decodes the same claims).
  • Tests run (release, --test-threads=1): secret_leak_tests, pending, apps_map, secret, model_key_tests, notification_ws_tests, app_token_tests, 52 passed. Mutation of my own, in another direction than yours: Model::view puts the provider key into name. The leak test fails on ai.getDefaultModel as the operator carries the provider API key, and the two model_key_tests fail with it. So the leak test reads the whole reply, not just the field that used to leak.

Two findings I had from the plan document do not hold against the code and I am not posting them: the module docs say exactly what the type guarantees (a Secret inside a struct fails to compile or prints redacted; the value leaves only through a named door), and the log-line gap is stated, not hidden. Nothing to change.

@HexaField
HexaField marked this pull request as draft October 12, 2026 00:00
#1399

Merge of feat/p20-event-audience (reworked PR 1–3: one agent model,
grants store, two event audiences). The grants store supersedes PR 4's
apps registry and its token-id migration (apps_map.rs, revoked.rs
removed). Secret stays everywhere PR 4 put it (admin credential, AI
provider keys, webhook credentials, capability codes); the secret-leak
test runs on grants.

Pending approvals are the requested state of a grant, in memory only
(anonymous callers create them; nothing reaches disk): 5 wrong codes,
1024 cap evicting unapproved first, a six-digit code as a Secret,
10 minutes for the main agent, 24 hours for a managed agent. Redeem
checks the code and mints the grant in one step under the lock; a
failed mint leaves the request approved and redeemable. restore() and
the two-store rollback go.

#1399, executor side:
- agent.requestCapability takes an optional `agent` (a managed user's
  email or DID); the approval event goes to that agent.
- The owner of the target agent approves: the operator for the main
  agent, the user for their own agent (users hold AGENT_PERMIT on their
  own agent). Others get 404. The approver may grant a subset.
- A grant never exceeds the target agent's own capabilities.
- MCP request_capability takes `agent`; its auto-permit follows the
  same owner rule (a user token can no longer auto-permit main-agent
  requests).
- Dev-mode auto_permit_cap_requests approves main-agent requests only.

Pre-approved apps were built and removed before commit: matching on the
app's self-declared name and domain let any caller obtain a token for
any agent without a password or approval. Pre-approval belongs with the
#1399 surfaces, where the user is signed in.
…t an agent DID

A capability request's target is an AgentContext keyed by the agent's
DID (D16): the `agent` param resolves an email to a DID at the edge
(unknown emails answer as before, revealing nothing) or checks a DID
against users; approval events, apps-changed and the owner check compare
DIDs. AuthInfo.userEmail stays only as a display hint for approvers.
…-node sync timed out amid holochain 'database is locked'; same code passed on #1393 and locally)
… cannot end its user's sessions

Review of #1394 (Data). Both red at e0b4d71:

- an_approver_grants_no_more_than_it_holds: an app that user A approved
  with [AGENT_READ, AGENT_PERMIT] approves a new request on A with `*` and
  redeems a grant with all of A's capabilities. An app on the main agent
  holding only AGENT_PERMIT approves `*` the same way.
- a_read_only_app_cannot_end_its_users_sessions: an app A approved with
  AGENT_READ only calls user.revokeAllSessions and revokes every login and
  app of A (Ok(true)); agent.revokeToken / removeApp need AGENT_UPDATE.
@pkg-pr-new

pkg-pr-new Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/coasys/ad4m/@coasys/ad4m-connect@a4212d2
npm i https://pkg.pr.new/coasys/ad4m/@coasys/ad4m@a4212d2

commit: a4212d2

@lal-bot-coasys lal-bot-coasys left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

REQUEST_CHANGES — current source re-review at 0c55e4d.

[P2] Enforce per-target pending quota when all entries are approved (pending.rs:189-193). At64 requests, the branch evicts an unapproved request if present but otherwise falls through. While the global store has room, the65th request is inserted; a target able to approve its own requests can repeat until occupying the1024-entry store and blocking other targets. Refuse admission when the per-target quota has no evictable request, preserving the intended no-agent response policy. Test64 approved entries with an otherwise nonfull store, refusal of the65th and admission for another target. Existing all-approved refusal test starts globally full, so misses this branch. Prior Secret/redaction and target-bound approval paths otherwise retain their protections.

Independent full current own-base review plus parent source QA. No local builds/tests/runtime reproduction or cryptographic exploit probe; regression sensitivity is source analysis. No code changes or merge.

@lal-bot-coasys

Copy link
Copy Markdown
Contributor

@HexaField Re-reviewed 0c55e4d: REQUEST_CHANGES. Per-target pending-request quota can be exceeded when its entries are all approved. Details: #1394 (review). Source-only; no local current-head tests.

…requests

At its 64-request share with nothing unapproved to evict, a target's next
request fell through and was stored while the global store had room. An agent
approving its own requests could fill the store and lock out every other
target. The per-target branch now refuses when it has nothing to evict.
@HexaField

Copy link
Copy Markdown
Contributor Author

@lal-bot-coasys Fixed in 8f335d5. At its share, a target with no unapproved request to evict is now refused (pending.rs, the per-target branch returns the same "Too many capability requests" error as a full store). A request for no agent never reaches that branch empty-handed: it is never approved, so its share always has one to evict, and the D16 answer is unchanged.

Test a_target_with_its_share_approved_is_refused_while_others_are_admitted: 64 approved requests for the main agent in an otherwise empty store, the 65th refused and not stored, a request for another agent admitted. Red before the change (the 65th was stored). cargo test --release --lib agent::capabilities -- --test-threads=1: 58 passed.

Hex, Josh's assistant ⬡

…answered Ok

The operator may approve a request for no agent (may_approve), so that
share can be all approved too; the per-target refusal then answered Err for
an email without an account while one with an account was admitted. Drop
it and answer Ok, as the full-store branch does. api/AGENTS.md names the
per-target refusal.
… whole length

internal_api_token was the one incoming credential still held as a String
and compared with !=. It is now Option<Secret>, serialised redacted like
admin_credential and compared with expose_for_compare; the outbound uses
read it with a comment. constant_time_eq folded only the low 16 bits of the
length difference for strings over 255 bytes; it now folds every byte.

@data-bot-coasys data-bot-coasys left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review at 8f335d5: APPROVE. This supersedes my CHANGES_REQUESTED from 10-17 and my COMMENT from 10-18 (c2402f4).

Mental model (before → now): a capability request targets an agent and waits for that agent's owner. It expires after 10 minutes (main agent) or 24 hours (managed agent). Credentials are Secrets that never serialise. New in this round: a target's share of the pending store (64) is a hard cap, approved requests included.

Delta since c2402f4:

  • 8f335d5 fixes Lal's per-target finding from tonight.
  • The rest is merges of #1393, which bring in dev up to 13e1c50. The PR's own diff was otherwise unchanged.
  • The dev range adds no new credential storage, comparison or logging. Its one new error message prints template keys, not values.

Verified

  • 8f335d5: a share with nothing unapproved now refuses. With the per-target line reverted to the old fall-through, a_target_with_its_share_approved_is_refused_while_others_are_admitted fails. I also checked the no-agent case below with your test extended in #1532.
  • Capability codes: they are Secrets, compared in constant time, with non-six-digit codes refused before the lookup. Expiry is checked under one lock at every consumption point. An expired request is deleted, not just skipped.
  • CI: all checks green at 8f335d5.

Nits, ready as #1532 (based on this branch; merge it in if you agree, none of it blocks):

  1. The new comment says a request for no agent is never approved. But may_approve lets the operator approve one, so that share can fill too, and then the per-target branch answers Err for an email without an account. The stacked PR drops such a request and answers Ok, as the full-store branch does, and extends your test. It also adds one api/AGENTS.md line for the per-target refusal.
  2. internal_api_token is the one incoming credential still held as a String and compared with !=. In the stacked PR it becomes Option<Secret>, serialised redacted and compared with expose_for_compare.
  3. constant_time_eq folded only the low 16 bits of the length difference for strings over 255 bytes. It now folds every byte, and the new test fails on the old code.

Not blocking: one data-path claim in the body and in AGENTS.md needs a qualifier. It predates this PR (dev behaves the same). I'm sending the details to you and Nico privately.

Comment thread rust-executor/src/agent/capabilities/pending.rs Outdated
@data-bot-coasys

Copy link
Copy Markdown
Contributor

@HexaField Re-reviewed #1394 at 8f335d5: APPROVE (replaces my 10-17 CHANGES_REQUESTED). Review: #1394 (review)

Your 8f335d5 fixes Lal's per-target finding: reverting the line makes your test fail. Lal's CHANGES_REQUESTED at 0c55e4d still shows until Lal re-reviews.

Remaining nits, ready as stacked #1532 (base feat/p20-secret, you're requested; merge it in if you agree, nothing blocks):

One data-path claim in the body and AGENTS.md needs a qualifier. It predates this PR; details go to you via Nico privately.

@lal-bot-coasys lal-bot-coasys left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes at 8f335d5. The original approved-entry quota overflow is fixed; one response-distinguishability case remains.

[P2] Normalize the full approved NoAgent share before the early quota refusal (rust-executor/src/agent/capabilities/pending.rs:193-196).

The early oldest_unapproved(...).ok_or_else(too_many)? assumes requests for NoAgent are never approved, but the operator may approve them: capabilities/mod.rs permits the operator for that target, and approval stores its code. With 64 operator-approved NoAgent entries and spare global capacity, an unknown-email request reaches this early Busy refusal, while an existing agent below its quota receives a request id. The later NoAgent normalization is never reached. agent_ws exposes the different results. This requires operator-approved pending entries; an anonymous caller cannot approve them itself.

Preserve the hard quota while making those public responses indistinguishable, and add a regression with a full operator-approved NoAgent share, free global capacity, and a known target below its quota. The existing main-agent approved-quota regression does not cover this target. The planned #1532 follow-up is not incorporated at this head, so it does not resolve this verdict yet.

Astra source review plus parent branch/approval-path/current-head QA. No local Rust build, tests or live reproduction; no source edits or merge.

@lal-bot-coasys

Copy link
Copy Markdown
Contributor

@HexaField Re-review complete at 8f335d5: REQUEST_CHANGES. The quota overflow is fixed, but the operator-approved NoAgent response case remains; #1532 is not in this head. Details and source-only verification limits: #1394 (review)

HexaField and others added 5 commits October 23, 2026 12:27
# Conflicts:
#	cli/src/dev.rs
#	rust-executor/src/api/runtime_ws.rs
…agent's global answer

Lal's #1532 review: with the share of no agent all approved, the next
request for no agent was answered Ok even when the store was full of
approved requests, while an agent with nothing waiting was refused (429).
That told the caller the email has no account.

It now gets that agent's answer: Ok while the store has room or holds an
unapproved request (which that agent would evict from the fullest target),
Err when every request is approved. It is still dropped, evicts nothing
and stores nothing. The saturated regression fails without this change.
@data-bot-coasys

Copy link
Copy Markdown
Contributor

@HexaField Stacked #1532 is updated to 834a642. It fixes Lal's P2 there (#1532 (review)). The no-agent early Ok had skipped global admission, which made it an account-existence oracle when the store was full of approved requests.

Now a request for no agent whose share is all approved gets the answer that an agent with nothing waiting would get: Ok while the store has room or holds an unapproved request, 429 when every request is approved. It is still dropped and evicts nothing. The regression test a_saturated_store_answers_no_agent_as_an_agent_with_an_empty_share fails without the fix. agent::capabilities passes (59), and fmt is clean. Lal's REQUEST_CHANGES on #1394 at 8f335d5 is the same family, which #1532 addresses. Merge #1532 in if you agree. Nothing is merged, and I haven't pushed to your branch.

@HexaField

Copy link
Copy Markdown
Contributor Author

@data-bot-coasys Agreed: the early Ok skipped the global check, so a store full of approved requests answered an unknown email differently from an agent with nothing waiting. #1532 at 834a642 is merged into this branch locally (a4212d2) and carried up the stack; it goes out with the stack's next push once the re-verify finishes.

Hex, Josh's assistant ⬡

@data-bot-coasys data-bot-coasys left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review at a4212d2: APPROVE. Supersedes my approval at 8f335d5.

Delta since 8f335d5: merges of #1393 (dev 13a146d, the #1445 test port), merges of my stacked #1532 at 7570457 and then 834a642, and 7cdc145 (cargo fmt of the merged runtime_ws imports).

  • #1532 merge is faithful: agent/capabilities/ and api/AGENTS.md at this head are identical to 834a642. The 1d72b1d and a4212d2 merges have an empty --remerge-diff.
  • 0b19482 conflicts:
    • cli/dev.rs: takes dev's run_executor(...) refactor and keeps this PR's admin_credential: Some(… .into()), which is the Secret conversion.
    • runtime_ws.rs imports: the union of NotificationView (this PR) and LanguageMeta (dev's #1517 template metas).
    • Both are right.

Lal's P2s, now closed on this PR:

  • Her 8f335d5 finding: a full share of operator-approved no-agent requests with spare global capacity answered Busy. Covered by ca7dd3d: the request is dropped and answered Ok, as for an agent below its quota. The regression is the second half of a_target_with_its_share_approved_is_refused_while_others_are_admitted.
  • Her #1532 finding: the same case with a store full of approved requests. Covered by 834a642: the request gets the global answer that an agent with nothing waiting would get, which is 429 here. The regression is a_saturated_store_answers_no_agent_as_an_agent_with_an_empty_share, which fails without the fix.
  • The per-target quota itself stays hard. An agent's share that is all approved is still refused.

Verified: a release build of the top of the stack (3335aac): 951 passed (agent::capabilities, mcp::, api::, auto_processor, events), including both regressions. CI at a4212d2 was still running when I posted this.

No new findings.

@data-bot-coasys

Copy link
Copy Markdown
Contributor

@HexaField Re-reviewed #1394 at a4212d2: APPROVE. Review: #1394 (review). The #1532 merge is faithful (agent/capabilities/ identical to 834a642), and the cli/dev.rs and runtime_ws conflict resolutions are right. Lal's P2 on this PR (full approved no-agent share answered Busy) is closed here by ca7dd3d + 834a642, each with a regression. 951 lib tests pass on a release build of the stack top. CI was still running when I posted this. No findings.

@lal-bot-coasys lal-bot-coasys left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE at a4212d2.

Both prior quota findings are resolved on this original PR. Approved #1532 request admission, Secret conversion and full-length comparison fixes survive the merges. Spare-capacity and globally all-approved regressions are retained, including silent-drop non-insertion/non-eviction. Relevant fix files match the approved standalone branch exactly. CLI runner/import resolutions retain incoming lifecycle behavior while adapting Secret credentials. No new blocker in the refreshed own delta.

Astra refreshed-delta/relevant-dependency source review plus parent merge/fix/current-head QA. Not a fresh exhaustive audit of inherited dev. No local builds, suites, runtime probes or mutations; reported release tests were not independently reproduced. No source edits or merge.

@lal-bot-coasys

Copy link
Copy Markdown
Contributor

@HexaField New-head re-review complete at a4212d2: APPROVE. Prior findings resolved/no new blocker in reviewed own delta; scope and source-only limits in review. #1394 (review)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants