From f268ebbe22b89f93699c6ad9100e122e491a8c94 Mon Sep 17 00:00:00 2001 From: Ian Pascoe Date: Tue, 20 Oct 2026 07:21:32 -0400 Subject: [PATCH 1/3] feat(secret): read Secrets from 1Password through Secret Grants Add Secret Providers to Pi Secret, starting with 1Password through the op CLI. secret_list searches a provider's items and fields (never values), secret_ask_grant asks the user to approve Secret Grants that are read again from the provider at each use, and secret_save saves a typed Secret into a new 1Password item after approval. Sessions store only a grant's locator. pi-utils' secret registry gains refresh callbacks so Child Agents read grants fresh (ADR-0010), and Pi Guardian allows the new tools by default. A backslash before a Secret Reference now makes it literal text. --- .changeset/guardian-secret-grant-tools.md | 5 + .changeset/secret-providers.md | 5 + .changeset/secret-registry-refresh.md | 5 + GLOSSARY-MAP.md | 38 +- .../0010-secret-registry-refresh-callbacks.md | 7 + packages/pi-guardian/README.md | 2 +- packages/pi-guardian/src/tool-policy.ts | 7 +- packages/pi-guardian/test/tool-policy.test.ts | 2 + packages/pi-secret/GLOSSARY.md | 26 +- packages/pi-secret/README.md | 61 +- .../0001-read-secret-grants-at-each-use.md | 13 + packages/pi-secret/skills/pi-secret/SKILL.md | 13 +- packages/pi-secret/src/approval-dialog.ts | 128 +++ .../pi-secret/src/one-password-provider.ts | 382 +++++++++ packages/pi-secret/src/pi-secret-extension.ts | 734 ++++++++++++++++-- packages/pi-secret/src/secret-dialog.ts | 2 +- packages/pi-secret/src/secret-provider.ts | 91 +++ packages/pi-secret/src/secret-references.ts | 108 ++- packages/pi-secret/src/secret-render.ts | 2 +- packages/pi-secret/src/secret-settings.ts | 23 +- .../pi-secret/test/fixtures/fake-provider.ts | 77 ++ .../pi-secret/test/fixtures/secret-harness.ts | 9 +- .../test/null-optional-arguments.test.ts | 25 +- .../test/one-password-provider.test.ts | 272 +++++++ .../test/pi-secret-extension.test.ts | 42 +- packages/pi-secret/test/secret-grants.test.ts | 440 +++++++++++ packages/pi-utils/README.md | 2 +- packages/pi-utils/src/secret-registry.ts | 56 +- .../pi-utils/test/secret-registry.test.ts | 42 + 29 files changed, 2458 insertions(+), 161 deletions(-) create mode 100644 .changeset/guardian-secret-grant-tools.md create mode 100644 .changeset/secret-providers.md create mode 100644 .changeset/secret-registry-refresh.md create mode 100644 docs/adr/0010-secret-registry-refresh-callbacks.md create mode 100644 packages/pi-secret/docs/adr/0001-read-secret-grants-at-each-use.md create mode 100644 packages/pi-secret/src/approval-dialog.ts create mode 100644 packages/pi-secret/src/one-password-provider.ts create mode 100644 packages/pi-secret/src/secret-provider.ts create mode 100644 packages/pi-secret/test/fixtures/fake-provider.ts create mode 100644 packages/pi-secret/test/one-password-provider.test.ts create mode 100644 packages/pi-secret/test/secret-grants.test.ts diff --git a/.changeset/guardian-secret-grant-tools.md b/.changeset/guardian-secret-grant-tools.md new file mode 100644 index 00000000..62ace830 --- /dev/null +++ b/.changeset/guardian-secret-grant-tools.md @@ -0,0 +1,5 @@ +--- +"@ian-pascoe/pi-guardian": minor +--- + +Allow Pi Secret's `secret_ask_grant` and `secret_save` by default, like its other `secret_*` tools: neither reveals a value, and both ask the user in Pi Secret's own dialog. diff --git a/.changeset/secret-providers.md b/.changeset/secret-providers.md new file mode 100644 index 00000000..6c26c127 --- /dev/null +++ b/.changeset/secret-providers.md @@ -0,0 +1,5 @@ +--- +"@ian-pascoe/pi-secret": minor +--- + +Add Secret Providers, starting with 1Password through the `op` CLI. `secret_list` now searches Secret Providers for items and fields (`query`, `provider`, `vault`, `item`, `status`, `limit`), never values. `secret_ask_grant` asks the user to approve Secret Grants, each a Secret Name and the 1Password Reference (`op://vault/item/field`) its value is read from again at each use, so one-time passwords stay current. `secret_save` saves a typed Secret into a new 1Password item after the user approves. The session stores only a grant's locator, never its value. Optional `secret.providers.1password.account` and `vaults` settings narrow what the agent can see. A backslash right before a Secret Reference now makes it literal text, so documentation and tests can contain the syntax. diff --git a/.changeset/secret-registry-refresh.md b/.changeset/secret-registry-refresh.md new file mode 100644 index 00000000..a49ee152 --- /dev/null +++ b/.changeset/secret-registry-refresh.md @@ -0,0 +1,5 @@ +--- +"@ian-pascoe/pi-utils": minor +--- + +Let the secret registry hold Secrets whose values live elsewhere: `holdRefreshableSecret(name, owner, { refresh, value?, expiresAt? })` registers a refresh callback, `refreshSecret(name)` reads the current value through it and keeps every value read redacted, and `isSecretHeld(name)` reports a held Secret before its first read. diff --git a/GLOSSARY-MAP.md b/GLOSSARY-MAP.md index ba0b3dcb..8178cc1e 100644 --- a/GLOSSARY-MAP.md +++ b/GLOSSARY-MAP.md @@ -3,25 +3,25 @@ Read the repository ADRs before a package glossary, then read only the package glossary relevant to the work: -| Package | Glossary | Domain focus | -| ----------------------------------- | ------------------------------------------------------------------------------------------ | ---------------------------------------- | -| `@ian-pascoe/pi-advisor` | [`packages/pi-advisor/GLOSSARY.md`](packages/pi-advisor/GLOSSARY.md) | Session review and tool grants | -| `@ian-pascoe/pi-minimal-subagents` | [`packages/pi-minimal-subagents/GLOSSARY.md`](packages/pi-minimal-subagents/GLOSSARY.md) | Persistent nested agents | -| `@ian-pascoe/pi-bible-verses` | [`packages/pi-bible-verses/GLOSSARY.md`](packages/pi-bible-verses/GLOSSARY.md) | Offline verse rotation and provenance | -| `@ian-pascoe/pi-tps-tracker` | [`packages/pi-tps-tracker/GLOSSARY.md`](packages/pi-tps-tracker/GLOSSARY.md) | Output-token throughput measurement | -| `@ian-pascoe/pi-command-deck` | [`packages/pi-command-deck/GLOSSARY.md`](packages/pi-command-deck/GLOSSARY.md) | Vim prompt editor and session chrome | -| `@ian-pascoe/pi-git-checkpoints` | [`packages/pi-git-checkpoints/GLOSSARY.md`](packages/pi-git-checkpoints/GLOSSARY.md) | Session-linked worktree restoration | -| `@ian-pascoe/pi-formatter` | [`packages/pi-formatter/GLOSSARY.md`](packages/pi-formatter/GLOSSARY.md) | Automatic post-edit formatting | -| `@ian-pascoe/pi-lsp` | [`packages/pi-lsp/GLOSSARY.md`](packages/pi-lsp/GLOSSARY.md) | Language-server tools and edit feedback | -| `@ian-pascoe/pi-dap` | [`packages/pi-dap/GLOSSARY.md`](packages/pi-dap/GLOSSARY.md) | Interactive debug sessions | -| `@ian-pascoe/pi-termctrl` | [`packages/pi-termctrl/GLOSSARY.md`](packages/pi-termctrl/GLOSSARY.md) | Terminals and Background jobs | -| `@ian-pascoe/pi-web-tools` | [`packages/pi-web-tools/GLOSSARY.md`](packages/pi-web-tools/GLOSSARY.md) | Public web search and retrieval | -| `@ian-pascoe/pi-todo` | [`packages/pi-todo/GLOSSARY.md`](packages/pi-todo/GLOSSARY.md) | Minimal session work tracking | -| `@ian-pascoe/pi-context-management` | [`packages/pi-context-management/GLOSSARY.md`](packages/pi-context-management/GLOSSARY.md) | Session Notes, History, and Rollover | -| `@ian-pascoe/pi-skills-selector` | [`packages/pi-skills-selector/GLOSSARY.md`](packages/pi-skills-selector/GLOSSARY.md) | Explicit Skill References in user input | -| `@ian-pascoe/pi-guardian` | [`packages/pi-guardian/GLOSSARY.md`](packages/pi-guardian/GLOSSARY.md) | Model-reviewed tool-call gating | -| `@ian-pascoe/pi-secret` | [`packages/pi-secret/GLOSSARY.md`](packages/pi-secret/GLOSSARY.md) | User-entered secrets kept from the model | -| `@ian-pascoe/pi-utils` | [`packages/pi-utils/GLOSSARY.md`](packages/pi-utils/GLOSSARY.md) | Shared UI conventions and Steer Yield | +| Package | Glossary | Domain focus | +| ----------------------------------- | ------------------------------------------------------------------------------------------ | ------------------------------------------------------------- | +| `@ian-pascoe/pi-advisor` | [`packages/pi-advisor/GLOSSARY.md`](packages/pi-advisor/GLOSSARY.md) | Session review and tool grants | +| `@ian-pascoe/pi-minimal-subagents` | [`packages/pi-minimal-subagents/GLOSSARY.md`](packages/pi-minimal-subagents/GLOSSARY.md) | Persistent nested agents | +| `@ian-pascoe/pi-bible-verses` | [`packages/pi-bible-verses/GLOSSARY.md`](packages/pi-bible-verses/GLOSSARY.md) | Offline verse rotation and provenance | +| `@ian-pascoe/pi-tps-tracker` | [`packages/pi-tps-tracker/GLOSSARY.md`](packages/pi-tps-tracker/GLOSSARY.md) | Output-token throughput measurement | +| `@ian-pascoe/pi-command-deck` | [`packages/pi-command-deck/GLOSSARY.md`](packages/pi-command-deck/GLOSSARY.md) | Vim prompt editor and session chrome | +| `@ian-pascoe/pi-git-checkpoints` | [`packages/pi-git-checkpoints/GLOSSARY.md`](packages/pi-git-checkpoints/GLOSSARY.md) | Session-linked worktree restoration | +| `@ian-pascoe/pi-formatter` | [`packages/pi-formatter/GLOSSARY.md`](packages/pi-formatter/GLOSSARY.md) | Automatic post-edit formatting | +| `@ian-pascoe/pi-lsp` | [`packages/pi-lsp/GLOSSARY.md`](packages/pi-lsp/GLOSSARY.md) | Language-server tools and edit feedback | +| `@ian-pascoe/pi-dap` | [`packages/pi-dap/GLOSSARY.md`](packages/pi-dap/GLOSSARY.md) | Interactive debug sessions | +| `@ian-pascoe/pi-termctrl` | [`packages/pi-termctrl/GLOSSARY.md`](packages/pi-termctrl/GLOSSARY.md) | Terminals and Background jobs | +| `@ian-pascoe/pi-web-tools` | [`packages/pi-web-tools/GLOSSARY.md`](packages/pi-web-tools/GLOSSARY.md) | Public web search and retrieval | +| `@ian-pascoe/pi-todo` | [`packages/pi-todo/GLOSSARY.md`](packages/pi-todo/GLOSSARY.md) | Minimal session work tracking | +| `@ian-pascoe/pi-context-management` | [`packages/pi-context-management/GLOSSARY.md`](packages/pi-context-management/GLOSSARY.md) | Session Notes, History, and Rollover | +| `@ian-pascoe/pi-skills-selector` | [`packages/pi-skills-selector/GLOSSARY.md`](packages/pi-skills-selector/GLOSSARY.md) | Explicit Skill References in user input | +| `@ian-pascoe/pi-guardian` | [`packages/pi-guardian/GLOSSARY.md`](packages/pi-guardian/GLOSSARY.md) | Model-reviewed tool-call gating | +| `@ian-pascoe/pi-secret` | [`packages/pi-secret/GLOSSARY.md`](packages/pi-secret/GLOSSARY.md) | User-entered and provider-granted secrets kept from the model | +| `@ian-pascoe/pi-utils` | [`packages/pi-utils/GLOSSARY.md`](packages/pi-utils/GLOSSARY.md) | Shared UI conventions and Steer Yield | `pi-context-management` implements session-owned Notes, selected-branch History retrieval with a read-only Lineage scope over related agents' sessions, and native Context Checkpoints for diff --git a/docs/adr/0010-secret-registry-refresh-callbacks.md b/docs/adr/0010-secret-registry-refresh-callbacks.md new file mode 100644 index 00000000..4ecddcf2 --- /dev/null +++ b/docs/adr/0010-secret-registry-refresh-callbacks.md @@ -0,0 +1,7 @@ +# Let secret registry entries carry a refresh callback + +[ADR-0008](0008-secret-redaction-registry-in-pi-utils.md) made `@ian-pascoe/pi-utils` own a process-wide registry of held Secret values, through which Child Agents resolve their root session's Secrets. Pi Secret now reads a Secret Grant's value from its Secret Provider at each use ([pi-secret ADR-0001](../../packages/pi-secret/docs/adr/0001-read-secret-grants-at-each-use.md)). A registry that only caches values would give Child Agents a stale value, such as an expired one-time password. So the owner of an entry can register an asynchronous refresh callback with it. A Child Agent's `tool_call` awaits the callback before substituting the value, and every value it returns stays registered for Redaction, including values that have since been replaced. The synchronous lookup and redaction APIs stay as they are, so Pi Guardian, which only redacts, is unaffected. + +## Considered Options + +- **Children use the root's most recently read value**: rejected because a Child Agent would then behave differently from its root for one-time passwords and revoked credentials. diff --git a/packages/pi-guardian/README.md b/packages/pi-guardian/README.md index 25517deb..ce70e839 100644 --- a/packages/pi-guardian/README.md +++ b/packages/pi-guardian/README.md @@ -29,7 +29,7 @@ Every `tool_call`, including calls a tool issues itself (such as a `codemode` sc 1. for `bash`, `terminal_start`, and `powershell`, `deny` when a segment of the command matches a `deny` **Command Rule** (see below), whatever the tool's Tool Policy; 2. the configured `tools.` setting, if any; 3. else the built-in default: - - `allow`: `read`, `grep`, `find`, `ls`, `codemode`, `tool_search`, `todo`, `web_search`, and Context Management's `context_notes`, `context_history`, and `context_rollover`, which only touch the session's own notes, journal, and handoff, pi-advisor's `advisor_report`, which only hands an Advisor Review's findings to pi-advisor, and Pi Secret's `secret_ask`, `secret_list`, and `secret_forget`, which never reveal a Secret's value; + - `allow`: `read`, `grep`, `find`, `ls`, `codemode`, `tool_search`, `todo`, `web_search`, and Context Management's `context_notes`, `context_history`, and `context_rollover`, which only touch the session's own notes, journal, and handoff, pi-advisor's `advisor_report`, which only hands an Advisor Review's findings to pi-advisor, and Pi Secret's `secret_ask`, `secret_ask_grant`, `secret_list`, `secret_save`, and `secret_forget`, which never reveal a Secret's value and ask the user in Pi Secret's own dialog before granting or saving one; - `edit` and `write`: `allow`, unless the target is a **Sensitive Path**, which is reviewed; - `bash`: `allow` only for a **Safe Command**, otherwise reviewed; - `terminal_start`, `terminal_send`, and `powershell`: reviewed, with the user's `deny` and `review` Command Rules named to the Guardian; a `terminal_start` or `powershell` command matching a `deny` rule is denied like `bash`; diff --git a/packages/pi-guardian/src/tool-policy.ts b/packages/pi-guardian/src/tool-policy.ts index 6a38ee36..c97c0743 100644 --- a/packages/pi-guardian/src/tool-policy.ts +++ b/packages/pi-guardian/src/tool-policy.ts @@ -30,10 +30,13 @@ export const allowedByDefault: readonly string[] = [ "context_rollover", // pi-advisor: finishes an Advisor Review; no side effects outside Pi. "advisor_report", - // Pi Secret: asks the user, lists, or forgets Secrets; none reveals a value. Calls that use a - // Secret Reference are judged by their own tool's policy. + // Pi Secret: asks the user for, lists, saves, or forgets Secrets; none reveals a value, and + // grants and saves need the user's approval in Pi Secret's own dialog. Calls that use a Secret + // Reference are judged by their own tool's policy. "secret_ask", + "secret_ask_grant", "secret_list", + "secret_save", "secret_forget", ]; /** Tools reviewed by default regardless of their annotations. */ diff --git a/packages/pi-guardian/test/tool-policy.test.ts b/packages/pi-guardian/test/tool-policy.test.ts index 7ca534e2..af419709 100644 --- a/packages/pi-guardian/test/tool-policy.test.ts +++ b/packages/pi-guardian/test/tool-policy.test.ts @@ -225,7 +225,9 @@ describe("Tool Policy resolution", () => { "context_rollover", "advisor_report", "secret_ask", + "secret_ask_grant", "secret_list", + "secret_save", "secret_forget", ])("allows %s by default", (toolName) => { expect(resolveToolPolicy(call({ toolName }))).toEqual({ policy: "allow", source: "default" }); diff --git a/packages/pi-secret/GLOSSARY.md b/packages/pi-secret/GLOSSARY.md index e8daada2..631893af 100644 --- a/packages/pi-secret/GLOSSARY.md +++ b/packages/pi-secret/GLOSSARY.md @@ -1,11 +1,11 @@ # Secret context -The Secret context lets an agent use credentials the user enters without the values reaching the model provider, the session file, or the screen. +The Secret context lets an agent use credentials the user enters, or grants from a Secret Provider, without the values reaching the model provider, the session file, or the screen. ## Language **Secret**: -A credential value the user entered in answer to a Secret Request, usable by the agent only through its Secret References until it expires or is forgotten. +A credential value, entered by the user or read through a Secret Grant, usable by the agent only through its Secret References until it expires or is forgotten. _Avoid_: Credential, password, env var **Secret Name**: @@ -16,12 +16,32 @@ _Avoid_: Key, secret key, label A string standing in for a Secret's value wherever the agent writes or reads it: `{{secret:NAME}}` in any tool argument, or the `PI_SECRET_NAME` environment variable in a spawned process. The agent and the transcript only ever see references. _Avoid_: Placeholder, token, handle +**Secret Provider**: +An external store of credentials, such as 1Password, that Secrets can come from instead of being typed by the user, or that the user's typed Secrets can be saved into. +_Avoid_: Backend, vault, secret manager, source + +**Secret Grant**: +The user's approval, given in answer to a Grant Request or when saving a typed Secret, for one Secret Name to be read from one location in a Secret Provider until the Secret Lifetime ends. The value is read again from the Secret Provider each time it is used. +_Avoid_: Permission, authorization, access + +**Secret Locator**: +The location of one value in a Secret Provider, written as a URI whose scheme names the Secret Provider. A Secret Grant points at one Secret Locator. +_Avoid_: Path, address, provider reference + +**1Password Reference**: +The Secret Locator form for 1Password: `op://vault/item/field`. +_Avoid_: Secret Reference (that term means the agent's stand-in for a Secret), op URI + **Secret Request**: An agent's request for one or more Secret Names, optionally with a reason shown to the user, which the user answers by entering, skipping, or cancelling each value. Every request prompts again and replaces any Secret already held under the same name. _Avoid_: Prompt, secret ask +**Grant Request**: +An agent's request for one or more Secret Grants, each naming a Secret Name and a Secret Locator, optionally with a reason shown to the user, which the user answers by approving or denying each grant or cancelling them all. Like a Secret Request, it replaces any Secret already held under the same name. +_Avoid_: Grant prompt, access request + **Secret Lifetime**: -The fixed span the user configures, counted from entry, after which a Secret expires and can no longer be read anywhere, including from the sessions that recorded it. The agent can neither choose nor extend it. +The fixed span the user configures, counted from entry or approval, after which a Secret or Secret Grant expires and can no longer be read anywhere, including from the sessions that recorded it. The agent can neither choose nor extend it. _Avoid_: TTL, session lifetime, timeout **Redaction**: diff --git a/packages/pi-secret/README.md b/packages/pi-secret/README.md index 82fbe757..df33946d 100644 --- a/packages/pi-secret/README.md +++ b/packages/pi-secret/README.md @@ -2,7 +2,9 @@ `@ian-pascoe/pi-secret` lets a Pi agent use **Secrets**, such as API keys, passwords, and tokens, that the user types into a masked prompt. The values never reach the model provider, are never written to the session file in plaintext, and are never shown on screen. The agent works only with **Secret References**. -Requires Node `>=22.19.0` and Pi `>=1.1.0`. +It can also read Secrets from a **Secret Provider**, currently 1Password, through **Secret Grants** the user approves, and save a typed Secret into one. + +Requires Node `>=22.19.0` and Pi `>=1.1.0`. Secret Providers need the [1Password CLI](https://developer.1password.com/docs/cli/) (`op`), signed in. > **Pi Secret keeps values out of the conversation; it is not a sandbox.** A Secret is usable by any command the agent runs while it is held, and a misbehaving agent can still send it somewhere, for example with `curl`. Pi Secret redacts values it recognizes in tool output, but pair it with [Pi Guardian](../pi-guardian) to review what the agent does with a Secret. @@ -26,15 +28,34 @@ pi -e ./src/index.ts A call that references a Secret this session does not hold, in either form, is blocked before it runs. For a name that expired, was forgotten, or was never provided, the agent is told to call `secret_ask` again; for a malformed name, such as `{{secret:lower}}` or an empty `{{secret:}}`, it is told what a Secret Name looks like. A Secret Name must be environment-variable style: `^[A-Z_][A-Z0-9_]*$`. +To write either form as literal text, such as in documentation or tests, put a backslash right before it: `\{{secret:NAME}}` or `\PI_SECRET_NAME`. An escaped reference names no Secret, and Pi Secret removes the backslash just before the tool runs, so the tool receives `{{secret:NAME}}`. The transcript keeps the call as written. In a shell command, the shell's own escape, `\$PI_SECRET_NAME` or `\${PI_SECRET_NAME}`, is literal to the shell as well, so Pi Secret leaves it exactly as written. Prefer it there: `$\PI_SECRET_NAME` loses its backslash and becomes an expansion the shell runs without Pi Secret checking or refreshing it. + +### Secret Providers and Secret Grants + +When the 1Password CLI (`op`) is on `PATH` at load, Pi Secret adds `secret_ask_grant` and `secret_save`, and one line of guidance to the system prompt. Both are decided once, so the tool set and prompt never change within a session. + +1. The agent finds an item with `secret_list` (for example `query: "github"`), then lists its fields with `item`. Each field comes with its **Secret Locator**, the 1Password Reference `op://vault/item/field`. Values are never listed. +2. The agent calls `secret_ask_grant({ grants: [{ name: "GITHUB_TOKEN", locator: "op://Dev/GitHub/token" }], reason?: "…" })`. Pi shows each grant's Secret Name, vault, item, field, and field type. Every grant starts allowed: Space toggles one between allow and deny, Enter submits, and Escape cancels them all. +3. Pi Secret reads each allowed grant once, so a wrong locator or a locked 1Password shows up in the result, as `granted`, `denied`, `cancelled`, or `failed` with the reason. A value shorter than 8 characters is granted with a warning. +4. The agent uses the Secret through its Secret References, as with a typed Secret. Pi Secret reads the value again from 1Password on every tool call that references it, so one-time passwords (`?attribute=otp`) are always current and a value rotated or revoked in 1Password takes effect at once. If the read fails, the call is blocked, and the last value read is never used in its place. + +`secret_save({ name, locator, category? })` saves a Secret the user typed into this session into a **new** 1Password item, such as `op://Dev/OpenAI API/credential`, after the user approves it in a dialog. The category defaults to API Credential. It never edits an existing item, and it refuses a title the vault already has. The value reaches `op` on stdin, never on its command line. The Secret then becomes a grant on a locator using the new item's ID, so it survives renames, and it keeps its original expiry. + +A Secret Name holds one Secret at a time: a newer `secret_ask`, `secret_ask_grant`, or `secret_save` replaces whatever was held under that name. + ### Tools -| Tool | Input | Result | -| --------------- | ------------------ | --------------------------------------------------------- | -| `secret_ask` | `names`, `reason?` | Each name's outcome, references, and expiry | -| `secret_list` | — | Held Secret Names and their expiry; never values | -| `secret_forget` | `names` | Forgets the named Secrets now, before their lifetime ends | +| Tool | Input | Result | +| ------------------ | ------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------- | +| `secret_ask` | `names`, `reason?` | Each name's outcome, references, and expiry | +| `secret_ask_grant` | `grants` (`name`, `locator`), `reason?` | Each grant's outcome, references, and expiry | +| `secret_list` | `query?`, `provider?`, `vault?`, `item?`, `status?`, `limit?` | Held Secrets with their expiry and each grant's Secret Locator, and matching Secret Provider items or fields; never values | +| `secret_save` | `name`, `locator`, `category?` | The new item's Secret Locator | +| `secret_forget` | `names` | Forgets the named Secrets, typed or granted, now, before their lifetime ends | + +`secret_list` returns held Secrets and up to `limit` (default 20) available items from each Secret Provider. With `item`, it lists that item's fields instead. `status: "held"` skips the Secret Provider search, and a Secret Provider that cannot be searched is reported in the result, not as an error. -Every `secret_ask` prompts again and replaces any Secret already held under the same name, so the user can correct a mistyped value. Pi Guardian allows all three tools by default, since none reveals a value. Calls that use a Secret Reference are judged by their own tool's policy, and Guardian reviews them with the reference, never the value. +Every `secret_ask` prompts again and replaces any Secret already held under the same name, so the user can correct a mistyped value. Pi Guardian allows all five tools by default, since none reveals a value, and grants and saves need the user's approval in Pi Secret's own dialog. Calls that use a Secret Reference are judged by their own tool's policy, and Guardian reviews them with the reference, never the value. ### `/secret` command @@ -50,7 +71,19 @@ The **Secret Lifetime** is fixed at entry and counted from when the user enters } ``` -`lifetimeMinutes` must be an integer from 1 to 1440 (default 15). An invalid value falls back to 15 minutes, with a warning. +`lifetimeMinutes` must be an integer from 1 to 1440 (default 15). A Secret Grant lasts the same lifetime, counted from approval. + +The 1Password Secret Provider uses `op`'s default account and every vault `op` can see. To narrow it, name the account and the vaults (names or IDs) the agent may search, request grants from, and save into: + +```json +{ + "secret": { + "providers": { "1password": { "account": "my.1password.com", "vaults": ["Dev"] } } + } +} +``` + +Invalid settings fall back to a 15-minute lifetime and no 1Password vaults, with a warning, so a mistyped vault list never widens access. Each Secret is encrypted with its own random AES-256-GCM key. The ciphertext is stored as a `pi-secret` custom entry in the session; the key is stored in `~/.pi/agent/pi-secret/keys.json` (mode `0600`), outside the session. The ciphertext is bound to its name, key, and expiry. As a result: @@ -58,11 +91,17 @@ Each Secret is encrypted with its own random AES-256-GCM key. The ciphertext is - When the lifetime ends, or the Secret is forgotten, its key is deleted, so the session's copy can never be decrypted again. Keys are deleted by a timer, before any tool call once expired, and when a session starts. After that the session file is safe to share as far as Pi Secret is concerned. - This does not protect against someone who can read your disk while a Secret is held: the key file and the session file sit on the same machine. -Child Agents, such as Minimal Subagents children in the same Pi process, can use Secrets their root session holds, through the process-wide registry in `@ian-pascoe/pi-utils` ([ADR-0008](../../docs/adr/0008-secret-redaction-registry-in-pi-utils.md)). They cannot ask for new ones or forget the root's. +A Secret Grant is stored differently ([ADR-0001](docs/adr/0001-read-secret-grants-at-each-use.md)): its `pi-secret-grant` entry holds only the Secret Name, Secret Locator, and expiry, never a value, since the value is read from the Secret Provider at each use. A resumed grant is not read until its first use, so resuming never prompts 1Password. Forgetting any Secret records a `pi-secret-forget` entry, so it is not restored. Saving a typed Secret deletes its key, as forgetting does. + +Child Agents, such as Minimal Subagents children in the same Pi process, can use Secrets their root session holds, through the process-wide registry in `@ian-pascoe/pi-utils` ([ADR-0008](../../docs/adr/0008-secret-redaction-registry-in-pi-utils.md)). They cannot ask for new ones or forget the root's. A Child Agent using a granted Secret reads it fresh from the Secret Provider, as the root does ([ADR-0010](../../docs/adr/0010-secret-registry-refresh-callbacks.md)). ## Limitations -- **Interactive terminal only**: `secret_ask` fails in print, JSON, and RPC modes and in Child Agents, because only Pi's terminal UI can show a masked prompt. +- **Interactive terminal only**: `secret_ask`, `secret_ask_grant`, and `secret_save` fail in print, JSON, and RPC modes and in Child Agents, because only Pi's terminal UI can show their dialogs. +- **1Password approves the CLI, not each item**: once `op` is unlocked, it can read any item in the vaults it can see. Pi Secret reads only locators the user granted, but `secret_list` sends item titles and field labels from those vaults to the model provider. Use `secret.providers.1password.vaults` to narrow what it can see; it is checked at every read, so narrowing it also stops grants already held. The grant dialog names the account only when `account` is set. +- **One-time passwords stay redacted**: every value read is redacted for the rest of the process, including each 6-digit code, so a later output containing the same digits by chance shows the Secret Reference instead. +- **Reads cost time**: each tool call using a granted Secret runs `op read` first, typically well under a second, and fails while 1Password is locked or `op` is signed out. +- **A restored grant's variable**: `PI_SECRET_NAME` for a grant restored on resume is exported at its first use, so a program that reads it from the environment without the call naming it sees nothing until then. - **Running processes keep their environment**: `PI_SECRET_NAME` is unset when a Secret expires or is forgotten, but a terminal or server started while it was held keeps its copy until it exits. - **Streaming output**: Pi shows a running tool's partial output live, before the final result is redacted, so a command that prints a Secret can show it on screen until its result arrives. The stored result and the provider never see it. - **Other extensions' hooks**: the arguments object Pi passes to later `tool_call` and `tool_result` handlers holds the resolved value, and a `tool_result` handler that runs before Pi Secret's sees unredacted output. Pi Guardian redacts its own copy (ADR-0008). @@ -70,5 +109,5 @@ Child Agents, such as Minimal Subagents children in the same Pi process, can use - **Calls another extension blocks**: Pi skips `tool_result` for a blocked call, so if an extension loaded after Pi Secret blocks a call and quotes its resolved arguments in the reason, that text is not redacted. - **One value under two names**: redaction names the Secret entered last with that value. - **Other transformations**: a value the agent reverses, hashes, splits, compresses, or prints only in part is not recognized, so it is not redacted. Redaction is a safety net for accidental output, not a defence against a command built to reveal a Secret. -- **Mentioning the prefix**: any `PI_SECRET_NAME` word in a call's arguments counts as a reference, so text that only mentions an unheld one, such as `env | grep PI_SECRET_X`, is blocked. The bare prefix names no Secret, so `env | grep PI_SECRET_` runs, and its output is redacted. +- **Mentioning the prefix**: any unescaped `PI_SECRET_NAME` word in a call's arguments counts as a reference, so text that only mentions an unheld one, such as `env | grep PI_SECRET_X`, is blocked; write `\PI_SECRET_X` instead. The bare prefix names no Secret, so `env | grep PI_SECRET_` runs, and its output is redacted. - **`{{secret:NAME}}` in shell commands** puts the value on the command line, where `ps` can see it; prefer `$PI_SECRET_NAME`. diff --git a/packages/pi-secret/docs/adr/0001-read-secret-grants-at-each-use.md b/packages/pi-secret/docs/adr/0001-read-secret-grants-at-each-use.md new file mode 100644 index 00000000..1b47e99a --- /dev/null +++ b/packages/pi-secret/docs/adr/0001-read-secret-grants-at-each-use.md @@ -0,0 +1,13 @@ +# Read Secret Grants from the Secret Provider at each use, not sealed into the session + +A typed Secret has no other home, so Pi Secret encrypts it into the session and keeps its key outside the session until the Secret Lifetime ends. A Secret Grant already points at a value that lives in a Secret Provider. The session therefore stores only the grant (Secret Name, Secret Locator, and expiry), and Pi Secret reads the value again from the Secret Provider on every tool call that uses the Secret. This keeps one-time passwords current, picks up rotated values, lets the user cut access by revoking it in the Secret Provider, and keeps another copy of the credential out of the session. If a read fails, the call is blocked, and the last value read is never used in its place. + +## Considered Options + +- **Read once at approval and seal the value like a typed Secret**: rejected because one-time passwords go stale within seconds, a value rotated or revoked in the Secret Provider would keep working until expiry, and the session would hold a copy of a credential that already has a home. + +## Consequences + +- Each use costs one Secret Provider read (about 0.3–1 s for the `op` CLI) and fails while the Secret Provider is locked or signed out. +- Child Agents need fresh reads too, so the shared redaction registry gains a refresh callback ([ADR-0010](../../../../docs/adr/0010-secret-registry-refresh-callbacks.md)). +- Saving a typed Secret into a Secret Provider turns it into a Secret Grant: its sealed copy and key are destroyed, and it keeps its original expiry. diff --git a/packages/pi-secret/skills/pi-secret/SKILL.md b/packages/pi-secret/skills/pi-secret/SKILL.md index 3f986fae..9fcc168d 100644 --- a/packages/pi-secret/skills/pi-secret/SKILL.md +++ b/packages/pi-secret/skills/pi-secret/SKILL.md @@ -1,6 +1,6 @@ --- name: pi-secret -description: Diagnose Pi Secret when secret_ask, Secret References, redaction, the Secret Lifetime setting, or restored Secrets are missing or incorrect. +description: Diagnose Pi Secret when secret_ask, Secret Grants from 1Password, Secret References, redaction, the Secret Lifetime setting, or restored Secrets are missing or incorrect. license: MIT disable-model-invocation: true --- @@ -10,8 +10,9 @@ disable-model-invocation: true 1. Read [`../../README.md`](../../README.md) for the expected prompt, reference, redaction, lifetime, and persistence behavior. Never ask the user to reveal a value while diagnosing. 2. Call `secret_list`. A missing tool is an extension-loading failure. An empty list after `secret_ask` reported `provided` means the Secret expired, was forgotten, or belongs to another session branch. 3. If `secret_ask` fails, check the mode: it needs Pi's interactive terminal and fails in print, JSON, and RPC modes and in Child Agents. -4. If a call was blocked, compare its references with `secret_list`: every `{{secret:NAME}}` and `PI_SECRET_NAME` it contains must name a held Secret. Mentioning `PI_SECRET_` in text that is not a reference also blocks the call. -5. Check `secret.lifetimeMinutes` in Pi's global settings, and in the project's settings when the project is trusted: an integer from 1 to 1440. An invalid value falls back to 15 minutes with a warning at session start. -6. If Secrets do not survive a resume, check that `~/.pi/agent/pi-secret/keys.json` exists, is readable by the user, and is valid JSON. The session's `pi-secret` entries hold only ciphertext; without the key file they cannot be restored, and the user must enter them again. -7. If a value appears in output, confirm the result came through Pi (redaction applies to stored results and provider requests, not to live streaming output) and that the value was entered through `secret_ask` in this Pi process. -8. Finish when `secret_list`, a test call using a reference, and its redacted result agree, or one named mode, setting, key-file, or branch boundary explains the mismatch. +4. If a call was blocked, compare its references with `secret_list`: every `{{secret:NAME}}` and `PI_SECRET_NAME` it contains must name a held Secret. Mentioning `PI_SECRET_NAME` in text that is not a reference also blocks the call unless a backslash comes right before it. +5. If `secret_ask_grant`, `secret_save`, or a call using a granted Secret fails to read 1Password, check in a terminal the user controls that `op whoami` succeeds and `op read` of the same locator works; 1Password may be locked or `op` signed out. If the tools are missing, `op` was not on `PATH` when Pi loaded. If an item is refused, check `secret.providers.1password.vaults` and `account`. +6. Check `secret.lifetimeMinutes` in Pi's global settings, and in the project's settings when the project is trusted: an integer from 1 to 1440. An invalid value falls back to 15 minutes with a warning at session start. +7. If Secrets do not survive a resume, check that `~/.pi/agent/pi-secret/keys.json` exists, is readable by the user, and is valid JSON. The session's `pi-secret` entries hold only ciphertext; without the key file they cannot be restored, and the user must enter them again. +8. If a value appears in output, confirm the result came through Pi (redaction applies to stored results and provider requests, not to live streaming output) and that the value was entered through `secret_ask` in this Pi process. +9. Finish when `secret_list`, a test call using a reference, and its redacted result agree, or one named mode, setting, key-file, or branch boundary explains the mismatch. diff --git a/packages/pi-secret/src/approval-dialog.ts b/packages/pi-secret/src/approval-dialog.ts new file mode 100644 index 00000000..a69a25ac --- /dev/null +++ b/packages/pi-secret/src/approval-dialog.ts @@ -0,0 +1,128 @@ +import { keyText } from "@earendil-works/pi-coding-agent"; +import { + Key, + matchesKey, + truncateToWidth, + wrapTextWithAnsi, + type Focusable, + type KeybindingsManager, +} from "@earendil-works/pi-tui"; +import { hintLine, type UiTheme } from "@ian-pascoe/pi-utils/ui"; + +/** How the user answered an approval dialog. */ +export type ApprovalDialogResult = + | { readonly kind: "cancelled" } + /** Indices of the approved rows; every other row was denied. */ + | { readonly kind: "submitted"; readonly approved: ReadonlySet }; + +/** One thing to approve: a Secret Name and where it is read from or saved to. */ +export interface ApprovalRow { + readonly name: string; + readonly detail: string; +} + +export interface ApprovalDialogOptions { + readonly title: string; + readonly reason: string | undefined; + /** What approving means, such as how long a grant lasts. */ + readonly explanation: string; + readonly rows: readonly ApprovalRow[]; + readonly theme: Pick; + readonly keybindings: Pick; + readonly requestRender: () => void; + readonly done: (result: ApprovalDialogResult) => void; +} + +/** + * Approve or deny each row of a Grant Request or a save. Every row starts approved: Enter + * submits, Space toggles the selected row, and Escape cancels them all. + */ +export class ApprovalDialog implements Focusable { + focused = false; + private readonly approved: boolean[]; + private active = 0; + private finished = false; + + constructor(private readonly options: ApprovalDialogOptions) { + this.approved = options.rows.map(() => true); + } + + invalidate(): void {} + + private finish(result: ApprovalDialogResult): void { + if (this.finished) return; + this.finished = true; + this.options.done(result); + } + + /** End the request as cancelled, such as when the tool call is aborted. */ + cancel(): void { + this.finish({ kind: "cancelled" }); + } + + private move(step: number): void { + const count = this.options.rows.length; + this.active = (this.active + step + count) % count; + } + + handleInput(data: string): void { + if (this.finished) return; + const { keybindings } = this.options; + if (keybindings.matches(data, "tui.select.cancel")) this.cancel(); + else if (keybindings.matches(data, "tui.select.confirm") || data === "\n") { + const approved = new Set(); + this.approved.forEach((yes, index) => { + if (yes) approved.add(index); + }); + this.finish({ kind: "submitted", approved }); + } else if (data === " ") this.approved[this.active] = !this.approved[this.active]; + else if (matchesKey(data, Key.shift("tab")) || keybindings.matches(data, "tui.select.up")) + this.move(-1); + else if (matchesKey(data, Key.tab) || keybindings.matches(data, "tui.select.down")) + this.move(1); + this.options.requestRender(); + } + + render(width: number): string[] { + const { theme, rows, reason, title, explanation } = this.options; + const inner = Math.max(1, width - 2); + const pad = (line: string) => truncateToWidth(` ${line}`, width); + const lines = [theme.fg("border", "─".repeat(Math.max(1, width))), ""]; + lines.push(pad(theme.fg("accent", theme.bold(title)))); + if (reason !== undefined && reason.trim() !== "") { + lines.push(""); + for (const line of wrapTextWithAnsi(theme.fg("text", reason.trim()), inner)) + lines.push(pad(line)); + } + lines.push(""); + for (const line of wrapTextWithAnsi(theme.fg("muted", explanation), inner)) + lines.push(pad(line)); + lines.push(""); + rows.forEach((row, index) => { + const selected = index === this.active; + const marker = selected ? theme.fg("accent", "›") : " "; + // A checkbox for the toggle state; Status Marks are reserved for outcomes (ADR-0006). + const box = this.approved[index] + ? theme.fg("accent", "[x] allow") + : theme.fg("muted", "[ ] deny "); + const label = theme.fg(selected ? "accent" : "text", row.name); + lines.push(pad(`${marker} ${box} ${label}`)); + // The location gets its own lines, so a long one is wrapped rather than cut off. + for (const line of wrapTextWithAnsi(theme.fg("muted", row.detail), Math.max(1, inner - 4))) + lines.push(pad(` ${line}`)); + }); + lines.push(""); + lines.push( + pad( + hintLine(theme, [ + { key: keyText("tui.select.confirm"), description: "submit" }, + { key: "space", description: "allow/deny" }, + { key: "tab", description: "next" }, + { key: keyText("tui.select.cancel"), description: "cancel" }, + ]), + ), + ); + lines.push("", theme.fg("border", "─".repeat(Math.max(1, width)))); + return lines; + } +} diff --git a/packages/pi-secret/src/one-password-provider.ts b/packages/pi-secret/src/one-password-provider.ts new file mode 100644 index 00000000..bc17afec --- /dev/null +++ b/packages/pi-secret/src/one-password-provider.ts @@ -0,0 +1,382 @@ +import { spawn } from "node:child_process"; +import { Type, type Static, type TSchema } from "typebox"; +import { Value } from "typebox/value"; +import type { + AvailableEntry, + LocatorDescription, + ProviderListing, + ProviderSearch, + SaveRequest, + SecretProvider, +} from "./secret-provider.js"; + +/** The `secret.providers.1password` settings. */ +export interface OnePasswordSettings { + /** The account `op` signs in to; `op`'s default when unset. */ + account?: string; + /** Vault names or IDs the agent may search, request, and save into; every vault when unset, none when empty. */ + vaults?: string[]; +} + +/** Runs `op` with arguments, piping `input` to stdin when given, and resolves its stdout. */ +export type OpRunner = ( + args: readonly string[], + options: { input?: string | undefined; signal?: AbortSignal | undefined }, +) => Promise; + +const OP_TIMEOUT_MS = 60_000; + +/** `[ERROR] 2026/10/20 06:26:28 account is not signed in` → `account is not signed in`. */ +function opErrorText(stderr: string): string { + const text = stderr + .trim() + .replace(/^\[ERROR\]\s+\S+\s+\S+\s+/, "") + .trim(); + return text === "" ? "op failed without a message" : text; +} + +/** Run the `op` CLI without a shell, so no argument is interpreted. */ +export const runOp: OpRunner = (args, options) => + new Promise((resolve, reject) => { + const child = spawn("op", [...args], { + stdio: ["pipe", "pipe", "pipe"], + signal: options.signal, + timeout: OP_TIMEOUT_MS, + }); + const stdout: Buffer[] = []; + const stderr: Buffer[] = []; + child.stdout.on("data", (chunk: Buffer) => stdout.push(chunk)); + child.stderr.on("data", (chunk: Buffer) => stderr.push(chunk)); + child.on("error", (cause) => + reject( + "code" in cause && cause.code === "ENOENT" + ? new Error("the 1Password CLI (op) is not installed or not on PATH") + : cause, + ), + ); + child.on("close", (code) => { + if (code === 0) resolve(Buffer.concat(stdout).toString("utf8")); + else reject(new Error(opErrorText(Buffer.concat(stderr).toString("utf8")))); + }); + child.stdin.end(options.input ?? ""); + }); + +const VaultSchema = Type.Object({ id: Type.String(), name: Type.Optional(Type.String()) }); +const ItemSummarySchema = Type.Object({ + id: Type.String(), + title: Type.String(), + category: Type.Optional(Type.String()), + vault: VaultSchema, +}); +const ItemListSchema = Type.Array(ItemSummarySchema); +const FieldSchema = Type.Object({ + id: Type.String(), + label: Type.Optional(Type.String()), + type: Type.Optional(Type.String()), + reference: Type.Optional(Type.String()), + section: Type.Optional( + Type.Object({ id: Type.Optional(Type.String()), label: Type.Optional(Type.String()) }), + ), +}); +const ItemSchema = Type.Object({ + id: Type.String(), + title: Type.String(), + category: Type.Optional(Type.String()), + vault: VaultSchema, + fields: Type.Optional(Type.Array(FieldSchema)), +}); +type Vault = Static; +type Field = Static; + +function parseJson(schema: T, text: string, what: string): Static { + const parsed: unknown = JSON.parse(text); + if (!Value.Check(schema, parsed)) throw new Error(`op returned an unexpected ${what}`); + return parsed; +} + +/** The parts of a 1Password Reference: `op://vault/item/[section/]field[?query]`. */ +interface ParsedReference { + readonly vault: string; + readonly item: string; + readonly section: string | undefined; + readonly field: string; + readonly attribute: string | undefined; +} + +const SCHEME = "op://"; + +function parseReference(locator: string): ParsedReference { + if (!locator.startsWith(SCHEME)) + throw new Error( + `${JSON.stringify(locator)} is not a 1Password Reference (op://vault/item/field)`, + ); + const [path = "", query = ""] = locator.slice(SCHEME.length).split("?", 2); + const parts = path.split("/"); + if (parts.length < 3 || parts.length > 4 || parts.some((part) => part.trim() === "")) + throw new Error( + `${JSON.stringify(locator)} is not a 1Password Reference: use op://vault/item/field or op://vault/item/section/field`, + ); + const [vault = "", item = "", ...rest] = parts; + const field = rest.at(-1) ?? ""; + const section = rest.length === 2 ? rest[0] : undefined; + const attribute = new URLSearchParams(query).get("attribute") ?? undefined; + return { vault, item, section, field, attribute }; +} + +/** Whether two 1Password names or IDs are equal; 1Password matches them case-insensitively. */ +const sameName = (a: string | undefined, b: string | undefined) => + a !== undefined && b !== undefined && a.toLowerCase() === b.toLowerCase(); + +/** A vault as the user knows it: its name, or its ID when op omits the name. */ +const vaultLabel = (vault: Vault): string => vault.name ?? vault.id; + +/** One field of an item JSON template, as `op item create` reads it on stdin. */ +interface TemplateField { + id: string; + label: string; + type: "CONCEALED"; + value: string; + purpose?: string; +} + +/** Built-in fields a template sets by purpose rather than as custom fields. */ +const FIELD_PURPOSES = new Map([ + ["username", "USERNAME"], + ["password", "PASSWORD"], + ["notesplain", "NOTES"], +]); + +/** A field's 1Password Reference; a one-time password field reads its current code. */ +function fieldLocator(field: Field): string | undefined { + if (field.reference === undefined) return undefined; + return sameName(field.type, "OTP") && !field.reference.includes("?") + ? `${field.reference}?attribute=otp` + : field.reference; +} + +/** `API Credential`, `api credential`, or `API_CREDENTIAL` → `API_CREDENTIAL`. */ +function categoryId(category: string | undefined): string { + return (category ?? "API Credential") + .trim() + .toUpperCase() + .replace(/[\s-]+/g, "_"); +} + +/** The 1Password Secret Provider, through the user's signed-in `op` CLI. */ +export class OnePasswordProvider implements SecretProvider { + readonly id = "1password"; + readonly label = "1Password"; + readonly scheme = "op"; + + constructor( + private readonly settings: () => OnePasswordSettings, + private readonly run: OpRunner = runOp, + ) {} + + private op( + args: string[], + options: { input?: string | undefined; signal?: AbortSignal | undefined } = {}, + ) { + const account = this.settings().account; + // A global flag, before the subcommand: after `--`, op would read it as an argument. + return this.run(account === undefined ? args : ["--account", account, ...args], options); + } + + private allowed(vault: Vault): boolean { + const vaults = this.settings().vaults; + return ( + vaults === undefined || + vaults.some((entry) => sameName(entry, vault.id) || sameName(entry, vault.name)) + ); + } + + /** Vaults resolved by the name or ID a locator uses, so each is looked up once. */ + private readonly vaults = new Map(); + + private assertAllowed(vault: Vault): void { + // The vault is not named: the agent may not learn which vaults exist outside the list. + if (!this.allowed(vault)) + throw new Error("that item is not in a vault listed in secret.providers.1password.vaults"); + } + + /** Refuse a locator's vault unless it is allowed; free when every vault is. */ + private async assertVaultAllowed(vault: string, signal?: AbortSignal): Promise { + if (this.settings().vaults === undefined) return; + let resolved = this.vaults.get(vault); + if (resolved === undefined) { + resolved = parseJson( + VaultSchema, + await this.op(["vault", "get", "--format", "json", "--", vault], { signal }), + "vault", + ); + this.vaults.set(vault, resolved); + } + this.assertAllowed(resolved); + } + + private async item(vault: string, item: string, signal?: AbortSignal) { + const text = await this.op(["item", "get", "--vault", vault, "--format", "json", "--", item], { + signal, + }); + const parsed = parseJson(ItemSchema, text, "item"); + this.assertAllowed(parsed.vault); + return parsed; + } + + async describe(locator: string, signal?: AbortSignal): Promise { + const reference = parseReference(locator); + const item = await this.item(reference.vault, reference.item, signal); + const field = (item.fields ?? []).find( + (candidate: Field) => + (sameName(candidate.label, reference.field) || sameName(candidate.id, reference.field)) && + (reference.section === undefined || + sameName(candidate.section?.label, reference.section) || + sameName(candidate.section?.id, reference.section)), + ); + if (field === undefined) + throw new Error( + `item ${JSON.stringify(item.title)} has no field ${JSON.stringify(reference.field)}`, + ); + const description: LocatorDescription = { + locator, + vault: vaultLabel(item.vault), + item: item.title, + field: field.label ?? field.id, + fieldType: (reference.attribute ?? field.type ?? "unknown").toLowerCase(), + }; + const account = this.settings().account; + return account === undefined ? description : { ...description, account }; + } + + async describeSave( + request: Omit, + signal?: AbortSignal, + ): Promise { + const reference = parseReference(request.locator); + if (reference.section !== undefined || reference.attribute !== undefined) + throw new Error("save to op://vault/item/field, without a section or query"); + const vault = parseJson( + VaultSchema, + await this.op(["vault", "get", "--format", "json", "--", reference.vault], { signal }), + "vault", + ); + this.assertAllowed(vault); + const items = parseJson( + ItemListSchema, + await this.op(["item", "list", "--vault", vault.id, "--format", "json"], { signal }), + "item list", + ); + if (items.some((item) => sameName(item.title, reference.item))) + throw new Error( + `vault ${JSON.stringify(vaultLabel(vault))} already has an item titled ${JSON.stringify(reference.item)}; secret_save only creates new items`, + ); + const description: LocatorDescription = { + locator: request.locator, + vault: vaultLabel(vault), + item: reference.item, + field: reference.field, + fieldType: `concealed, new ${categoryId(request.category).toLowerCase().replaceAll("_", " ")} item`, + }; + const account = this.settings().account; + return account === undefined ? description : { ...description, account }; + } + + async read(locator: string, signal?: AbortSignal): Promise { + // Checked at every read, so narrowing the vaults also stops grants already held or restored. + await this.assertVaultAllowed(parseReference(locator).vault, signal); + const value = await this.op(["read", "--no-newline", "--", locator], { signal }); + if (value === "") throw new Error(`${locator} is empty`); + return value; + } + + async search(search: ProviderSearch, signal?: AbortSignal): Promise { + const query = search.query?.toLowerCase(); + let entries: AvailableEntry[]; + if (search.item !== undefined) { + const item = await this.findItem(search.item, search.vault, signal); + // Values are dropped here: only labels, types, and references leave this method. + entries = (item.fields ?? []).flatMap((field: Field): AvailableEntry[] => { + const label = field.label ?? field.id; + const locator = fieldLocator(field); + if (locator === undefined) return []; + if (query !== undefined && !label.toLowerCase().includes(query)) return []; + return [ + { + kind: "field", + provider: this.id, + item: item.title, + vault: vaultLabel(item.vault), + label, + type: (field.type ?? "unknown").toLowerCase(), + locator, + }, + ]; + }); + } else { + const args = ["item", "list", "--format", "json"]; + if (search.vault !== undefined) args.push("--vault", search.vault); + const items = parseJson(ItemListSchema, await this.op(args, { signal }), "item list"); + entries = items + .filter((item) => this.allowed(item.vault)) + .filter((item) => query === undefined || item.title.toLowerCase().includes(query)) + .map((item) => ({ + kind: "item", + provider: this.id, + id: item.id, + title: item.title, + vault: vaultLabel(item.vault), + category: (item.category ?? "unknown").toLowerCase().replaceAll("_", " "), + })); + } + return { entries: entries.slice(0, search.limit), truncated: entries.length > search.limit }; + } + + /** + * One item by title or ID, looked up only in allowed vaults, so an item elsewhere is never + * fetched and op's errors never name one. + */ + private async findItem(item: string, vault: string | undefined, signal?: AbortSignal) { + const vaults = vault === undefined ? this.settings().vaults : [vault]; + if (vaults === undefined) { + const text = await this.op(["item", "get", "--format", "json", "--", item], { signal }); + return parseJson(ItemSchema, text, "item"); + } + let lastError = new Error("no vault is listed in secret.providers.1password.vaults"); + for (const candidate of vaults) { + try { + return await this.item(candidate, item, signal); + } catch (cause) { + lastError = cause instanceof Error ? cause : new Error(String(cause)); + } + } + throw lastError; + } + + async save(request: SaveRequest, signal?: AbortSignal): Promise { + await this.describeSave(request, signal); + const reference = parseReference(request.locator); + const purpose = FIELD_PURPOSES.get(reference.field.toLowerCase()); + const field: TemplateField = { + id: reference.field, + label: reference.field, + type: "CONCEALED", + value: request.value, + }; + if (purpose !== undefined) field.purpose = purpose; + const template = { + title: reference.item, + category: categoryId(request.category), + fields: [field], + }; + // The value reaches op on stdin, never on its command line, where other processes can see it. + const created = parseJson( + ItemSchema, + await this.op(["item", "create", "--vault", reference.vault, "--format", "json", "-"], { + input: JSON.stringify(template), + signal, + }), + "created item", + ); + return `${SCHEME}${created.vault.id}/${created.id}/${reference.field}`; + } +} diff --git a/packages/pi-secret/src/pi-secret-extension.ts b/packages/pi-secret/src/pi-secret-extension.ts index 7b837eab..5529bf74 100644 --- a/packages/pi-secret/src/pi-secret-extension.ts +++ b/packages/pi-secret/src/pi-secret-extension.ts @@ -1,6 +1,7 @@ +import { accessSync, constants } from "node:fs"; import { lstat, readFile, realpath, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; -import { sep } from "node:path"; +import { delimiter, join, sep } from "node:path"; import type { JsonValue } from "@earendil-works/pi-ai"; import { defineTool, @@ -14,8 +15,11 @@ import { import { acceptNullForOptionalArguments } from "@ian-pascoe/pi-utils/null-optional-arguments"; import { heldSecretNames, + holdRefreshableSecret, holdSecret, + isSecretHeld, redactSecrets, + refreshSecret, resolveSecret, revokeSecret, SECRET_NAME_PATTERN, @@ -23,22 +27,44 @@ import { type SecretOwner, } from "@ian-pascoe/pi-utils/secret-registry"; import { noticeText } from "@ian-pascoe/pi-utils/ui"; -import { Type } from "typebox"; +import { Type, type Static } from "typebox"; import { Value } from "typebox/value"; -import { SecretDialog, type SecretDialogResult } from "./secret-dialog.js"; +import { ApprovalDialog, type ApprovalDialogResult } from "./approval-dialog.js"; +import { OnePasswordProvider } from "./one-password-provider.js"; +import { + lifetimeText, + SecretDialog, + SHORT_SECRET_LENGTH, + type SecretDialogResult, +} from "./secret-dialog.js"; +import { + providerFor, + type AvailableEntry, + type LocatorDescription, + type SaveRequest, + type SecretProvider, +} from "./secret-provider.js"; import { renderSecretCall, renderSecretResult } from "./secret-render.js"; import { isSecretName, referencedSecretNames, - resolvePlaceholders, + rewriteReferences, secretEnvName, } from "./secret-references.js"; -import { DEFAULT_LIFETIME_MINUTES, readSecretSettings } from "./secret-settings.js"; +import { + DEFAULT_LIFETIME_MINUTES, + readSecretSettings, + type SecretSettings, +} from "./secret-settings.js"; import { keyFilePath, SealedSecretSchema, SecretVault, type SealedSecret } from "./secret-vault.js"; import { TROUBLESHOOTING_HINT } from "./troubleshooting-skill.js"; -/** Custom session entry type holding one Secret's ciphertext. */ +/** Custom session entry type holding one typed Secret's ciphertext. */ export const SECRET_ENTRY_TYPE = "pi-secret"; +/** Custom session entry type holding one Secret Grant: a Secret Locator, never a value. */ +export const GRANT_ENTRY_TYPE = "pi-secret-grant"; +/** Custom session entry type recording that a Secret was forgotten, so it is not restored. */ +export const FORGET_ENTRY_TYPE = "pi-secret-forget"; /** Static system prompt lines, so they never move the cache prefix within a session. */ export const SECRET_PROMPT_SNIPPET = @@ -47,20 +73,40 @@ export const SECRET_PROMPT_GUIDELINES = [ "When a task needs a Secret such as an API key or password, call secret_ask instead of asking the user to paste it into the conversation.", "Use a held Secret as $PI_SECRET_NAME in shell commands, which keeps it off the command line, and as {{secret:NAME}} in other tool arguments. Tool output shows {{secret:NAME}} where a value was redacted.", ]; +/** Added, once at load, only when a Secret Provider is available. */ +export const SECRET_PROVIDER_GUIDELINE = + "When a Secret may already be in a Secret Provider such as 1Password, find it with secret_list and request it with secret_ask_grant instead of asking the user to type it."; -const SECRET_TOOLS: readonly string[] = ["secret_ask", "secret_list", "secret_forget"]; +const SECRET_TOOLS: readonly string[] = [ + "secret_ask", + "secret_ask_grant", + "secret_list", + "secret_save", + "secret_forget", +]; export interface PiSecretOptions { /** Pi's agent directory, which holds the key file; tests isolate it. */ getAgentDirectory?: () => string; /** Milliseconds since the epoch; tests pass the Secret Lifetime instantly. */ now?: () => number; + /** + * The Secret Providers, given the provider settings read at session start. By default, the + * 1Password Secret Provider when `op` is on `PATH` at load. + */ + providers?: (settings: () => SecretSettings["providers"]) => SecretProvider[]; } +/** How a held Secret got its value: typed by the user, or granted from a Secret Provider. */ +type HeldSecretOrigin = + | { readonly kind: "typed"; readonly keyId: string } + | { readonly kind: "granted"; readonly locator: string; readonly provider: SecretProvider }; + interface LiveSecret { readonly name: string; - readonly value: string; - readonly keyId: string; + /** The value last entered or read; undefined until a restored grant is first used. */ + value: string | undefined; + readonly origin: HeldSecretOrigin; readonly expiresAt: number; readonly timer: ReturnType; } @@ -68,10 +114,23 @@ interface LiveSecret { /** One Secret's outcome in a tool result; a type alias so it is assignable to JSON. */ type SecretOutcome = { name: string; - status: "provided" | "skipped" | "cancelled" | "held" | "forgotten" | "not_held"; + status: + | "provided" + | "skipped" + | "cancelled" + | "granted" + | "denied" + | "failed" + | "saved" + | "held" + | "forgotten" + | "not_held"; reference?: string; env?: string; expires_at?: string; + locator?: string; + error?: string; + warning?: string; }; const SecretNameSchema = Type.String({ @@ -79,6 +138,11 @@ const SecretNameSchema = Type.String({ description: "Secret Name in environment-variable style, such as OPENCODE_API_KEY", }); +const LocatorSchema = Type.String({ + minLength: 1, + description: "Secret Locator, such as the 1Password Reference op://vault/item/field", +}); + const AskParameters = Type.Object({ names: Type.Array(SecretNameSchema, { minItems: 1, @@ -91,6 +155,60 @@ const AskParameters = Type.Object({ ), }); +const AskGrantParameters = Type.Object({ + grants: Type.Array(Type.Object({ name: SecretNameSchema, locator: LocatorSchema }), { + minItems: 1, + maxItems: 20, + description: + "Secret Grants to request, each a Secret Name and the Secret Locator its value is read from; names must be unique", + }), + reason: Type.Optional( + Type.String({ description: "Why the Secrets are needed, shown to the user" }), + ), +}); + +const ListParameters = Type.Object({ + query: Type.Optional( + Type.String({ + description: + "Case-insensitive text matched against Secret Names, item titles, and field labels", + }), + ), + provider: Type.Optional( + Type.String({ description: "Only this Secret Provider, such as 1password" }), + ), + vault: Type.Optional(Type.String({ description: "Only available items in this vault" })), + item: Type.Optional( + Type.String({ + description: "List this item's fields, with the Secret Locator to request each by", + }), + ), + status: Type.Optional( + Type.Union([Type.Literal("held"), Type.Literal("available")], { + description: "held: Secrets this session can use; available: items in Secret Providers", + }), + ), + limit: Type.Optional( + Type.Integer({ + minimum: 1, + maximum: 100, + description: "Most available entries per Secret Provider (default 20)", + }), + ), +}); + +const SaveParameters = Type.Object({ + name: SecretNameSchema, + locator: Type.String({ + minLength: 1, + description: + "Secret Locator of the new item's field, such as op://vault/new item title/credential; the item must not exist yet", + }), + category: Type.Optional( + Type.String({ description: "Category of the new item (default API Credential)" }), + ), +}); + const ForgetParameters = Type.Object({ names: Type.Array(SecretNameSchema, { minItems: 1, @@ -99,8 +217,27 @@ const ForgetParameters = Type.Object({ }), }); +const SecretGrantSchema = Type.Object( + { + version: Type.Literal(1), + name: Type.String(), + locator: Type.String(), + expiresAt: Type.Number(), + }, + { additionalProperties: false }, +); +type SecretGrant = Static; +const ForgottenSchema = Type.Object( + { version: Type.Literal(1), name: Type.String() }, + { additionalProperties: false }, +); + +const DEFAULT_LIST_LIMIT = 20; + const WITHHELD_RESULT = "Pi Secret could not check this tool result for Secret values, so it was withheld. The tool did run."; +const ESCAPE_HINT = + "To write a reference as literal text instead, put a backslash right before it; Pi Secret removes the backslash."; function iso(time: number): string { return new Date(time).toISOString(); @@ -112,7 +249,7 @@ function iso(time: number): string { */ function unresolvedReason(names: readonly string[]): string | undefined { const malformed = names.filter((name) => !isSecretName(name)); - const unheld = names.filter((name) => isSecretName(name) && resolveSecret(name) === undefined); + const unheld = names.filter((name) => isSecretName(name) && !isSecretHeld(name)); const sentences = ["The call did not run."]; if (malformed.length > 0) { const listed = malformed.map((name) => JSON.stringify(name)).join(", "); @@ -128,13 +265,41 @@ function unresolvedReason(names: readonly string[]): string | undefined { `It references ${listed}, which this session does not hold. The Secret expired, was forgotten, or was never provided. Call secret_ask to request it again.`, ); } - return sentences.length === 1 ? undefined : sentences.join(" "); + if (sentences.length === 1) return undefined; + sentences.push(ESCAPE_HINT); + return sentences.join(" "); +} + +/** The reason to show the user, if the agent gave one that is not blank. */ +function shownReason(reason: string | undefined): string | undefined { + return reason?.trim() === "" ? undefined : reason; } function errorMessage(cause: unknown): string { return cause instanceof Error ? cause.message : String(cause); } +/** Whether an executable named `command` is on `PATH`, without running it. */ +function onPath(command: string): boolean { + const names = process.platform === "win32" ? [`${command}.exe`, `${command}.cmd`] : [command]; + for (const directory of (process.env["PATH"] ?? "").split(delimiter)) { + if (directory === "") continue; + for (const name of names) { + try { + accessSync(join(directory, name), constants.X_OK); + return true; + } catch { + // Not here; keep looking. + } + } + } + return false; +} + +function defaultProviders(settings: () => SecretSettings["providers"]): SecretProvider[] { + return onPath("op") ? [new OnePasswordProvider(() => settings()["1password"] ?? {})] : []; +} + /** The fields through which Pi's tools point at a full output file. */ const OutputFileFieldsSchema = Type.Object({ fullOutputPath: Type.Optional(Type.String({ minLength: 1 })), @@ -175,18 +340,33 @@ async function redactOutputFile(path: string): Promise { if (redacted !== text) await writeFile(path, redacted); } +function usage(outcome: SecretOutcome): string { + return `Use ${outcome.env === undefined ? "" : `$${outcome.env} in shell commands and `}${outcome.reference} in other tool arguments.`; +} + function outcomeLine(outcome: SecretOutcome): string { + const warning = outcome.warning === undefined ? "" : ` Warning: ${outcome.warning}`; switch (outcome.status) { case "provided": - return `${outcome.name}: provided. Use ${outcome.env === undefined ? "" : `$${outcome.env} in shell commands and `}${outcome.reference} in other tool arguments. Expires ${outcome.expires_at}.`; - case "held": + return `${outcome.name}: provided. ${usage(outcome)} Expires ${outcome.expires_at}.${warning}`; + case "granted": + return `${outcome.name}: granted from ${outcome.locator}, read again at each use. ${usage(outcome)} Expires ${outcome.expires_at}.${warning}`; + case "saved": + return `${outcome.name}: saved to ${outcome.locator}, and now read from there at each use. ${usage(outcome)} Expires ${outcome.expires_at}.`; + case "held": { + const from = outcome.locator === undefined ? "" : ` from ${outcome.locator}`; return outcome.expires_at === undefined ? `${outcome.name}: held by another session in this process. Use $${outcome.env} or ${outcome.reference}.` - : `${outcome.name}: held until ${outcome.expires_at}. Use $${outcome.env} or ${outcome.reference}.`; + : `${outcome.name}: held${from} until ${outcome.expires_at}. Use $${outcome.env} or ${outcome.reference}.`; + } case "skipped": return `${outcome.name}: skipped by the user.`; + case "denied": + return `${outcome.name}: denied by the user.`; + case "failed": + return `${outcome.name}: not granted: ${outcome.error}`; case "cancelled": - return `${outcome.name}: not provided; the user cancelled the Secret Request.`; + return `${outcome.name}: not provided; the user cancelled the request.`; case "forgotten": return `${outcome.name}: forgotten.`; case "not_held": @@ -194,10 +374,35 @@ function outcomeLine(outcome: SecretOutcome): string { } } -function outcomeResult(outcomes: SecretOutcome[], lead?: string) { +function availableLine(entry: AvailableEntry): string { + return entry.kind === "item" + ? `- ${entry.title} (${entry.category}) in ${entry.vault}; list its fields with secret_list item="${entry.id}"` + : `- ${entry.label} (${entry.type}) of ${entry.item}: ${entry.locator}`; +} + +/** A `secret_*` result's details, never a value; a type alias so it is assignable to JSON. */ +type SecretResultDetails = { + secrets: SecretOutcome[]; + available?: AvailableEntry[]; + notes?: string[]; +}; + +interface SecretResultParts { + outcomes: SecretOutcome[]; + available: AvailableEntry[]; + notes: string[]; + lead?: string; +} + +function secretResult({ outcomes, available, notes, lead }: SecretResultParts) { const lines = outcomes.map(outcomeLine); + if (available.length > 0) + lines.push("Available to request with secret_ask_grant:", ...available.map(availableLine)); + lines.push(...notes); const text = lead === undefined ? lines.join("\n") : [lead, ...lines].join("\n"); - const details = { secrets: outcomes }; + const details: SecretResultDetails = { secrets: outcomes }; + if (available.length > 0) details.available = available; + if (notes.length > 0) details.notes = notes; return { content: [{ type: "text" as const, text }], details, @@ -205,7 +410,26 @@ function outcomeResult(outcomes: SecretOutcome[], lead?: string) { }; } -/** Compose Pi Secret with an injectable agent directory and clock. */ +const outcomesResult = (outcomes: SecretOutcome[], lead?: string) => + secretResult( + lead === undefined + ? { outcomes, available: [], notes: [] } + : { outcomes, available: [], notes: [], lead }, + ); + +function describeLocation(provider: SecretProvider, description: LocatorDescription): string { + const account = description.account === undefined ? "" : ` (${description.account})`; + const type = description.fieldType === undefined ? "" : ` · ${description.fieldType}`; + return `${provider.label}${account} · ${description.vault} / ${description.item} / ${description.field}${type}`; +} + +function shortValueWarning(value: string): string | undefined { + return value.length < SHORT_SECRET_LENGTH + ? `the value is shorter than ${SHORT_SECRET_LENGTH} characters, so redacting it may also hide unrelated output.` + : undefined; +} + +/** Compose Pi Secret with an injectable agent directory, clock, and Secret Providers. */ export function createPiSecretExtension(options: PiSecretOptions = {}): ExtensionFactory { const getAgentDirectory = options.getAgentDirectory ?? getAgentDir; const now = options.now ?? Date.now; @@ -214,10 +438,13 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio const owner: SecretOwner = { description: "pi-secret" }; const live = new Map(); let lifetimeMinutes = DEFAULT_LIFETIME_MINUTES; + let providerSettings: SecretSettings["providers"] = {}; let generation = 0; const vault = () => new SecretVault(keyFilePath(getAgentDirectory()), now); + // Decided once at load, so the tool set and system prompt never change within a session. + const providers = (options.providers ?? defaultProviders)(() => providerSettings); - /** Stop holding one Secret here; its key is left for the caller to destroy or keep. */ + /** Stop holding one Secret here; a typed Secret's key is left for the caller to handle. */ function release(name: string): LiveSecret | undefined { const secret = live.get(name); if (secret === undefined) return undefined; @@ -225,32 +452,86 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio clearTimeout(secret.timer); revokeSecret(name, owner); const env = secretEnvName(name); - if (process.env[env] === secret.value) Reflect.deleteProperty(process.env, env); + if (secret.value !== undefined && process.env[env] === secret.value) + Reflect.deleteProperty(process.env, env); return secret; } - async function expire(name: string, keyId: string): Promise { - if (live.get(name)?.keyId !== keyId) return; - release(name); + /** Release a Secret and destroy its key, if it is a typed one. */ + async function discard(name: string): Promise { + const released = release(name); + if (released?.origin.kind === "typed") await vault().destroy([released.origin.keyId]); + } + + async function expire(secret: LiveSecret): Promise { + if (live.get(secret.name) !== secret) return; + release(secret.name); + if (secret.origin.kind !== "typed") return; // A key that cannot be deleted now is swept at the next session start. await vault() - .destroy([keyId]) + .destroy([secret.origin.keyId]) .catch(() => {}); } - function adopt(name: string, value: string, keyId: string, expiresAt: number): void { + function track( + name: string, + value: string | undefined, + origin: HeldSecretOrigin, + expiresAt: number, + ): LiveSecret { release(name); - const timer = setTimeout(() => void expire(name, keyId), Math.max(0, expiresAt - now())); - timer.unref(); - live.set(name, { name, value, keyId, expiresAt, timer }); + const secret: LiveSecret = { + name, + value, + origin, + expiresAt, + timer: setTimeout(() => void expire(secret), Math.max(0, expiresAt - now())), + }; + secret.timer.unref(); + live.set(name, secret); + if (value !== undefined) process.env[secretEnvName(name)] = value; + return secret; + } + + function adoptTyped(name: string, value: string, keyId: string, expiresAt: number): void { + track(name, value, { kind: "typed", keyId }, expiresAt); holdSecret(name, value, owner, expiresAt); - process.env[secretEnvName(name)] = value; + } + + /** Hold a Secret Grant, read again from its Secret Provider at each use (ADR-0001). */ + function adoptGrant( + name: string, + locator: string, + provider: SecretProvider, + expiresAt: number, + value: string | undefined, + ): void { + const secret = track(name, value, { kind: "granted", locator, provider }, expiresAt); + const refresh = async () => { + const read = await provider.read(locator); + if (live.get(name) === secret) { + secret.value = read; + process.env[secretEnvName(name)] = read; + } + return read; + }; + holdRefreshableSecret( + name, + owner, + value === undefined ? { refresh, expiresAt } : { refresh, expiresAt, value }, + ); + } + + /** Record a Secret Grant in the session: its locator and expiry, never its value. */ + function recordGrant(name: string, locator: string, expiresAt: number): void { + const grant: SecretGrant = { version: 1, name, locator, expiresAt }; + pi.appendEntry(GRANT_ENTRY_TYPE, grant); } /** End every Secret whose lifetime has passed, as its timer would. */ async function expireDue(): Promise { const due = [...live.values()].filter((secret) => secret.expiresAt <= now()); - for (const secret of due) await expire(secret.name, secret.keyId); + for (const secret of due) await expire(secret); } function releaseAll(): void { @@ -258,6 +539,14 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio for (const name of live.keys()) release(name); } + type Recorded = + | { readonly kind: "sealed"; readonly data: SealedSecret } + | { + readonly kind: "grant"; + readonly data: SecretGrant; + } + | { readonly kind: "forgotten" }; + async function restore(context: ExtensionContext): Promise { const current = ++generation; releaseAll(); @@ -273,19 +562,44 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio sessionManager: context.sessionManager, }); lifetimeMinutes = settings.lifetimeMinutes; + providerSettings = settings.providers; if (warning !== undefined) warn(warning); - const latest = new Map(); + // The newest entry for each name wins, whether it entered, granted, or forgot a Secret. + const latest = new Map(); for (const entry of context.sessionManager.getBranch()) { - if (entry.type !== "custom" || entry.customType !== SECRET_ENTRY_TYPE) continue; - if (Value.Check(SealedSecretSchema, entry.data)) latest.set(entry.data.name, entry.data); + if (entry.type !== "custom") continue; + if (entry.customType === SECRET_ENTRY_TYPE && Value.Check(SealedSecretSchema, entry.data)) + latest.set(entry.data.name, { kind: "sealed", data: entry.data }); + else if ( + entry.customType === GRANT_ENTRY_TYPE && + Value.Check(SecretGrantSchema, entry.data) + ) + latest.set(entry.data.name, { kind: "grant", data: entry.data }); + else if (entry.customType === FORGET_ENTRY_TYPE && Value.Check(ForgottenSchema, entry.data)) + latest.set(entry.data.name, { kind: "forgotten" }); } try { await vault().destroy([]); } catch (cause) { warn(`cannot delete expired Secret keys: ${errorMessage(cause)}`); } + if (current !== generation) return; // Each Secret is restored on its own, so one unreadable entry cannot hide the others. - for (const sealed of latest.values()) { + for (const recorded of latest.values()) { + if (recorded.kind === "forgotten") continue; + if (recorded.kind === "grant") { + const { name, locator, expiresAt } = recorded.data; + if (expiresAt <= now()) continue; + const provider = providerFor(providers, locator); + if (provider === undefined) { + warn(`cannot restore Secret Grant ${name}: no Secret Provider reads ${locator}`); + continue; + } + // Read lazily, at first use, so resuming never prompts the Secret Provider. + adoptGrant(name, locator, provider, expiresAt, undefined); + continue; + } + const sealed = recorded.data; let value: string | undefined; try { value = await vault().open(sealed); @@ -293,7 +607,7 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio warn(`cannot restore Secret ${sealed.name}: ${errorMessage(cause)}`); } if (current !== generation) return; - if (value !== undefined) adopt(sealed.name, value, sealed.keyId, sealed.expiresAt); + if (value !== undefined) adoptTyped(sealed.name, value, sealed.keyId, sealed.expiresAt); } } @@ -310,14 +624,25 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio // SAFETY: tool arguments are parsed JSON, validated against the tool's schema. const input = event.input as { [key: string]: JsonValue }; const names = referencedSecretNames(input); - if (names.size === 0) return undefined; const reason = unresolvedReason([...names]); if (reason !== undefined) return { block: true, reason }; - // Pi passes the same arguments object on to the tool: resolve in place. - Object.assign( - input, - resolvePlaceholders(input, (name) => resolveSecret(name) ?? ""), - ); + // Granted Secrets are read again now, in either reference form (ADR-0001). + for (const name of names) { + let value: string | undefined; + try { + value = await refreshSecret(name); + } catch (cause) { + return { + block: true, + reason: `The call did not run. Pi Secret could not read ${secretReference(name)} from its Secret Provider: ${errorMessage(cause)}. Ask the user to unlock or sign in to it, or call secret_ask_grant again. ${TROUBLESHOOTING_HINT}`, + }; + } + if (value === undefined) + return { block: true, reason: unresolvedReason([name]) ?? "The call did not run." }; + } + // Pi passes the same arguments object on to the tool: rewrite in place. + const rewritten = rewriteReferences(input, (name) => resolveSecret(name) ?? ""); + if (rewritten !== input) Object.assign(input, rewritten); return undefined; }); @@ -353,6 +678,11 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio return messages === event.messages ? undefined : { messages }; }); + const needsTerminal = (tool: string, what: string) => + new Error( + `${tool} needs Pi's interactive terminal, where the user ${what}; it is unavailable in print, JSON, and RPC modes and in Child Agents. Ask the user how they want to provide the Secret.\n\n${TROUBLESHOOTING_HINT}`, + ); + const ask = defineTool({ name: "secret_ask", label: "Secret Ask", @@ -374,10 +704,8 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio renderSecretResult(result, options, theme, context), async execute(_toolCallId, params, signal, _onUpdate, context) { if (context.mode !== "tui") - throw new Error( - `secret_ask needs Pi's interactive terminal, where values are entered in a masked prompt; it is unavailable in print, JSON, and RPC modes and in Child Agents. Ask the user how they want to provide the Secret.\n\n${TROUBLESHOOTING_HINT}`, - ); - const reason = params.reason?.trim() === "" ? undefined : params.reason; + throw needsTerminal("secret_ask", "enters values in a masked prompt"); + const reason = shownReason(params.reason); const answer = await context.ui.custom( (tui, theme, keybindings, done) => { const dialog = new SecretDialog({ @@ -394,7 +722,7 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio }, ); if (answer === undefined || answer.kind === "cancelled") - return outcomeResult( + return outcomesResult( params.names.map((name) => ({ name, status: "cancelled" })), "The user cancelled the Secret Request. Do not ask again unless they want to.", ); @@ -422,9 +750,8 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio { cause }, ); } - const replaced = release(name); - if (replaced !== undefined) await vault().destroy([replaced.keyId]); - adopt(name, value, sealed.keyId, expiresAt); + await discard(name); + adoptTyped(name, value, sealed.keyId, expiresAt); outcomes.push({ name, status: "provided", @@ -433,7 +760,148 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio expires_at: iso(expiresAt), }); } - return outcomeResult(outcomes); + return outcomesResult(outcomes); + }, + }); + + /** Ask the user to approve rows in one dialog; undefined when they cancel. */ + async function approve( + context: Parameters[4], + signal: AbortSignal | undefined, + dialog: { title: string; reason: string | undefined; explanation: string }, + rows: { name: string; detail: string }[], + ): Promise | undefined> { + const answer = await context.ui.custom( + (tui, theme, keybindings, done) => { + const component = new ApprovalDialog({ + ...dialog, + rows, + theme, + keybindings, + requestRender: () => tui.requestRender(), + done, + }); + signal?.addEventListener("abort", () => component.cancel(), { once: true }); + return component; + }, + ); + return answer === undefined || answer.kind === "cancelled" ? undefined : answer.approved; + } + + const schemes = () => providers.map((provider) => `${provider.scheme}://`).join(", "); + + const askGrant = defineTool({ + name: "secret_ask_grant", + label: "Secret Ask Grant", + description: + "Ask the user to grant Secrets from a Secret Provider such as 1Password, each a Secret Name and the Secret Locator (such as op://vault/item/field) its value is read from at each use. Find locators with secret_list. Returns how to reference each one; never its value.", + promptGuidelines: [SECRET_PROVIDER_GUIDELINE], + parameters: AskGrantParameters, + annotations: { + readOnlyHint: false, + destructiveHint: false, + idempotentHint: false, + openWorldHint: true, + }, + executionMode: "sequential", + renderCall: (params, theme, context) => + renderSecretCall( + "secret_ask_grant", + { names: params.grants?.map((grant) => grant.name) ?? [], reason: params.reason }, + theme, + context, + ), + renderResult: (result, options, theme, context) => + renderSecretResult(result, options, theme, context), + async execute(_toolCallId, params, signal, _onUpdate, context) { + if (context.mode !== "tui") + throw needsTerminal("secret_ask_grant", "approves each Secret Grant"); + const names = params.grants.map((grant) => grant.name); + const duplicate = names.find((name, index) => names.indexOf(name) !== index); + if (duplicate !== undefined) + throw new Error(`Secret Name ${duplicate} appears in more than one grant`); + const outcomes: SecretOutcome[] = params.grants.map((grant) => ({ + name: grant.name, + status: "failed", + locator: grant.locator, + })); + const pending: { + index: number; + provider: SecretProvider; + description: LocatorDescription; + }[] = []; + for (const [index, grant] of params.grants.entries()) { + const outcome = outcomes[index]; + if (outcome === undefined) continue; + const provider = providerFor(providers, grant.locator); + if (provider === undefined) { + outcome.error = `no Secret Provider reads ${JSON.stringify(grant.locator)}; available schemes: ${schemes()}`; + continue; + } + try { + pending.push({ + index, + provider, + description: await provider.describe(grant.locator, signal), + }); + } catch (cause) { + outcome.error = errorMessage(cause); + } + } + if (pending.length === 0) return outcomesResult(outcomes); + const approved = await approve( + context, + signal, + { + title: "Grant request", + reason: shownReason(params.reason), + explanation: `Each allowed Secret is read from its Secret Provider at each use for ${lifetimeText(lifetimeMinutes)}, and never shown to the model.`, + }, + pending.map(({ index, provider, description }) => ({ + name: params.grants[index]?.name ?? "", + detail: describeLocation(provider, description), + })), + ); + if (approved === undefined) { + for (const { index } of pending) { + const outcome = outcomes[index]; + if (outcome !== undefined) outcome.status = "cancelled"; + } + return outcomesResult( + outcomes, + "The user cancelled the Grant Request. Do not ask again unless they want to.", + ); + } + for (const [row, { index, provider }] of pending.entries()) { + const outcome = outcomes[index]; + const grant = params.grants[index]; + if (outcome === undefined || grant === undefined) continue; + if (!approved.has(row)) { + outcome.status = "denied"; + continue; + } + let value: string; + try { + // Read now, so a wrong locator or a locked Secret Provider shows up here. + value = await provider.read(grant.locator, signal); + } catch (cause) { + outcome.error = errorMessage(cause); + continue; + } + const expiresAt = now() + lifetimeMinutes * 60_000; + recordGrant(grant.name, grant.locator, expiresAt); + await discard(grant.name); + adoptGrant(grant.name, grant.locator, provider, expiresAt, value); + Object.assign(outcome, { + status: "granted", + reference: secretReference(grant.name), + env: secretEnvName(grant.name), + expires_at: iso(expiresAt), + }); + const warning = shortValueWarning(value); + if (warning !== undefined) outcome.warning = warning; + } + return outcomesResult(outcomes); }, }); @@ -441,38 +909,146 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio name: "secret_list", label: "Secret List", description: - "List the Secrets this session can use and when each expires; never their values.", - parameters: Type.Object({}), + "List the Secrets this session can use and when each expires, and search Secret Providers such as 1Password for items and fields to request with secret_ask_grant; never values.", + parameters: ListParameters, annotations: { readOnlyHint: true, destructiveHint: false, idempotentHint: true, - openWorldHint: false, + openWorldHint: providers.length > 0, }, renderCall: (_params, theme, context) => renderSecretCall("secret_list", {}, theme, context), renderResult: (result, options, theme, context) => renderSecretResult(result, options, theme, context), - async execute() { + async execute(_toolCallId, params, signal) { await expireDue(); - const outcomes: SecretOutcome[] = heldSecretNames() - .toSorted() - .map((name) => { - const own = live.get(name); - const outcome: SecretOutcome = { - name, - status: "held", - reference: secretReference(name), - env: secretEnvName(name), + const query = params.query?.toLowerCase(); + const matches = (...texts: (string | undefined)[]) => + query === undefined || texts.some((text) => text?.toLowerCase().includes(query)); + const outcomes: SecretOutcome[] = + params.status === "available" + ? [] + : heldSecretNames() + .toSorted() + .flatMap((name): SecretOutcome[] => { + const own = live.get(name); + const locator = own?.origin.kind === "granted" ? own.origin.locator : undefined; + const providerId = + own?.origin.kind === "granted" ? own.origin.provider.id : undefined; + if (params.provider !== undefined && params.provider !== providerId) return []; + if (!matches(name, locator)) return []; + const outcome: SecretOutcome = { + name, + status: "held", + reference: secretReference(name), + env: secretEnvName(name), + }; + if (own !== undefined) outcome.expires_at = iso(own.expiresAt); + if (locator !== undefined) outcome.locator = locator; + return [outcome]; + }); + const available: AvailableEntry[] = []; + const notes: string[] = []; + if (params.status !== "held") { + for (const provider of providers) { + if (params.provider !== undefined && params.provider !== provider.id) continue; + const search: Parameters[0] = { + limit: params.limit ?? DEFAULT_LIST_LIMIT, }; - if (own !== undefined) outcome.expires_at = iso(own.expiresAt); - return outcome; - }); - return outcomeResult( - outcomes, - outcomes.length === 0 - ? "No Secrets are held. Call secret_ask to request one." - : undefined, + const filters = { query: params.query, vault: params.vault, item: params.item }; + for (const [key, value] of Object.entries(filters)) + if (value !== undefined) Object.assign(search, { [key]: value }); + try { + const listing = await provider.search(search, signal); + available.push(...listing.entries); + if (listing.truncated) + notes.push( + `More ${provider.label} entries matched; narrow them with query, vault, or item, or raise limit.`, + ); + } catch (cause) { + notes.push(`Could not search ${provider.label}: ${errorMessage(cause)}`); + } + } + } + const empty = outcomes.length === 0 && available.length === 0 && notes.length === 0; + const result: SecretResultParts = { outcomes, available, notes }; + if (empty) + result.lead = + providers.length === 0 + ? "No Secrets are held. Call secret_ask to request one." + : "No Secrets matched. Call secret_ask to request one, or search a Secret Provider with other filters."; + return secretResult(result); + }, + }); + + const save = defineTool({ + name: "secret_save", + label: "Secret Save", + description: + "Save a Secret the user typed into this session into a new item in a Secret Provider such as 1Password, after the user approves. The Secret is then read from there at each use, with its original expiry.", + parameters: SaveParameters, + annotations: { + readOnlyHint: false, + destructiveHint: false, + idempotentHint: false, + openWorldHint: true, + }, + executionMode: "sequential", + renderCall: (params, theme, context) => + renderSecretCall("secret_save", { names: [params.name] }, theme, context), + renderResult: (result, options, theme, context) => + renderSecretResult(result, options, theme, context), + async execute(_toolCallId, params, signal, _onUpdate, context) { + if (context.mode !== "tui") throw needsTerminal("secret_save", "approves each save"); + await expireDue(); + const secret = live.get(params.name); + if (secret?.origin.kind !== "typed" || secret.value === undefined) + throw new Error( + `${params.name} is not a Secret the user typed into this session; only those can be saved. Granted Secrets already live in a Secret Provider.`, + ); + const { value, expiresAt, origin } = secret; + const provider = providerFor(providers, params.locator); + if (provider === undefined) + throw new Error( + `No Secret Provider saves to ${JSON.stringify(params.locator)}; available schemes: ${schemes()}`, + ); + const request: Omit = + params.category === undefined + ? { locator: params.locator } + : { locator: params.locator, category: params.category }; + const description = await provider.describeSave(request, signal); + const approved = await approve( + context, + signal, + { + title: "Save Secret", + reason: undefined, + explanation: `Pi Secret will create a new ${provider.label} item holding this value. The Secret is then read from there at each use, and still expires at ${iso(expiresAt)}.`, + }, + [{ name: params.name, detail: describeLocation(provider, description) }], ); + if (approved === undefined || !approved.has(0)) + return outcomesResult([ + { name: params.name, status: approved === undefined ? "cancelled" : "denied" }, + ]); + const locator = await provider.save({ ...request, value }, signal); + recordGrant(params.name, locator, expiresAt); + adoptGrant(params.name, locator, provider, expiresAt, value); + // The typed copy is no longer needed. The newer grant entry wins on restore either way, so a + // key that cannot be deleted now only waits for its expiry sweep. + await vault() + .destroy([origin.keyId]) + .catch(() => {}); + return outcomesResult([ + { + name: params.name, + status: "saved", + locator, + reference: secretReference(params.name), + env: secretEnvName(params.name), + expires_at: iso(expiresAt), + }, + ]); }, }); @@ -485,7 +1061,8 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio outcomes.push({ name, status: "not_held" }); continue; } - destroyed.push(released.keyId); + if (released.origin.kind === "typed") destroyed.push(released.origin.keyId); + pi.appendEntry(FORGET_ENTRY_TYPE, { version: 1, name }); outcomes.push({ name, status: "forgotten" }); } try { @@ -502,7 +1079,7 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio const forgetTool = defineTool({ name: "secret_forget", label: "Secret Forget", - description: "Forget Secrets now, before their lifetime ends.", + description: "Forget Secrets, typed or granted, now, before their lifetime ends.", parameters: ForgetParameters, annotations: { readOnlyHint: false, @@ -516,12 +1093,14 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio renderSecretResult(result, options, theme, context), async execute(_toolCallId, params) { await expireDue(); - return outcomeResult(await forget(params.names)); + return outcomesResult(await forget(params.names)); }, }); pi.registerTool(acceptNullForOptionalArguments(ask)); + if (providers.length > 0) pi.registerTool(acceptNullForOptionalArguments(askGrant)); pi.registerTool(acceptNullForOptionalArguments(list)); + if (providers.length > 0) pi.registerTool(acceptNullForOptionalArguments(save)); pi.registerTool(acceptNullForOptionalArguments(forgetTool)); pi.registerCommand("secret", { @@ -544,7 +1123,10 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio if (action === "list") { const lines = [...live.values()] .toSorted((a, b) => a.name.localeCompare(b.name)) - .map((secret) => `${secret.name}: expires ${iso(secret.expiresAt)}`); + .map((secret) => { + const from = secret.origin.kind === "granted" ? ` from ${secret.origin.locator}` : ""; + return `${secret.name}${from}: expires ${iso(secret.expiresAt)}`; + }); context.ui.notify(lines.length === 0 ? "No Secrets are held" : lines.join("\n"), "info"); return; } diff --git a/packages/pi-secret/src/secret-dialog.ts b/packages/pi-secret/src/secret-dialog.ts index 354d2559..61f8d0c8 100644 --- a/packages/pi-secret/src/secret-dialog.ts +++ b/packages/pi-secret/src/secret-dialog.ts @@ -44,7 +44,7 @@ function graphemes(text: string): string[] { return Array.from(graphemeSegmenter.segment(text), (segment) => segment.segment); } -function lifetimeText(minutes: number): string { +export function lifetimeText(minutes: number): string { if (minutes % 60 === 0) { const hours = minutes / 60; return `${hours} hour${hours === 1 ? "" : "s"}`; diff --git a/packages/pi-secret/src/secret-provider.ts b/packages/pi-secret/src/secret-provider.ts new file mode 100644 index 00000000..f0b332bb --- /dev/null +++ b/packages/pi-secret/src/secret-provider.ts @@ -0,0 +1,91 @@ +/** + * The internal seam between Pi Secret and the Secret Providers it reads Secret Grants from and + * saves typed Secrets into. Each Secret Provider owns one Secret Locator scheme, such as `op` for + * 1Password References. Providers never return a value except from {@link SecretProvider.read}. + */ + +/** Where a Secret Locator points, as the user sees it when approving a grant or a save. */ +export interface LocatorDescription { + /** The Secret Locator, as the agent wrote it. */ + readonly locator: string; + readonly vault: string; + readonly item: string; + readonly field: string; + /** The field's type, such as `concealed` or `text`, when known. */ + readonly fieldType?: string; + /** The account the provider reads from, when it names one. */ + readonly account?: string; +} + +/** A Secret Provider search; every filter is optional and case-insensitive. */ +export interface ProviderSearch { + /** Matches item titles or field labels. */ + readonly query?: string; + readonly vault?: string; + /** List this item's fields instead of items. */ + readonly item?: string; + readonly limit: number; +} + +/** One searchable entry: an item, or a field of one item with the locator to request it by. */ +export type AvailableEntry = + | { + readonly kind: "item"; + readonly provider: string; + readonly id: string; + readonly title: string; + readonly vault: string; + readonly category: string; + } + | { + readonly kind: "field"; + readonly provider: string; + readonly item: string; + readonly vault: string; + readonly label: string; + readonly type: string; + readonly locator: string; + }; + +/** What a search found, and whether more matched than `limit`. */ +export interface ProviderListing { + readonly entries: AvailableEntry[]; + readonly truncated: boolean; +} + +/** A typed Secret to save into a new item. */ +export interface SaveRequest { + readonly locator: string; + readonly value: string; + readonly category?: string; +} + +export interface SecretProvider { + /** Stable id the agent filters by, such as `1password`. */ + readonly id: string; + /** Display name, such as `1Password`. */ + readonly label: string; + /** The Secret Locator scheme this provider owns, without `://`, such as `op`. */ + readonly scheme: string; + /** Check that a locator points at an existing, permitted field, and say where it points. */ + describe(locator: string, signal?: AbortSignal): Promise; + /** Check that a save can create the item the locator names, and say where it would go. */ + describeSave( + request: Omit, + signal?: AbortSignal, + ): Promise; + /** The current value at a locator. */ + read(locator: string, signal?: AbortSignal): Promise; + search(search: ProviderSearch, signal?: AbortSignal): Promise; + /** Create a new item holding the value, and return a locator for it that survives renames. */ + save(request: SaveRequest, signal?: AbortSignal): Promise; +} + +/** The provider that owns a locator's scheme, if any. */ +export function providerFor( + providers: readonly SecretProvider[], + locator: string, +): SecretProvider | undefined { + const scheme = /^([a-z][a-z0-9+.-]*):\/\//i.exec(locator)?.[1]?.toLowerCase(); + return providers.find((provider) => provider.scheme === scheme); +} diff --git a/packages/pi-secret/src/secret-references.ts b/packages/pi-secret/src/secret-references.ts index bbb8eac0..48fa70e9 100644 --- a/packages/pi-secret/src/secret-references.ts +++ b/packages/pi-secret/src/secret-references.ts @@ -9,59 +9,111 @@ export function secretEnvName(name: string): string { return `${SECRET_ENV_PREFIX}${name}`; } -/** `{{secret:NAME}}`, with anything between the colon and the braces, so malformed names are caught. */ -const PLACEHOLDER = /\{\{secret:([^{}]*)\}\}/g; -/** `PI_SECRET_NAME` as a whole word, however a shell or program spells the expansion around it. - * The bare prefix names no Secret, so a command that only mentions it, such as `env | grep` for the - * prefix, is not a reference. +/** + * A Secret Reference in either form, optionally escaped by a backslash right before it: + * - `{{secret:NAME}}`, with anything between the colon and the braces, so malformed names are caught; + * - `PI_SECRET_NAME` as a whole word, however a shell or program spells the expansion around it. The + * bare prefix names no Secret, so a command that only mentions it, such as `env | grep` for the + * prefix, is not a reference. + * + * An escaped reference is literal text: it names no Secret, and the backslash is removed before + * the tool runs, so documentation and tests can contain the syntax. A shell's own escape of an + * expansion, `\$PI_SECRET_NAME` or `\${PI_SECRET_NAME}`, is literal to the shell too, so it names no Secret + * and is left exactly as written. */ -const ENV_REFERENCE = /(? void): void { - // oxlint-disable-next-line anti-slop/no-runtime-typeof -- SAFETY: tool arguments are model-supplied JSON; only strings carry references. - if (typeof value === "string") { - visit(value); - return; - } - // oxlint-disable-next-line anti-slop/no-runtime-typeof -- SAFETY: tool arguments are model-supplied JSON; other primitives carry no references. - if (value === null || typeof value !== "object") return; - for (const item of Object.values(value)) visitStrings(item, visit); +/** One reference found in text. */ +interface ReferenceMatch { + readonly escaped: boolean; + readonly name: string; + readonly form: "placeholder" | "env"; +} + +/** `\$` or `\${` right before an expansion: the shell prints it literally. */ +const SHELL_ESCAPE = /\\\$\{?$/; + +/** + * The reference matched at `index` of `text`, or undefined when a shell escape makes it literal. + */ +function matchOf( + groups: (string | undefined)[], + text: string, + index: number, +): ReferenceMatch | undefined { + const [escape, placeholder, env] = groups; + if (placeholder !== undefined) + return { escaped: escape === "\\", name: placeholder, form: "placeholder" }; + if (escape !== "\\" && SHELL_ESCAPE.test(text.slice(Math.max(0, index - 3), index))) + return undefined; + return { escaped: escape === "\\", name: env ?? "", form: "env" }; } /** * Every Secret Name a value references, in either form, including names that are not valid Secret * Names (such as `{{secret:lower}}`), so a call naming one can be refused rather than run as typed. + * Escaped references name nothing. */ export function referencedSecretNames(value: JsonValue): Set { const names = new Set(); visitStrings(value, (text) => { - for (const match of text.matchAll(PLACEHOLDER)) names.add(match[1] ?? ""); - for (const match of text.matchAll(ENV_REFERENCE)) names.add(match[1] ?? ""); + for (const found of text.matchAll(REFERENCE)) { + const match = matchOf(found.slice(1), text, found.index); + if (match !== undefined && !match.escaped) names.add(match.name); + } }); return names; } +function visitStrings(value: JsonValue, visit: (text: string) => void): void { + // oxlint-disable-next-line anti-slop/no-runtime-typeof -- SAFETY: tool arguments are model-supplied JSON; only strings carry references. + if (typeof value === "string") { + visit(value); + return; + } + // oxlint-disable-next-line anti-slop/no-runtime-typeof -- SAFETY: tool arguments are model-supplied JSON; other primitives carry no references. + if (value === null || typeof value !== "object") return; + for (const item of Object.values(value)) visitStrings(item, visit); +} + /** Whether `name` is a valid Secret Name. */ export function isSecretName(name: string): boolean { return SECRET_NAME_PATTERN.test(name); } /** - * A copy of `record` with every `{{secret:NAME}}` replaced by `resolve(NAME)`. Environment - * references are left as written: the process environment resolves them. + * `record` with every `{{secret:NAME}}` replaced by `resolve(NAME)` and every escaped reference + * turned into literal text without its backslash. Environment references are left as written: + * the process environment resolves them. Returns `record` itself when nothing changed. */ -export function resolvePlaceholders( - record: JsonValue, - resolve: (name: string) => string, -): JsonValue { +export function rewriteReferences(record: JsonValue, resolve: (name: string) => string): JsonValue { // oxlint-disable-next-line anti-slop/no-runtime-typeof -- SAFETY: tool arguments are model-supplied JSON; only strings carry references. if (typeof record === "string") { - return record.replace(PLACEHOLDER, (_match, name: string) => resolve(name)); + let text = ""; + let last = 0; + for (const found of record.matchAll(REFERENCE)) { + const [whole] = found; + const match = matchOf(found.slice(1), record, found.index); + text += record.slice(last, found.index); + if (match === undefined || (!match.escaped && match.form === "env")) text += whole; + else text += match.escaped ? whole.slice(1) : resolve(match.name); + last = found.index + whole.length; + } + text += record.slice(last); + return text === record ? record : text; } // oxlint-disable-next-line anti-slop/no-runtime-typeof -- SAFETY: tool arguments are model-supplied JSON; other primitives carry no references. if (record === null || typeof record !== "object") return record; - if (Array.isArray(record)) return record.map((item) => resolvePlaceholders(item, resolve)); - return Object.fromEntries( - Object.entries(record).map(([key, item]) => [key, resolvePlaceholders(item, resolve)]), - ); + let changed = false; + const rewrite = (item: JsonValue) => { + const next = rewriteReferences(item, resolve); + if (next !== item) changed = true; + return next; + }; + if (Array.isArray(record)) { + const items = record.map(rewrite); + return changed ? items : record; + } + const entries = Object.entries(record).map(([key, item]) => [key, rewrite(item)] as const); + return changed ? Object.fromEntries(entries) : record; } diff --git a/packages/pi-secret/src/secret-render.ts b/packages/pi-secret/src/secret-render.ts index ebb587c5..9828ecaf 100644 --- a/packages/pi-secret/src/secret-render.ts +++ b/packages/pi-secret/src/secret-render.ts @@ -17,7 +17,7 @@ export type SecretRenderTheme = Pick; /** What a `secret_*` call row shows: Secret Names only, since the tools never take values. */ export interface SecretCallArguments { readonly names?: readonly string[]; - readonly reason?: string | null; + readonly reason?: string | null | undefined; } /** A `secret_*` call row: the tool name, the Secret Names it targets, then the reason. */ diff --git a/packages/pi-secret/src/secret-settings.ts b/packages/pi-secret/src/secret-settings.ts index 17b73b36..bf5bff93 100644 --- a/packages/pi-secret/src/secret-settings.ts +++ b/packages/pi-secret/src/secret-settings.ts @@ -1,11 +1,14 @@ import type { SessionManager, SettingsManager } from "@earendil-works/pi-coding-agent"; import { defineLayeredSettings } from "@ian-pascoe/pi-utils/layered-settings"; import { Type } from "typebox"; +import type { OnePasswordSettings } from "./one-password-provider.js"; /** Pi Secret's settings: the `secret` namespace of Pi's global and trusted-project settings. */ export interface SecretSettings { /** The Secret Lifetime, in minutes, counted from when the user enters a value. */ lifetimeMinutes: number; + /** Secret Provider settings; a provider without settings uses its defaults. */ + providers: { "1password"?: OnePasswordSettings }; } export const DEFAULT_LIFETIME_MINUTES = 15; @@ -15,8 +18,18 @@ const layered = defineLayeredSettings({ label: "Pi Secret", schema: Type.Object({ lifetimeMinutes: Type.Optional(Type.Integer({ minimum: 1, maximum: 1440 })), + providers: Type.Optional( + Type.Object({ + "1password": Type.Optional( + Type.Object({ + account: Type.Optional(Type.String({ minLength: 1 })), + vaults: Type.Optional(Type.Array(Type.String({ minLength: 1 }), { minItems: 1 })), + }), + ), + }), + ), }), - defaults: { lifetimeMinutes: DEFAULT_LIFETIME_MINUTES } satisfies SecretSettings, + defaults: { lifetimeMinutes: DEFAULT_LIFETIME_MINUTES, providers: {} } satisfies SecretSettings, }); /** Settings as read, with why they fell back to the defaults, if they did. */ @@ -35,8 +48,12 @@ export function readSecretSettings(session: { } catch (cause) { const message = cause instanceof Error ? cause.message : String(cause); return { - settings: { lifetimeMinutes: DEFAULT_LIFETIME_MINUTES }, - warning: `${message}; using a ${DEFAULT_LIFETIME_MINUTES}-minute Secret Lifetime`, + // Fail closed: an unreadable vault allowlist must not widen to every vault. + settings: { + lifetimeMinutes: DEFAULT_LIFETIME_MINUTES, + providers: { "1password": { vaults: [] } }, + }, + warning: `${message}; using a ${DEFAULT_LIFETIME_MINUTES}-minute Secret Lifetime and no 1Password vaults until it is fixed`, }; } } diff --git a/packages/pi-secret/test/fixtures/fake-provider.ts b/packages/pi-secret/test/fixtures/fake-provider.ts new file mode 100644 index 00000000..c9242e41 --- /dev/null +++ b/packages/pi-secret/test/fixtures/fake-provider.ts @@ -0,0 +1,77 @@ +import type { + AvailableEntry, + LocatorDescription, + ProviderListing, + ProviderSearch, + SaveRequest, + SecretProvider, +} from "../../src/secret-provider.js"; + +/** A Secret Provider over an in-memory map, with `fake://vault/item/field` locators. */ +export class FakeProvider implements SecretProvider { + readonly id = "fake"; + readonly label = "Fake Vault"; + readonly scheme = "fake"; + /** Values by locator; a function is called at each read, like a one-time password. */ + readonly values = new Map string)>(); + /** Every locator read, in order. */ + readonly reads: string[] = []; + readonly saved: SaveRequest[] = []; + /** Set to make every read and search fail, as a locked Secret Provider does. */ + locked = false; + + constructor(values: Record string)> = {}) { + for (const [locator, value] of Object.entries(values)) this.values.set(locator, value); + } + + private where(locator: string): LocatorDescription { + const [vault = "", item = "", field = ""] = locator.replace("fake://", "").split("/"); + return { locator, vault, item, field, fieldType: "concealed" }; + } + + async describe(locator: string): Promise { + if (!this.values.has(locator)) throw new Error(`no field at ${locator}`); + return this.where(locator); + } + + async describeSave(request: Omit): Promise { + if (this.values.has(request.locator)) throw new Error("that item already exists"); + return this.where(request.locator); + } + + async read(locator: string): Promise { + this.reads.push(locator); + if (this.locked) throw new Error("Fake Vault is locked"); + const value = this.values.get(locator); + if (value === undefined) throw new Error(`no field at ${locator}`); + return value instanceof Function ? value() : value; + } + + async search(search: ProviderSearch): Promise { + if (this.locked) throw new Error("Fake Vault is locked"); + const entries: AvailableEntry[] = [...this.values.keys()] + .filter((locator) => search.query === undefined || locator.includes(search.query)) + .map((locator) => { + const { vault, item, field } = this.where(locator); + return search.item === undefined + ? { kind: "item", provider: this.id, id: item, title: item, vault, category: "password" } + : { + kind: "field", + provider: this.id, + item, + vault, + label: field, + type: "concealed", + locator, + }; + }); + return { entries: entries.slice(0, search.limit), truncated: entries.length > search.limit }; + } + + async save(request: SaveRequest): Promise { + this.saved.push(request); + const locator = `fake://Dev/item-${this.saved.length}/credential`; + this.values.set(locator, request.value); + return locator; + } +} diff --git a/packages/pi-secret/test/fixtures/secret-harness.ts b/packages/pi-secret/test/fixtures/secret-harness.ts index ecd96cd5..c1f5d109 100644 --- a/packages/pi-secret/test/fixtures/secret-harness.ts +++ b/packages/pi-secret/test/fixtures/secret-harness.ts @@ -29,6 +29,7 @@ import { getKeybindings, type Component } from "@earendil-works/pi-tui"; import { taggedTheme } from "@ian-pascoe/pi-utils/ui-testing"; import { Type } from "typebox"; import { createPiSecretExtension } from "../../src/pi-secret-extension.js"; +import type { SecretProvider } from "../../src/secret-provider.js"; /** Raw terminal input, one element per keypress or paste, typed into one Secret Request dialog. */ export type DialogScript = readonly string[]; @@ -63,6 +64,8 @@ export interface SecretHarnessOptions { builtinTools?: boolean; /** Extensions loaded before Pi Secret. */ before?: ExtensionFactory[]; + /** Secret Providers; none by default, so tests never reach a real 1Password CLI. */ + providers?: SecretProvider[]; } /** What a test tool received, after every `tool_call` handler ran. */ @@ -158,7 +161,11 @@ export async function createSecretHarness(options: SecretHarnessOptions = {}) { extensionFactories: [ ...(options.before ?? []), testTools(received), - createPiSecretExtension({ getAgentDirectory: () => dir, now: () => clock.now }), + createPiSecretExtension({ + getAgentDirectory: () => dir, + now: () => clock.now, + providers: () => options.providers ?? [], + }), ], systemPromptOverride: () => "Standing instructions: finish the user's task.", }); diff --git a/packages/pi-secret/test/null-optional-arguments.test.ts b/packages/pi-secret/test/null-optional-arguments.test.ts index c7f77301..7a197833 100644 --- a/packages/pi-secret/test/null-optional-arguments.test.ts +++ b/packages/pi-secret/test/null-optional-arguments.test.ts @@ -3,25 +3,40 @@ import { recordToolRegistrations, } from "@ian-pascoe/pi-utils/tool-testing"; import { expect, test } from "vitest"; -import { createPiSecretExtension } from "../src/pi-secret-extension.js"; +import { createPiSecretExtension, SECRET_PROVIDER_GUIDELINE } from "../src/pi-secret-extension.js"; +import { FakeProvider } from "./fixtures/fake-provider.js"; -function registeredTools() { +function registeredTools(withProvider = true) { const { pi, tools } = recordToolRegistrations(); - void createPiSecretExtension()(pi); + void createPiSecretExtension({ providers: () => (withProvider ? [new FakeProvider()] : []) })(pi); return tools; } -test("registers exactly the secret tools", () => { +test("registers the Secret Provider tools only when a Secret Provider is available at load", () => { + expect(registeredTools(false).map(({ name }) => name)).toEqual([ + "secret_ask", + "secret_list", + "secret_forget", + ]); expect(registeredTools().map(({ name }) => name)).toEqual([ "secret_ask", + "secret_ask_grant", "secret_list", + "secret_save", "secret_forget", ]); }); +test("adds the Secret Provider guidance through secret_ask_grant, so it appears only with a provider", () => { + const grant = registeredTools().find(({ name }) => name === "secret_ask_grant"); + expect(grant).toMatchObject({ promptGuidelines: [SECRET_PROVIDER_GUIDELINE] }); +}); + test.each([ ["secret_ask", { names: ["API_KEY"] }, ["reason"]], - ["secret_list", {}, []], + ["secret_ask_grant", { grants: [{ name: "API_KEY", locator: "fake://a/b/c" }] }, ["reason"]], + ["secret_list", {}, ["query", "provider", "vault", "item", "status", "limit"]], + ["secret_save", { name: "API_KEY", locator: "fake://a/b/c" }, ["category"]], ["secret_forget", { names: ["API_KEY"] }, []], ] as const)("%s treats null for an optional parameter like omitting it", (name, args, optional) => { const tool = registeredTools().find((candidate) => candidate.name === name); diff --git a/packages/pi-secret/test/one-password-provider.test.ts b/packages/pi-secret/test/one-password-provider.test.ts new file mode 100644 index 00000000..3797d1fa --- /dev/null +++ b/packages/pi-secret/test/one-password-provider.test.ts @@ -0,0 +1,272 @@ +import type { JsonValue } from "@earendil-works/pi-ai"; +import { describe, expect, it } from "vitest"; +import { + OnePasswordProvider, + type OnePasswordSettings, + type OpRunner, +} from "../src/one-password-provider.js"; + +interface Call { + args: readonly string[]; + input: string | undefined; +} + +const DEV = { id: "vault-dev", name: "Dev" }; +const PERSONAL = { id: "vault-personal", name: "Personal" }; + +const githubItem = { + id: "item-github", + title: "GitHub", + category: "API_CREDENTIAL", + vault: DEV, + fields: [ + { + id: "credential", + label: "credential", + type: "CONCEALED", + value: "ghp-value-never-listed", + reference: "op://Dev/GitHub/credential", + }, + { + id: "username", + label: "username", + type: "STRING", + value: "octocat", + reference: "op://Dev/GitHub/username", + }, + ], +}; + +/** A fake `op` that answers by subcommand with JSON text, and records every call. */ +function fakeOp(answers: Readonly>) { + const calls: Call[] = []; + const run: OpRunner = async (args, options) => { + calls.push({ args, input: options.input }); + const rest = args[0] === "--account" ? args.slice(2) : args; + const words = rest.filter((arg) => !arg.startsWith("-")); + const answer = answers[words.slice(0, 2).join(" ")] ?? answers[words[0] ?? ""]; + if (answer === undefined) throw new Error(`unexpected op ${args.join(" ")}`); + return answer; + }; + return { calls, run }; +} + +/** A fake `op` answering subcommands with JSON, or with raw text such as `op read` prints. */ +function provider( + answers: Readonly>, + settings: OnePasswordSettings = {}, + raw: Readonly> = {}, +) { + const serialized = Object.entries(answers).map(([command, answer]) => [ + command, + JSON.stringify(answer), + ]); + const op = fakeOp({ ...Object.fromEntries(serialized), ...raw }); + return { op, provider: new OnePasswordProvider(() => settings, op.run) }; +} + +describe("1Password Secret Provider", () => { + it("reads a 1Password Reference with op read, passing the account as a global flag", async () => { + const { op, provider: onePassword } = provider( + {}, + { account: "my.1password.com" }, + { read: "ghp-value-1234" }, + ); + + expect(await onePassword.read("op://Dev/GitHub/credential")).toBe("ghp-value-1234"); + expect(op.calls[0]?.args).toEqual([ + "--account", + "my.1password.com", + "read", + "--no-newline", + "--", + "op://Dev/GitHub/credential", + ]); + }); + + it("rejects a locator that is not a 1Password Reference before running op", async () => { + const { op, provider: onePassword } = provider({}); + await expect(onePassword.read("op://Dev/GitHub")).rejects.toThrow( + "is not a 1Password Reference", + ); + await expect(onePassword.describe("vault://Dev/GitHub/x")).rejects.toThrow( + "is not a 1Password Reference", + ); + expect(op.calls).toEqual([]); + }); + + it("describes a field, including a one-time password attribute", async () => { + const { provider: onePassword } = provider({ "item get": githubItem }); + + expect(await onePassword.describe("op://Dev/GitHub/credential")).toEqual({ + locator: "op://Dev/GitHub/credential", + vault: "Dev", + item: "GitHub", + field: "credential", + fieldType: "concealed", + }); + expect((await onePassword.describe("op://Dev/GitHub/username?attribute=otp")).fieldType).toBe( + "otp", + ); + await expect(onePassword.describe("op://Dev/GitHub/password")).rejects.toThrow( + 'item "GitHub" has no field "password"', + ); + }); + + it("refuses items outside the configured vaults", async () => { + const { provider: onePassword } = provider( + { "item get": { ...githubItem, vault: PERSONAL } }, + { vaults: ["Dev"] }, + ); + await expect(onePassword.describe("op://Personal/GitHub/credential")).rejects.toThrow( + "that item is not in a vault listed in secret.providers.1password.vaults", + ); + }); + + it("searches items in the configured vaults by title, and lists one item's fields without values", async () => { + const items = [ + { id: "item-github", title: "GitHub", category: "API_CREDENTIAL", vault: DEV }, + { id: "item-gitlab", title: "GitLab", category: "LOGIN", vault: DEV }, + { id: "item-bank", title: "Bank", category: "LOGIN", vault: PERSONAL }, + ]; + const { provider: onePassword } = provider( + { "item list": items, "item get": githubItem }, + { vaults: ["vault-dev"] }, + ); + + expect(await onePassword.search({ query: "git", limit: 1 })).toEqual({ + entries: [ + { + kind: "item", + provider: "1password", + id: "item-github", + title: "GitHub", + vault: "Dev", + category: "api credential", + }, + ], + truncated: true, + }); + const fields = await onePassword.search({ item: "GitHub", limit: 20 }); + expect(fields.entries.map((entry) => entry.kind === "field" && entry.locator)).toEqual([ + "op://Dev/GitHub/credential", + "op://Dev/GitHub/username", + ]); + expect(JSON.stringify(fields)).not.toContain("ghp-value-never-listed"); + expect(JSON.stringify(fields)).not.toContain("octocat"); + }); + + it("creates a new item from a template on stdin, never putting the value on the command line", async () => { + const created = { id: "item-new", title: "OpenAI", vault: DEV, fields: [] }; + const { op, provider: onePassword } = provider({ + "vault get": DEV, + "item list": [], + "item create": created, + }); + + const locator = await onePassword.save({ + locator: "op://Dev/OpenAI/credential", + value: "sk-openai-5c6d7e8f", + }); + + expect(locator).toBe("op://vault-dev/item-new/credential"); + const create = op.calls.find((call) => call.args.includes("create")); + expect(create?.args.join(" ")).not.toContain("sk-openai-5c6d7e8f"); + expect(JSON.parse(create?.input ?? "{}")).toEqual({ + title: "OpenAI", + category: "API_CREDENTIAL", + fields: [ + { id: "credential", label: "credential", type: "CONCEALED", value: "sk-openai-5c6d7e8f" }, + ], + }); + }); + + it("refuses to save over an existing item title or outside the configured vaults", async () => { + const existing = provider({ + "vault get": DEV, + "item list": [{ id: "item-github", title: "GitHub", vault: DEV }], + }); + await expect( + existing.provider.describeSave({ locator: "op://Dev/github/credential" }), + ).rejects.toThrow("already has an item titled"); + + const outside = provider({ "vault get": PERSONAL }, { vaults: ["Dev"] }); + await expect( + outside.provider.describeSave({ locator: "op://Personal/New/credential" }), + ).rejects.toThrow("not in a vault listed in secret.providers.1password.vaults"); + expect(outside.op.calls.some((call) => call.args.includes("create"))).toBe(false); + }); + + it("checks the vault at every read when vaults are configured, looking each vault up once", async () => { + const { op, provider: onePassword } = provider( + { "vault get": PERSONAL }, + { vaults: ["Dev"] }, + { read: "never-read-value" }, + ); + await expect(onePassword.read("op://Personal/Bank/password")).rejects.toThrow( + "not in a vault listed in secret.providers.1password.vaults", + ); + await expect(onePassword.read("op://Personal/Bank/pin")).rejects.toThrow("not in a vault"); + expect(op.calls.map((call) => call.args[0])).toEqual(["vault"]); + }); + + it("lists a one-time password field with the locator that reads its current code", async () => { + const otpItem = { + ...githubItem, + fields: [ + { + id: "totp", + label: "one-time password", + type: "OTP", + reference: "op://Dev/GitHub/one-time password", + }, + ], + }; + const { provider: onePassword } = provider({ "item get": otpItem }); + const { entries } = await onePassword.search({ item: "GitHub", limit: 20 }); + expect(entries).toMatchObject([ + { kind: "field", type: "otp", locator: "op://Dev/GitHub/one-time password?attribute=otp" }, + ]); + }); + + it("looks an item up only in the configured vaults", async () => { + const { op, provider: onePassword } = provider({ "item get": githubItem }, { vaults: ["Dev"] }); + await onePassword.search({ item: "GitHub", limit: 20 }); + expect(op.calls[0]?.args).toEqual([ + "item", + "get", + "--vault", + "Dev", + "--format", + "json", + "--", + "GitHub", + ]); + }); + + it("saves into a built-in field by its purpose", async () => { + const created = { id: "item-new", title: "Router", vault: DEV, fields: [] }; + const { op, provider: onePassword } = provider({ + "vault get": DEV, + "item list": [], + "item create": created, + }); + await onePassword.save({ + locator: "op://Dev/Router/password", + value: "router-pass-1a2b", + category: "Password", + }); + const create = op.calls.find((call) => call.args.includes("create")); + expect(JSON.parse(create?.input ?? "{}")).toMatchObject({ + category: "PASSWORD", + fields: [{ id: "password", purpose: "PASSWORD", type: "CONCEALED" }], + }); + }); + + it("treats an empty vault list, as invalid settings fall back to, as no vaults", async () => { + const { provider: onePassword } = provider({ "item get": githubItem }, { vaults: [] }); + await expect(onePassword.describe("op://Dev/GitHub/credential")).rejects.toThrow( + "not in a vault listed in secret.providers.1password.vaults", + ); + }); +}); diff --git a/packages/pi-secret/test/pi-secret-extension.test.ts b/packages/pi-secret/test/pi-secret-extension.test.ts index 26bf2e4c..453fc044 100644 --- a/packages/pi-secret/test/pi-secret-extension.test.ts +++ b/packages/pi-secret/test/pi-secret-extension.test.ts @@ -133,10 +133,50 @@ describe("Secret Request and Secret References", () => { expect(harness.received).toEqual([]); expect(harness.resultText("call-echo")).toBe( - 'The call did not run. "bad name" is not a valid Secret Name: a Secret Name uses uppercase letters, digits, and underscores, and does not start with a digit. It references {{secret:NEVER_PROVIDED}}, which this session does not hold. The Secret expired, was forgotten, or was never provided. Call secret_ask to request it again.', + 'The call did not run. "bad name" is not a valid Secret Name: a Secret Name uses uppercase letters, digits, and underscores, and does not start with a digit. It references {{secret:NEVER_PROVIDED}}, which this session does not hold. The Secret expired, was forgotten, or was never provided. Call secret_ask to request it again. To write a reference as literal text instead, put a backslash right before it; Pi Secret removes the backslash.', ); }); + it("runs a call whose references are escaped with a backslash, passing them on as literal text", async () => { + const harness = await createSecretHarness(); + const text = "Use \\{{secret:NAME}} in arguments and $\\PI_SECRET_NAME in shell commands."; + harness.responses.push(toolCalls(["echo", { text }, "call-echo"]), reply("Done.")); + await harness.session.prompt("Document the syntax."); + + expect(harness.received).toEqual([ + { + tool: "echo", + args: { text: "Use {{secret:NAME}} in arguments and $PI_SECRET_NAME in shell commands." }, + }, + ]); + // The transcript keeps the call as the model wrote it. + expect(JSON.stringify(harness.session.messages)).toContain("Use \\\\{{secret:NAME}}"); + }); + + it("leaves a shell-escaped expansion exactly as written, since the shell prints it literally", async () => { + const harness = await createSecretHarness(); + const text = 'echo "\\$PI_SECRET_NEVER_PROVIDED and \\${PI_SECRET_NEVER_PROVIDED}"'; + harness.responses.push(toolCalls(["echo", { text }, "call-echo"]), reply("Done.")); + await harness.session.prompt("Explain the variable."); + + expect(harness.received).toEqual([{ tool: "echo", args: { text } }]); + }); + + it("resolves a held Secret next to an escaped reference in the same argument", async () => { + const value = "mixed-value-3e4f5a6b"; + const harness = await createSecretHarness({ dialogs: [[keys.paste(value), keys.enter]] }); + harness.responses.push( + toolCalls(["secret_ask", { names: ["MIXED"] }, "call-ask"]), + toolCalls(["echo", { text: "\\{{secret:MIXED}}={{secret:MIXED}}" }, "call-echo"]), + reply("Done."), + ); + await harness.session.prompt("Go."); + + expect(harness.received).toEqual([ + { tool: "echo", args: { text: `{{secret:MIXED}}=${value}` } }, + ]); + }); + it("runs a call that only mentions the environment prefix, such as listing held Secrets' variables", async () => { const harness = await createSecretHarness(); harness.responses.push( diff --git a/packages/pi-secret/test/secret-grants.test.ts b/packages/pi-secret/test/secret-grants.test.ts new file mode 100644 index 00000000..96530181 --- /dev/null +++ b/packages/pi-secret/test/secret-grants.test.ts @@ -0,0 +1,440 @@ +import { describe, expect, it } from "vitest"; +import { FakeProvider } from "./fixtures/fake-provider.js"; +import { createSecretHarness, keys, reply, toolCalls } from "./fixtures/secret-harness.js"; + +const TOKEN = "fake://Dev/GitHub/token"; +const OTP = "fake://Dev/Bank/otp"; + +/** A one-time password that changes at every read. */ +function otp(): () => string { + let count = 0; + return () => `otp-code-${String(++count).padStart(6, "0")}`; +} + +describe("Secret Grants", () => { + it("reads a granted Secret again at each use, so a one-time password is always current, and stores only its locator", async () => { + const provider = new FakeProvider({ [OTP]: otp() }); + const harness = await createSecretHarness({ providers: [provider], dialogs: [[keys.enter]] }); + harness.responses.push( + toolCalls([ + "secret_ask_grant", + { grants: [{ name: "BANK_OTP", locator: OTP }], reason: "Log in" }, + "call-grant", + ]), + toolCalls(["echo", { text: "code={{secret:BANK_OTP}}" }, "call-echo-1"]), + toolCalls(["echo", { text: "code={{secret:BANK_OTP}}" }, "call-echo-2"]), + reply("Done."), + ); + await harness.session.prompt("Log in to the bank."); + + expect(harness.result("call-grant")).toMatchObject({ + details: { + secrets: [ + { name: "BANK_OTP", status: "granted", locator: OTP, reference: "{{secret:BANK_OTP}}" }, + ], + }, + }); + // One read checks the grant on approval, then one per use. + expect(harness.received).toEqual([ + { tool: "echo", args: { text: "code=otp-code-000002" } }, + { tool: "echo", args: { text: "code=otp-code-000003" } }, + ]); + expect(harness.resultText("call-echo-2")).toBe("echo: code={{secret:BANK_OTP}}"); + // The dialog showed where the value is read from, and the session holds the locator only. + const dialog = harness.frames[0]?.join("\n") ?? ""; + expect(dialog).toContain("Grant request"); + expect(dialog).toContain("Fake Vault · Dev / Bank / otp · concealed"); + const sessionText = await harness.sessionFileText(); + expect(sessionText).toContain(`"customType":"pi-secret-grant"`); + expect(sessionText).toContain(OTP); + for (const code of ["otp-code-000001", "otp-code-000002", "otp-code-000003"]) { + expect(sessionText).not.toContain(code); + expect(harness.requests.join("\n")).not.toContain(code); + } + }); + + it("reports denied, cancelled, and failed grants and holds none of them", async () => { + const provider = new FakeProvider({ [TOKEN]: "ghp-token-0a1b2c3d", [OTP]: otp() }); + const harness = await createSecretHarness({ + providers: [provider], + // Deny the first row, allow the second; then cancel a second request outright. + dialogs: [[" ", keys.enter], [keys.escape]], + }); + harness.responses.push( + toolCalls([ + "secret_ask_grant", + { + grants: [ + { name: "DENIED_ONE", locator: TOKEN }, + { name: "MISSING_ONE", locator: "fake://Dev/Nowhere/field" }, + { name: "UNKNOWN_SCHEME", locator: "vault://x/y/z" }, + { name: "GRANTED_ONE", locator: OTP }, + ], + }, + "call-grant-1", + ]), + toolCalls([ + "secret_ask_grant", + { grants: [{ name: "CANCELLED_ONE", locator: TOKEN }] }, + "call-grant-2", + ]), + reply("Done."), + ); + await harness.session.prompt("Grant."); + + expect(harness.result("call-grant-1")).toMatchObject({ + details: { + secrets: [ + { name: "DENIED_ONE", status: "denied" }, + { name: "MISSING_ONE", status: "failed", error: "no field at fake://Dev/Nowhere/field" }, + { name: "UNKNOWN_SCHEME", status: "failed" }, + { name: "GRANTED_ONE", status: "granted" }, + ], + }, + }); + expect(harness.resultText("call-grant-1")).toContain("available schemes: fake://"); + expect(harness.result("call-grant-2")).toMatchObject({ + details: { secrets: [{ name: "CANCELLED_ONE", status: "cancelled" }] }, + }); + // Only rows a provider could describe reach the dialog. + expect(harness.frames[0]?.join("\n")).not.toContain("MISSING_ONE"); + }); + + it("fails a grant whose check read fails, without holding it", async () => { + const provider = new FakeProvider({ [TOKEN]: "ghp-token-0a1b2c3d" }); + provider.locked = true; + const harness = await createSecretHarness({ providers: [provider], dialogs: [[keys.enter]] }); + harness.responses.push( + toolCalls([ + "secret_ask_grant", + { grants: [{ name: "LOCKED", locator: TOKEN }] }, + "call-grant", + ]), + toolCalls(["secret_list", { status: "held" }, "call-list"]), + reply("Done."), + ); + await harness.session.prompt("Grant."); + + expect(harness.resultText("call-grant")).toBe("LOCKED: not granted: Fake Vault is locked"); + expect(harness.resultText("call-list")).not.toContain("LOCKED"); + }); + + it("blocks a call when the Secret Provider cannot be read at use, and keeps the grant", async () => { + const provider = new FakeProvider({ [TOKEN]: "ghp-token-0a1b2c3d" }); + const harness = await createSecretHarness({ providers: [provider], dialogs: [[keys.enter]] }); + harness.responses.push( + toolCalls([ + "secret_ask_grant", + { grants: [{ name: "GH_TOKEN", locator: TOKEN }] }, + "call-grant", + ]), + ); + await harness.session.prompt("Grant."); + provider.locked = true; + harness.responses.push( + toolCalls(["echo", { text: "curl -H $PI_SECRET_GH_TOKEN" }, "call-echo"]), + toolCalls(["secret_list", { status: "held" }, "call-list"]), + reply("Done."), + ); + await harness.session.prompt("Use it."); + + expect(harness.received).toEqual([]); + expect(harness.resultText("call-echo")).toContain( + "could not read {{secret:GH_TOKEN}} from its Secret Provider: Fake Vault is locked", + ); + expect(harness.resultText("call-list")).toContain(`GH_TOKEN: held from ${TOKEN}`); + }); + + it("replaces a typed Secret with a grant under the same name, and the other way round", async () => { + const provider = new FakeProvider({ [TOKEN]: "ghp-granted-5e6f7a8b" }); + const harness = await createSecretHarness({ + providers: [provider], + dialogs: [[keys.paste("typed-value-1a2b3c4d"), keys.enter], [keys.enter]], + }); + harness.responses.push( + toolCalls(["secret_ask", { names: ["SWAPPED"] }, "call-ask"]), + toolCalls([ + "secret_ask_grant", + { grants: [{ name: "SWAPPED", locator: TOKEN }] }, + "call-grant", + ]), + toolCalls(["echo", { text: "{{secret:SWAPPED}}" }, "call-echo"]), + reply("Done."), + ); + await harness.session.prompt("Swap."); + + expect(harness.received).toEqual([{ tool: "echo", args: { text: "ghp-granted-5e6f7a8b" } }]); + }); + + it("lets a Child Agent use its root's grant, read fresh through the registry", async () => { + const provider = new FakeProvider({ [OTP]: otp() }); + const root = await createSecretHarness({ providers: [provider], dialogs: [[keys.enter]] }); + root.responses.push( + toolCalls([ + "secret_ask_grant", + { grants: [{ name: "ROOT_OTP", locator: OTP }] }, + "call-grant", + ]), + ); + await root.session.prompt("Grant."); + + const child = await createSecretHarness({ headless: true }); + child.responses.push( + toolCalls(["echo", { text: "{{secret:ROOT_OTP}}" }, "call-echo"]), + reply("Done."), + ); + await child.session.prompt("Use it."); + + expect(child.received).toEqual([{ tool: "echo", args: { text: "otp-code-000002" } }]); + expect(child.resultText("call-echo")).toBe("echo: {{secret:ROOT_OTP}}"); + }); +}); + +describe("prompt cache", () => { + it("keeps the system prompt and ordered tool definitions identical across turns that grant, use, and forget", async () => { + const provider = new FakeProvider({ [TOKEN]: "ghp-token-0a1b2c3d" }); + const harness = await createSecretHarness({ providers: [provider], dialogs: [[keys.enter]] }); + harness.responses.push( + toolCalls([ + "secret_ask_grant", + { grants: [{ name: "GH_TOKEN", locator: TOKEN }] }, + "call-grant", + ]), + toolCalls(["echo", { text: "{{secret:GH_TOKEN}}" }, "call-echo"]), + toolCalls(["secret_forget", { names: ["GH_TOKEN"] }, "call-forget"]), + reply("Done."), + ); + await harness.session.prompt("Grant, use, forget."); + + // Pi carries the system prompt and the ordered tool declarations in the first system message. + const prefixes = harness.requests.map((request) => + JSON.stringify(JSON.parse(request).messages[0]), + ); + expect(prefixes).toHaveLength(4); + expect(new Set(prefixes).size).toBe(1); + const [prefix = ""] = prefixes; + expect(JSON.parse(prefix).role).toBe("system"); + const positions = [ + "secret_ask", + "secret_ask_grant", + "secret_list", + "secret_save", + "secret_forget", + ].map((name) => prefix.indexOf(`"${name}"`)); + expect(positions.every((position) => position >= 0)).toBe(true); + expect(positions).toEqual(positions.toSorted((a, b) => a - b)); + }); +}); + +describe("secret_list", () => { + it("lists held Secrets with their source and searches Secret Providers without values", async () => { + const provider = new FakeProvider({ + [TOKEN]: "ghp-token-0a1b2c3d", + "fake://Dev/Stripe/key": "sk-stripe-9f8e7d6c", + }); + const harness = await createSecretHarness({ providers: [provider], dialogs: [[keys.enter]] }); + harness.responses.push( + toolCalls([ + "secret_ask_grant", + { grants: [{ name: "GH_TOKEN", locator: TOKEN }] }, + "call-grant", + ]), + toolCalls(["secret_list", {}, "call-list"]), + toolCalls(["secret_list", { query: "Stripe", item: "Stripe" }, "call-fields"]), + toolCalls(["secret_list", { provider: "other" }, "call-other"]), + reply("Done."), + ); + await harness.session.prompt("List."); + + const listed = harness.resultText("call-list") ?? ""; + expect(listed).toContain(`GH_TOKEN: held from ${TOKEN}`); + expect(listed).toContain("Available to request with secret_ask_grant:"); + expect(listed).toContain("- Stripe (password) in Dev"); + expect(harness.result("call-fields")).toMatchObject({ + details: { + secrets: [], + available: [{ kind: "field", label: "key", locator: "fake://Dev/Stripe/key" }], + }, + }); + expect(harness.resultText("call-other")).toMatch(/^No Secrets matched/); + for (const value of ["ghp-token-0a1b2c3d", "sk-stripe-9f8e7d6c"]) + expect(harness.requests.join("\n")).not.toContain(value); + }); + + it("still lists held Secrets when a Secret Provider cannot be searched", async () => { + const provider = new FakeProvider({ [TOKEN]: "ghp-token-0a1b2c3d" }); + provider.locked = true; + const harness = await createSecretHarness({ + providers: [provider], + dialogs: [[keys.paste("typed-value-1a2b3c4d"), keys.enter]], + }); + harness.responses.push( + toolCalls(["secret_ask", { names: ["TYPED"] }, "call-ask"]), + toolCalls(["secret_list", {}, "call-list"]), + reply("Done."), + ); + await harness.session.prompt("List."); + + const listed = harness.resultText("call-list") ?? ""; + expect(listed).toContain("TYPED: held until"); + expect(listed).toContain("Could not search Fake Vault: Fake Vault is locked"); + }); +}); + +describe("secret_save", () => { + it("saves a typed Secret into a new item after approval, then reads it from there with its original expiry", async () => { + const value = "typed-to-save-4c5d6e7f"; + const provider = new FakeProvider(); + const harness = await createSecretHarness({ + providers: [provider], + dialogs: [[keys.paste(value), keys.enter], [keys.enter]], + }); + const expiresAt = new Date(harness.clock.now + 15 * 60_000).toISOString(); + harness.responses.push( + toolCalls(["secret_ask", { names: ["NEW_KEY"] }, "call-ask"]), + toolCalls([ + "secret_save", + { name: "NEW_KEY", locator: "fake://Dev/New API/credential" }, + "call-save", + ]), + toolCalls(["echo", { text: "{{secret:NEW_KEY}}" }, "call-echo"]), + reply("Done."), + ); + await harness.session.prompt("Save it."); + + expect(provider.saved).toEqual([{ locator: "fake://Dev/New API/credential", value }]); + expect(harness.result("call-save")).toMatchObject({ + details: { + secrets: [ + { + name: "NEW_KEY", + status: "saved", + locator: "fake://Dev/item-1/credential", + expires_at: expiresAt, + }, + ], + }, + }); + expect(harness.frames.at(-1)?.join("\n")).toContain("Save Secret"); + expect(harness.received).toEqual([{ tool: "echo", args: { text: value } }]); + expect(provider.reads).toEqual(["fake://Dev/item-1/credential"]); + expect(harness.requests.join("\n")).not.toContain(value); + expect(await harness.sessionFileText()).not.toContain(value); + }); + + it("saves nothing when the user denies, and refuses Secrets that were not typed here", async () => { + const provider = new FakeProvider({ [TOKEN]: "ghp-token-0a1b2c3d" }); + const harness = await createSecretHarness({ + providers: [provider], + dialogs: [[keys.paste("typed-value-1a2b3c4d"), keys.enter], [" ", keys.enter], [keys.enter]], + }); + harness.responses.push( + toolCalls(["secret_ask", { names: ["TYPED"] }, "call-ask"]), + toolCalls([ + "secret_save", + { name: "TYPED", locator: "fake://Dev/New/credential" }, + "call-deny", + ]), + toolCalls([ + "secret_ask_grant", + { grants: [{ name: "GRANTED", locator: TOKEN }] }, + "call-grant", + ]), + toolCalls([ + "secret_save", + { name: "GRANTED", locator: "fake://Dev/Copy/credential" }, + "call-granted", + ]), + reply("Done."), + ); + await harness.session.prompt("Save."); + + expect(harness.result("call-deny")).toMatchObject({ + details: { secrets: [{ name: "TYPED", status: "denied" }] }, + }); + expect(harness.result("call-granted")).toMatchObject({ isError: true }); + expect(harness.resultText("call-granted")).toContain("only those can be saved"); + expect(provider.saved).toEqual([]); + }); +}); + +describe("Secret Grants across sessions", () => { + async function grantAndQuit(provider: FakeProvider) { + const first = await createSecretHarness({ providers: [provider], dialogs: [[keys.enter]] }); + first.responses.push( + toolCalls([ + "secret_ask_grant", + { grants: [{ name: "GH_TOKEN", locator: TOKEN }] }, + "call-grant", + ]), + reply("Ok."), + ); + await first.session.prompt("Grant."); + const sessionFile = first.session.sessionManager.getSessionFile(); + if (sessionFile === undefined) throw new Error("The session has no file"); + return { first, sessionFile }; + } + + it("restores a grant on resume without reading it until it is used", async () => { + const provider = new FakeProvider({ [TOKEN]: "ghp-token-0a1b2c3d" }); + const { first, sessionFile } = await grantAndQuit(provider); + await first.quit(); + provider.reads.length = 0; + + const resumed = await createSecretHarness({ + dir: first.dir, + sessionFile, + clock: first.clock, + providers: [provider], + }); + expect(provider.reads).toEqual([]); + resumed.responses.push( + toolCalls(["echo", { text: "{{secret:GH_TOKEN}}" }, "call-echo"]), + reply("Done."), + ); + await resumed.session.prompt("Use it."); + + expect(resumed.received).toEqual([{ tool: "echo", args: { text: "ghp-token-0a1b2c3d" } }]); + }); + + it("does not restore a grant after its lifetime or once forgotten", async () => { + const provider = new FakeProvider({ [TOKEN]: "ghp-token-0a1b2c3d" }); + const { first, sessionFile } = await grantAndQuit(provider); + first.responses.push( + toolCalls(["secret_forget", { names: ["GH_TOKEN"] }, "call-forget"]), + reply("Ok."), + ); + await first.session.prompt("Forget it."); + await first.quit(); + + const resumed = await createSecretHarness({ + dir: first.dir, + sessionFile, + clock: first.clock, + providers: [provider], + }); + resumed.responses.push( + toolCalls(["echo", { text: "{{secret:GH_TOKEN}}" }, "call-echo"]), + reply("Done."), + ); + await resumed.session.prompt("Use it."); + expect(resumed.received).toEqual([]); + await resumed.quit(); + + const { first: other, sessionFile: otherFile } = await grantAndQuit(provider); + await other.quit(); + other.clock.now += 16 * 60_000; + const late = await createSecretHarness({ + dir: other.dir, + sessionFile: otherFile, + clock: other.clock, + providers: [provider], + }); + late.responses.push( + toolCalls(["echo", { text: "{{secret:GH_TOKEN}}" }, "call-echo"]), + reply("Done."), + ); + await late.session.prompt("Use it."); + expect(late.received).toEqual([]); + }); +}); diff --git a/packages/pi-utils/README.md b/packages/pi-utils/README.md index 381445c0..e81a9da1 100644 --- a/packages/pi-utils/README.md +++ b/packages/pi-utils/README.md @@ -40,7 +40,7 @@ Type-only imports from `@earendil-works/pi-ai` and `@earendil-works/pi-coding-ag ## Secret registry -`@ian-pascoe/pi-utils/secret-registry` is the process-wide registry of held Secrets that Pi Secret fills and other packages read without depending on it ([ADR-0008](../../docs/adr/0008-secret-redaction-registry-in-pi-utils.md)). Every copy of pi-utils in one process shares it. `holdSecret(name, value, owner, expiresAt?)` makes a value resolve under a Secret Name (`SECRET_NAME_PATTERN`) until the same `owner` calls `revokeSecret(name, owner)` or the wall clock passes `expiresAt`, so a late revocation timer cannot extend a Secret's life. `resolveSecret(name)` and `heldSecretNames()` read what currently resolves. `redactSecrets(value)` returns a copy of a JSON-like value with every value ever held in the process replaced by its Secret Reference (`secretReference(name)`, `{{secret:NAME}}`), longest value first in one pass, whether written as is or in a common encoding (JSON string escaping, percent-encoding, hex, and base64 and base64url at every alignment; only for values of at least `MIN_ENCODED_REDACTION_LENGTH`, 8, characters), or the value itself when nothing matched. Revoked and replaced values stay redacted for the rest of the process, so output printed after a Secret expires is still hidden. +`@ian-pascoe/pi-utils/secret-registry` is the process-wide registry of held Secrets that Pi Secret fills and other packages read without depending on it ([ADR-0008](../../docs/adr/0008-secret-redaction-registry-in-pi-utils.md)). Every copy of pi-utils in one process shares it. `holdSecret(name, value, owner, expiresAt?)` makes a value resolve under a Secret Name (`SECRET_NAME_PATTERN`) until the same `owner` calls `revokeSecret(name, owner)` or the wall clock passes `expiresAt`, so a late revocation timer cannot extend a Secret's life. `holdRefreshableSecret(name, owner, { refresh, value?, expiresAt? })` holds a Secret whose value lives elsewhere, such as in a Secret Provider: `refreshSecret(name)` awaits its `refresh` callback, keeps and returns the value read, and registers it for redaction, so Child Agents read the current value too ([ADR-0010](../../docs/adr/0010-secret-registry-refresh-callbacks.md)); for a Secret held without one it returns the held value. `resolveSecret(name)` and `heldSecretNames()` read what currently resolves, and `isSecretHeld(name)` is true for a held Secret even before its first read. `redactSecrets(value)` returns a copy of a JSON-like value with every value ever held in the process replaced by its Secret Reference (`secretReference(name)`, `{{secret:NAME}}`), longest value first in one pass, whether written as is or in a common encoding (JSON string escaping, percent-encoding, hex, and base64 and base64url at every alignment; only for values of at least `MIN_ENCODED_REDACTION_LENGTH`, 8, characters), or the value itself when nothing matched. Revoked and replaced values stay redacted for the rest of the process, so output printed after a Secret expires is still hidden. ## Queued Steers diff --git a/packages/pi-utils/src/secret-registry.ts b/packages/pi-utils/src/secret-registry.ts index 683891e6..a3e94215 100644 --- a/packages/pi-utils/src/secret-registry.ts +++ b/packages/pi-utils/src/secret-registry.ts @@ -14,10 +14,13 @@ export interface SecretOwner { } interface HeldSecret { - readonly value: string; + /** The value last held or read; undefined until a refreshable Secret is first read. */ + value: string | undefined; readonly owner: SecretOwner; /** Milliseconds since the epoch after which the Secret no longer resolves, if it expires. */ readonly expiresAt: number | undefined; + /** Reads the current value from where it lives, such as a Secret Provider (ADR-0010). */ + readonly refresh: (() => Promise) | undefined; } /** Secrets that resolve, by Secret Name. */ @@ -67,10 +70,53 @@ export function holdSecret( ): void { assertSecretName(name); if (value === "") throw new Error(`Secret ${name} cannot be empty`); - held().set(name, { value, owner, expiresAt }); + held().set(name, { value, owner, expiresAt, refresh: undefined }); redacted().set(value, name); } +export interface RefreshableSecretOptions { + /** Reads the current value; every value it returns is redacted for the rest of the process. */ + refresh: () => Promise; + /** A value already read, if any; otherwise nothing resolves until the first refresh. */ + value?: string; + /** Milliseconds since the epoch after which the Secret no longer resolves. */ + expiresAt?: number; +} + +/** + * Hold a Secret whose value lives elsewhere, such as in a Secret Provider, for `owner`, replacing + * any value already held under `name`. {@link refreshSecret} reads it again through `refresh`, so + * every session in the process, including Child Agents, uses the current value (ADR-0010). + */ +export function holdRefreshableSecret( + name: string, + owner: SecretOwner, + options: RefreshableSecretOptions, +): void { + assertSecretName(name); + const value = options.value === "" ? undefined : options.value; + held().set(name, { value, owner, expiresAt: options.expiresAt, refresh: options.refresh }); + if (value !== undefined) redacted().set(value, name); +} + +/** + * Read the current value of the Secret held under `name` through its refresh callback, keep it, + * and return it; a Secret without one returns its held value. Undefined when no Secret by that + * name is held. Rejects, keeping the last value read, when the refresh fails or reads nothing. + */ +export async function refreshSecret(name: string): Promise { + const secret = held().get(name); + if (secret === undefined || !current(secret)) return undefined; + if (secret.refresh === undefined) return secret.value; + const value = await secret.refresh(); + if (value === "") throw new Error(`Secret ${name} read as empty`); + redacted().set(value, name); + // The Secret may have been revoked or replaced while it was read. + if (held().get(name) !== secret) return undefined; + secret.value = value; + return value; +} + /** Stop resolving `name` if `owner` holds it; its values are still redacted. */ export function revokeSecret(name: string, owner: SecretOwner): void { const secrets = held(); @@ -88,6 +134,12 @@ export function resolveSecret(name: string): string | undefined { return secret !== undefined && current(secret) ? secret.value : undefined; } +/** Whether a Secret by that name is held, even one not read yet. */ +export function isSecretHeld(name: string): boolean { + const secret = held().get(name); + return secret !== undefined && current(secret); +} + /** Secret Names that currently resolve. */ export function heldSecretNames(): string[] { return [...held()].flatMap(([name, secret]) => (current(secret) ? [name] : [])); diff --git a/packages/pi-utils/test/secret-registry.test.ts b/packages/pi-utils/test/secret-registry.test.ts index 0d1429e8..72651ce8 100644 --- a/packages/pi-utils/test/secret-registry.test.ts +++ b/packages/pi-utils/test/secret-registry.test.ts @@ -1,8 +1,10 @@ import { expect, test } from "vitest"; import { heldSecretNames, + holdRefreshableSecret, holdSecret, redactSecrets, + refreshSecret, resolveSecret, revokeSecret, secretReference, @@ -136,3 +138,43 @@ test("Secret Names must be environment-variable style and values non-empty", () expect(() => holdSecret("EMPTY_VALUE", "", owner)).toThrow(/empty/); expect(secretReference("OPENCODE_API_KEY")).toBe("{{secret:OPENCODE_API_KEY}}"); }); + +test("a refreshable Secret reads its value on each refresh, and every value read stays redacted", async () => { + const codes = ["otp-code-111111", "otp-code-222222"]; + const refresh = async () => codes.shift() ?? "exhausted"; + holdRefreshableSecret("REGISTRY_OTP", owner, { refresh }); + expect(heldSecretNames()).toContain("REGISTRY_OTP"); + // Nothing has been read yet, so nothing resolves until it is refreshed. + expect(resolveSecret("REGISTRY_OTP")).toBeUndefined(); + + expect(await refreshSecret("REGISTRY_OTP")).toBe("otp-code-111111"); + expect(resolveSecret("REGISTRY_OTP")).toBe("otp-code-111111"); + expect(await refreshSecret("REGISTRY_OTP")).toBe("otp-code-222222"); + expect(redactSecrets("otp-code-111111 otp-code-222222")).toBe( + "{{secret:REGISTRY_OTP}} {{secret:REGISTRY_OTP}}", + ); + revokeSecret("REGISTRY_OTP", owner); + expect(await refreshSecret("REGISTRY_OTP")).toBeUndefined(); +}); + +test("refreshing a Secret without a refresh callback returns its held value", async () => { + holdSecret("REGISTRY_TYPED", "typed-value-9e8d7c", owner); + expect(await refreshSecret("REGISTRY_TYPED")).toBe("typed-value-9e8d7c"); + revokeSecret("REGISTRY_TYPED", owner); +}); + +test("a failed refresh rejects and keeps the last value read", async () => { + let fail = false; + holdRefreshableSecret("REGISTRY_FLAKY", owner, { + value: "flaky-value-5a5a5a", + refresh: async () => { + if (fail) throw new Error("1Password is locked"); + return "flaky-value-6b6b6b"; + }, + }); + expect(resolveSecret("REGISTRY_FLAKY")).toBe("flaky-value-5a5a5a"); + fail = true; + await expect(refreshSecret("REGISTRY_FLAKY")).rejects.toThrow("1Password is locked"); + expect(resolveSecret("REGISTRY_FLAKY")).toBe("flaky-value-5a5a5a"); + revokeSecret("REGISTRY_FLAKY", owner); +}); From 7bfcdd8d2ed62dba3246d8d9840db84f9067945b Mon Sep 17 00:00:00 2001 From: Ian Pascoe Date: Tue, 20 Oct 2026 07:26:25 -0400 Subject: [PATCH 2/3] chore: run the test suites in CI only, not in the pre-push hook The pre-push hook now runs pnpm verify:push (format, lint, typechecks). CI still runs the full pnpm verify. --- .husky/pre-push | 2 +- AGENTS.md | 2 +- README.md | 2 +- package.json | 1 + 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.husky/pre-push b/.husky/pre-push index 7612cc82..4b5bb26d 100755 --- a/.husky/pre-push +++ b/.husky/pre-push @@ -1,2 +1,2 @@ unset $(git rev-parse --local-env-vars) -pnpm verify +pnpm verify:push diff --git a/AGENTS.md b/AGENTS.md index 01e4af02..96a13393 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -8,7 +8,7 @@ - **Coexistence** — When changing tool activation, context delivery, or a package's tool-result text, trace other extensions using the same Pi APIs. Test affected standalone and combined modes: combined-mode tests live in the root `test/` and run with `pnpm test:root`, which `pnpm --filter` skips. - **Tool registration** — Register every extension tool through `acceptNullForOptionalArguments` from `@ian-pascoe/pi-utils/null-optional-arguments` so `null` for an optional parameter behaves like omitting it, and cover new tools with `expectNullOptionalArgumentsOmitted` from `@ian-pascoe/pi-utils/tool-testing`. - **Cache proofs** — Prove prefix stability in offline SDK integration tests by comparing the affected ordered tool definitions, system prompt, or message history. Equal name sets or token estimates alone are insufficient. -- **Verification** — While iterating, run focused checks such as `pnpm --filter test`. `pnpm verify` and the pre-push hook share a Turborepo cache keyed on file contents, so rerunning either on unchanged files replays instantly; never bypass the hook with `--no-verify`. +- **Verification** — While iterating, run focused checks such as `pnpm --filter test`. The pre-push hook runs `pnpm verify:push` (format, lint, and typechecks); CI runs the full `pnpm verify`, including every test suite, so leave full test runs to CI. Both share a Turborepo cache keyed on file contents, so reruns on unchanged files replay instantly; never bypass the hook with `--no-verify`. - **PR completion** — Include a Changeset covering every package with releasable changes before opening a PR; test-only package edits need no bump. Follow [docs/releases.md](docs/releases.md) for versioning and release gates. ## Context pointers diff --git a/README.md b/README.md index e4acd141..43269796 100644 --- a/README.md +++ b/README.md @@ -149,7 +149,7 @@ pnpm install pnpm verify ``` -`pnpm verify` runs through Turborepo, which caches each check by its inputs. A rerun on unchanged files, including the pre-push hook after a commit, replays from `.turbo/cache`. +`pnpm verify` runs through Turborepo, which caches each check by its inputs. A rerun on unchanged files replays from `.turbo/cache`. The pre-push hook runs `pnpm verify:push`, the same checks without the test suites; CI runs the full `pnpm verify`. A package task's inputs cover only its own files and its declared workspace dependencies. When a test imports another package by relative path or reads a repository file, add that path to the task's `inputs` in `turbo.json`; otherwise a change there replays a stale pass. Focused package checks: diff --git a/package.json b/package.json index f9f68e17..6dec4417 100644 --- a/package.json +++ b/package.json @@ -26,6 +26,7 @@ "typecheck": "turbo run typecheck:root typecheck", "typecheck:root": "tsc --noEmit", "verify": "turbo run format:check lint typecheck:root typecheck test test:root --output-logs=errors-only", + "verify:push": "turbo run format:check lint typecheck:root typecheck --output-logs=errors-only", "version-packages": "changeset version && pnpm install --lockfile-only" }, "devDependencies": { From d7182977aed3b70b7ddd3a5c45fffbf8de75f207 Mon Sep 17 00:00:00 2001 From: Ian Pascoe Date: Tue, 20 Oct 2026 07:44:31 -0400 Subject: [PATCH 3/3] fix(secret): keep messages already sent unchanged when a later Secret appears in them Redacting the whole history with every value held rewrote earlier messages when a Secret Grant read a value the model had already seen, discarding the provider's prompt cache. Each message is now redacted at the Redaction Generation it was first sent at (pi-secret ADR-0002). --- .changeset/secret-providers.md | 2 +- .changeset/secret-registry-refresh.md | 2 +- packages/pi-secret/GLOSSARY.md | 4 ++ packages/pi-secret/README.md | 3 +- ...sent-messages-at-their-first-generation.md | 14 ++++++ packages/pi-secret/src/pi-secret-extension.ts | 36 +++++++++++++- .../test/pi-secret-extension.test.ts | 27 +++++++++++ packages/pi-utils/src/secret-registry.ts | 48 ++++++++++++------- .../pi-utils/test/secret-registry.test.ts | 17 +++++++ 9 files changed, 131 insertions(+), 22 deletions(-) create mode 100644 packages/pi-secret/docs/adr/0002-redact-sent-messages-at-their-first-generation.md diff --git a/.changeset/secret-providers.md b/.changeset/secret-providers.md index 6c26c127..bcddb5ca 100644 --- a/.changeset/secret-providers.md +++ b/.changeset/secret-providers.md @@ -2,4 +2,4 @@ "@ian-pascoe/pi-secret": minor --- -Add Secret Providers, starting with 1Password through the `op` CLI. `secret_list` now searches Secret Providers for items and fields (`query`, `provider`, `vault`, `item`, `status`, `limit`), never values. `secret_ask_grant` asks the user to approve Secret Grants, each a Secret Name and the 1Password Reference (`op://vault/item/field`) its value is read from again at each use, so one-time passwords stay current. `secret_save` saves a typed Secret into a new 1Password item after the user approves. The session stores only a grant's locator, never its value. Optional `secret.providers.1password.account` and `vaults` settings narrow what the agent can see. A backslash right before a Secret Reference now makes it literal text, so documentation and tests can contain the syntax. +Add Secret Providers, starting with 1Password through the `op` CLI. `secret_list` now searches Secret Providers for items and fields (`query`, `provider`, `vault`, `item`, `status`, `limit`), never values. `secret_ask_grant` asks the user to approve Secret Grants, each a Secret Name and the 1Password Reference (`op://vault/item/field`) its value is read from again at each use, so one-time passwords stay current. `secret_save` saves a typed Secret into a new 1Password item after the user approves. The session stores only a grant's locator, never its value. Optional `secret.providers.1password.account` and `vaults` settings narrow what the agent can see. A backslash right before a Secret Reference now makes it literal text, so documentation and tests can contain the syntax. Provider-request scrubbing now leaves a message already sent unchanged when a value held later appears in it, so holding a Secret no longer discards the provider's prompt cache. diff --git a/.changeset/secret-registry-refresh.md b/.changeset/secret-registry-refresh.md index a49ee152..ff32ee1a 100644 --- a/.changeset/secret-registry-refresh.md +++ b/.changeset/secret-registry-refresh.md @@ -2,4 +2,4 @@ "@ian-pascoe/pi-utils": minor --- -Let the secret registry hold Secrets whose values live elsewhere: `holdRefreshableSecret(name, owner, { refresh, value?, expiresAt? })` registers a refresh callback, `refreshSecret(name)` reads the current value through it and keeps every value read redacted, and `isSecretHeld(name)` reports a held Secret before its first read. +Let the secret registry hold Secrets whose values live elsewhere: `holdRefreshableSecret(name, owner, { refresh, value?, expiresAt? })` registers a refresh callback, `refreshSecret(name)` reads the current value through it and keeps every value read redacted, and `isSecretHeld(name)` reports a held Secret before its first read. `redactionGeneration()` counts the values held so far, and `redactSecrets(value, generation)` redacts only the values held by that generation, so text redacted once can be redacted the same way later. diff --git a/packages/pi-secret/GLOSSARY.md b/packages/pi-secret/GLOSSARY.md index 631893af..ed025923 100644 --- a/packages/pi-secret/GLOSSARY.md +++ b/packages/pi-secret/GLOSSARY.md @@ -47,3 +47,7 @@ _Avoid_: TTL, session lifetime, timeout **Redaction**: Replacing every occurrence of a held Secret's value, as is or in a common encoding such as base64, with its Secret Reference in what a tool returns or the provider receives. _Avoid_: Masking, scrubbing + +**Redaction Generation**: +How many values have been held in the process so far. A message is redacted at the generation it was first sent at, so values held later never change what the provider already cached. +_Avoid_: Redaction version, epoch diff --git a/packages/pi-secret/README.md b/packages/pi-secret/README.md index df33946d..729809c6 100644 --- a/packages/pi-secret/README.md +++ b/packages/pi-secret/README.md @@ -24,7 +24,7 @@ pi -e ./src/index.ts 4. The agent uses the Secret through a reference: - **`$PI_SECRET_NAME`** in shell commands. While a Secret is held, Pi Secret exports it as an environment variable of the Pi process, so `bash`, Terminal Control's terminals, and other spawned processes inherit it. The value stays off the command line. - **`{{secret:NAME}}`** in any string argument of any tool, such as a `write` to a `.env` file or a header of an HTTP tool. Pi Secret replaces it just before the tool runs. The transcript, the session file, and the provider keep the reference. -5. Every tool result is redacted before Pi stores it or sends it to the provider. Each occurrence of a held value, or of any value held earlier in this process, is replaced with its `{{secret:NAME}}` reference, whether it is printed as is or in a common encoding: JSON string escaping, percent-encoding, hex, or base64 and base64url at any alignment, as in an HTTP Basic `Authorization` header. Encoded forms are matched only for values of 8 characters or more, since shorter ones can occur by chance. This covers `content`, `details`, and `structuredContent`, nested `codemode` calls and the outer `codemode` result, and the full output file a truncated `bash` result points at (`fullOutputPath` or `full_output_path`), which is redacted in place. If a result cannot be checked, it is withheld and replaced with an error. Each provider request is also scrubbed, so a value the user pastes into the chat by mistake is replaced as well. +5. Every tool result is redacted before Pi stores it or sends it to the provider. Each occurrence of a held value, or of any value held earlier in this process, is replaced with its `{{secret:NAME}}` reference, whether it is printed as is or in a common encoding: JSON string escaping, percent-encoding, hex, or base64 and base64url at any alignment, as in an HTTP Basic `Authorization` header. Encoded forms are matched only for values of 8 characters or more, since shorter ones can occur by chance. This covers `content`, `details`, and `structuredContent`, nested `codemode` calls and the outer `codemode` result, and the full output file a truncated `bash` result points at (`fullOutputPath` or `full_output_path`), which is redacted in place. If a result cannot be checked, it is withheld and replaced with an error. Each provider request is also scrubbed, so a value the user pastes into the chat by mistake is replaced as well. A message already sent is scrubbed the same way on every later request, so a value held afterwards never rewrites it and the provider's cached prompt prefix stays valid. A call that references a Secret this session does not hold, in either form, is blocked before it runs. For a name that expired, was forgotten, or was never provided, the agent is told to call `secret_ask` again; for a malformed name, such as `{{secret:lower}}` or an empty `{{secret:}}`, it is told what a Secret Name looks like. A Secret Name must be environment-variable style: `^[A-Z_][A-Z0-9_]*$`. @@ -107,6 +107,7 @@ Child Agents, such as Minimal Subagents children in the same Pi process, can use - **Other extensions' hooks**: the arguments object Pi passes to later `tool_call` and `tool_result` handlers holds the resolved value, and a `tool_result` handler that runs before Pi Secret's sees unredacted output. Pi Guardian redacts its own copy (ADR-0008). - **Values the agent or user write themselves**: Pi Secret scrubs provider requests, but a value typed into the chat is still saved in the session file as typed. Likewise, a user shell command such as `!env` prints the `PI_SECRET_*` variables to the screen and records them in the session. - **Calls another extension blocks**: Pi skips `tool_result` for a blocked call, so if an extension loaded after Pi Secret blocks a call and quotes its resolved arguments in the reason, that text is not redacted. +- **Values seen before they were held**: a message the provider received before a value was held keeps that value in later requests, because the provider already has it and rewriting the message would discard its prompt cache. This happens when a Secret Grant reads a value that already appeared, such as a username in a 1Password item title that `secret_list` returned. Everything sent after the value is held is redacted. - **One value under two names**: redaction names the Secret entered last with that value. - **Other transformations**: a value the agent reverses, hashes, splits, compresses, or prints only in part is not recognized, so it is not redacted. Redaction is a safety net for accidental output, not a defence against a command built to reveal a Secret. - **Mentioning the prefix**: any unescaped `PI_SECRET_NAME` word in a call's arguments counts as a reference, so text that only mentions an unheld one, such as `env | grep PI_SECRET_X`, is blocked; write `\PI_SECRET_X` instead. The bare prefix names no Secret, so `env | grep PI_SECRET_` runs, and its output is redacted. diff --git a/packages/pi-secret/docs/adr/0002-redact-sent-messages-at-their-first-generation.md b/packages/pi-secret/docs/adr/0002-redact-sent-messages-at-their-first-generation.md new file mode 100644 index 00000000..c7c6a65b --- /dev/null +++ b/packages/pi-secret/docs/adr/0002-redact-sent-messages-at-their-first-generation.md @@ -0,0 +1,14 @@ +# Redact each message the provider received at the Redaction Generation it was first sent at + +Pi Secret scrubs every provider request in the `context` hook, because values can reach messages without passing through a tool result, such as a value the user pastes into the chat. Redacting the whole history again with every value held so far rewrote earlier messages whenever a new value appeared in them, which changed the prompt prefix and discarded the provider's cache. Secret Grants made this common: a grant often reads a value the model has already seen, such as a username in a 1Password item title that `secret_list` returned. Pi Secret therefore records, for each message, the Redaction Generation it was first sent at, and redacts it at that generation on every later request. Messages sent afterwards are redacted with every value held. + +## Considered Options + +- **Redact the whole history at the current generation**: rejected because the value had already reached the provider in that message, so rewriting it later hides little, and each rewrite costs a full uncached request. +- **Key messages by object identity or timestamp**: rejected because Pi clones the messages for every request, and timestamps can collide. Messages are keyed by a hash of their content instead. + +## Consequences + +- A value seen before it was held stays in the messages that already carried it, in every later request in that process. +- A message another extension rewrites gets a new hash, so it is redacted at the current generation, like a new message. +- After a restart the records are empty, so every message is redacted at the generation when it is first sent in the new process. That request starts a fresh cache anyway. diff --git a/packages/pi-secret/src/pi-secret-extension.ts b/packages/pi-secret/src/pi-secret-extension.ts index 5529bf74..7ec0000b 100644 --- a/packages/pi-secret/src/pi-secret-extension.ts +++ b/packages/pi-secret/src/pi-secret-extension.ts @@ -1,3 +1,4 @@ +import { createHash } from "node:crypto"; import { accessSync, constants } from "node:fs"; import { lstat, readFile, realpath, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; @@ -7,6 +8,7 @@ import { defineTool, getAgentDir, SettingsManager, + type ContextEvent, type ExtensionAPI, type ExtensionContext, type ExtensionFactory, @@ -18,6 +20,7 @@ import { holdRefreshableSecret, holdSecret, isSecretHeld, + redactionGeneration, redactSecrets, refreshSecret, resolveSecret, @@ -443,6 +446,29 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio const vault = () => new SecretVault(keyFilePath(getAgentDirectory()), now); // Decided once at load, so the tool set and system prompt never change within a session. const providers = (options.providers ?? defaultProviders)(() => providerSettings); + /** + * The Redaction Generation each message was first sent at, by a hash of its content: Pi + * clones the messages for every request, so they have no lasting identity. + */ + const sentAt = new Map(); + + /** + * The Redaction Generation to redact `message` at: the one it was first sent at, so a value + * held later never rewrites it and breaks the provider's cached prompt prefix. That value + * already reached the provider in this message (ADR-0002). + */ + function sentGeneration(message: ContextEvent["messages"][number], current: number): number { + let key: string; + try { + key = createHash("sha256").update(JSON.stringify(message)).digest("base64"); + } catch { + return current; + } + const sent = sentAt.get(key); + if (sent !== undefined) return sent; + sentAt.set(key, current); + return current; + } /** Stop holding one Secret here; a typed Secret's key is left for the caller to handle. */ function release(name: string): LiveSecret | undefined { @@ -674,8 +700,14 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio }); pi.on("context", (event) => { - const messages = redactSecrets(event.messages); - return messages === event.messages ? undefined : { messages }; + const current = redactionGeneration(); + let changed = false; + const messages = event.messages.map((message) => { + const next = redactSecrets(message, sentGeneration(message, current)); + if (next !== message) changed = true; + return next; + }); + return changed ? { messages } : undefined; }); const needsTerminal = (tool: string, what: string) => diff --git a/packages/pi-secret/test/pi-secret-extension.test.ts b/packages/pi-secret/test/pi-secret-extension.test.ts index 453fc044..16e33d5e 100644 --- a/packages/pi-secret/test/pi-secret-extension.test.ts +++ b/packages/pi-secret/test/pi-secret-extension.test.ts @@ -1,6 +1,7 @@ import { readFile, symlink, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; +import { FakeProvider } from "./fixtures/fake-provider.js"; import { createSecretHarness, keys, reply, toolCalls } from "./fixtures/secret-harness.js"; describe("Secret Request and Secret References", () => { @@ -389,4 +390,30 @@ describe("provider requests", () => { expect(third?.slice(0, second?.length)).toEqual(second); expect(JSON.stringify(second)).toContain("Oops, I pasted it here too: {{secret:CHAT_TYPED}}"); }); + + it("leaves messages already sent unchanged when a later Secret's value appears in them, so the cached prefix survives", async () => { + const value = "deploy-user@example.com"; + const locator = "fake://Dev/Deploy/username"; + const harness = await createSecretHarness({ + providers: [new FakeProvider({ [locator]: value })], + dialogs: [[keys.enter]], + }); + await harness.session.prompt(`The deploy account is ${value}.`); + harness.responses.push( + toolCalls(["secret_ask_grant", { grants: [{ name: "DEPLOY_USER", locator }] }, "call-grant"]), + toolCalls(["echo", { text: "user={{secret:DEPLOY_USER}}" }, "call-echo"]), + reply("Done."), + ); + await harness.session.prompt("Grant it."); + await harness.session.prompt(`Was it ${value}?`); + + // Each request's messages begin with the previous request's messages, unchanged. + for (const [index, earlier] of harness.requestMessages.slice(0, -1).entries()) + expect(harness.requestMessages[index + 1]?.slice(0, earlier.length)).toEqual(earlier); + // The message sent before the grant keeps the value; everything after it is redacted. + const last = JSON.stringify(harness.requestMessages.at(-1)); + expect(last).toContain(`The deploy account is ${value}.`); + expect(last).toContain("echo: user={{secret:DEPLOY_USER}}"); + expect(last).toContain("Was it {{secret:DEPLOY_USER}}?"); + }); }); diff --git a/packages/pi-utils/src/secret-registry.ts b/packages/pi-utils/src/secret-registry.ts index a3e94215..bb13a429 100644 --- a/packages/pi-utils/src/secret-registry.ts +++ b/packages/pi-utils/src/secret-registry.ts @@ -191,33 +191,37 @@ function encodedForms(value: string): string[] { } interface RedactionPattern { - /** How many values were redacted when the pattern was built; values are only ever added. */ - readonly size: number; readonly pattern: RegExp; /** The held value each matchable text stands for, whether written as is or encoded. */ readonly sources: ReadonlyMap; } -let cachedPattern: RedactionPattern | undefined; +/** + * Patterns by Redaction Generation. Values are only ever added, in order, so a generation always + * names the same values; few generations are ever asked for. + */ +const cachedPatterns = new Map(); -function redactionPattern(values: Map): RedactionPattern | undefined { - if (values.size === 0) return undefined; - if (cachedPattern?.size !== values.size) { +function redactionPattern( + values: Map, + generation: number, +): RedactionPattern | undefined { + if (generation === 0) return undefined; + let cached = cachedPatterns.get(generation); + if (cached === undefined) { + const included = [...values.keys()].slice(0, generation); const sources = new Map(); - for (const value of values.keys()) + for (const value of included) for (const form of encodedForms(value)) if (!sources.has(form)) sources.set(form, value); // A value as written wins over another value's encoded form that happens to equal it. - for (const value of values.keys()) sources.set(value, value); + for (const value of included) sources.set(value, value); const alternatives = [...sources.keys()] .toSorted((a, b) => b.length - a.length) .map(escapeRegExp); - cachedPattern = { - size: values.size, - pattern: new RegExp(alternatives.join("|"), "g"), - sources, - }; + cached = { pattern: new RegExp(alternatives.join("|"), "g"), sources }; + cachedPatterns.set(generation, cached); } - return cachedPattern; + return cached; } function redactValue(value: T, values: Map, pattern: RedactionPattern): T { @@ -261,10 +265,20 @@ function redactValue(value: T, values: Map, pattern: Redactio * `value` with every occurrence of any value held in this process, written as is or in one of its * common encodings (JSON string escaping, percent-encoding, hex, base64, base64url), replaced by * its Secret Reference, in strings nested in plain objects and arrays. Returns `value` itself when nothing - * was redacted, and never modifies it. + * was redacted, and never modifies it. With `generation`, only the values held by that + * {@link redactionGeneration} are redacted. */ -export function redactSecrets(value: T): T { +export function redactSecrets(value: T, generation?: number): T { const values = redacted(); - const pattern = redactionPattern(values); + const pattern = redactionPattern(values, Math.min(generation ?? values.size, values.size)); return pattern === undefined ? value : redactValue(value, values, pattern); } + +/** + * The Redaction Generation: how many values have been held in this process. It only grows, so + * text redacted at one generation can be redacted the same way later, even after more values are + * held, as a message already sent to a model provider must be to keep its cached prompt prefix. + */ +export function redactionGeneration(): number { + return redacted().size; +} diff --git a/packages/pi-utils/test/secret-registry.test.ts b/packages/pi-utils/test/secret-registry.test.ts index 72651ce8..377c7fb7 100644 --- a/packages/pi-utils/test/secret-registry.test.ts +++ b/packages/pi-utils/test/secret-registry.test.ts @@ -3,6 +3,7 @@ import { heldSecretNames, holdRefreshableSecret, holdSecret, + redactionGeneration, redactSecrets, refreshSecret, resolveSecret, @@ -120,6 +121,22 @@ test("holding a Secret again replaces its value and keeps redacting the old one" ); }); +test("redacting at an earlier Redaction Generation leaves values held since then as written", () => { + holdSecret("REGISTRY_EARLIER", "earlier-value-2468", owner); + const generation = redactionGeneration(); + holdSecret("REGISTRY_LATER", "later-value-1357", owner); + // Holding a value again, even under another name, keeps the generation. + holdSecret("REGISTRY_EARLIER_AGAIN", "earlier-value-2468", owner); + expect(redactionGeneration()).toBe(generation + 1); + + const text = "earlier-value-2468 later-value-1357"; + expect(redactSecrets(text, generation)).toBe( + "{{secret:REGISTRY_EARLIER_AGAIN}} later-value-1357", + ); + expect(redactSecrets(text, generation + 1)).toBe(redactSecrets(text)); + expect(redactSecrets(text, 0)).toBe(text); +}); + test("every copy of pi-utils in the process shares one registry", async () => { // A second module instance, as when two packages each install their own pi-utils. const copy: typeof import("../src/secret-registry.js") = await import(