From e3aa13099101975e348eaac3f9b23cd166828d06 Mon Sep 17 00:00:00 2001 From: Ian Pascoe Date: Tue, 20 Oct 2026 11:53:31 -0400 Subject: [PATCH 1/2] feat(secret): grant Secrets from Env Files and save typed Secrets into them Adds the Env File Secret Provider (dotenv://#). Closes #478. --- .changeset/env-file-secret-provider.md | 5 + packages/pi-secret/GLOSSARY.md | 4 + packages/pi-secret/README.md | 55 ++- .../0003-save-typed-secrets-into-env-files.md | 13 + packages/pi-secret/skills/pi-secret/SKILL.md | 11 +- packages/pi-secret/src/approval-dialog.ts | 8 + packages/pi-secret/src/env-file-provider.ts | 423 ++++++++++++++++++ .../pi-secret/src/one-password-provider.ts | 3 + packages/pi-secret/src/pi-secret-extension.ts | 195 +++++--- packages/pi-secret/src/secret-provider.ts | 12 +- packages/pi-secret/src/secret-settings.ts | 142 ++++-- .../pi-secret/test/env-file-grants.test.ts | 352 +++++++++++++++ .../pi-secret/test/env-file-provider.test.ts | 361 +++++++++++++++ .../pi-secret/test/fixtures/fake-provider.ts | 2 + .../pi-secret/test/fixtures/secret-harness.ts | 22 +- .../test/null-optional-arguments.test.ts | 6 +- 16 files changed, 1497 insertions(+), 117 deletions(-) create mode 100644 .changeset/env-file-secret-provider.md create mode 100644 packages/pi-secret/docs/adr/0003-save-typed-secrets-into-env-files.md create mode 100644 packages/pi-secret/src/env-file-provider.ts create mode 100644 packages/pi-secret/test/env-file-grants.test.ts create mode 100644 packages/pi-secret/test/env-file-provider.test.ts diff --git a/.changeset/env-file-secret-provider.md b/.changeset/env-file-secret-provider.md new file mode 100644 index 00000000..f3c6383e --- /dev/null +++ b/.changeset/env-file-secret-provider.md @@ -0,0 +1,5 @@ +--- +"@ian-pascoe/pi-secret": minor +--- + +Add the Env File Secret Provider: grant Secrets from `.env` and `.env.*` files in the project, or files listed in global `secret.providers.dotenv.files`, through `dotenv://#` Secret Locators, read again at each use, and save typed Secrets into them as new keys. It is on when Pi starts in a directory with a top-level Env File, or when global settings list files or set `enabled: true`; global `enabled: false` turns it off. The approval dialogs warn when git does not ignore a project Env File. Project settings for one Secret Provider no longer replace global settings for the others. diff --git a/packages/pi-secret/GLOSSARY.md b/packages/pi-secret/GLOSSARY.md index ed025923..4a976f02 100644 --- a/packages/pi-secret/GLOSSARY.md +++ b/packages/pi-secret/GLOSSARY.md @@ -32,6 +32,10 @@ _Avoid_: Path, address, provider 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 +**Env File**: +A file of `NAME=value` lines, named `.env` or `.env.*` in the project or listed in the user's settings, that Secret Grants can be read from and typed Secrets saved into. +_Avoid_: Dotenv, environment file, secrets file + **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 diff --git a/packages/pi-secret/README.md b/packages/pi-secret/README.md index 729809c6..616974ce 100644 --- a/packages/pi-secret/README.md +++ b/packages/pi-secret/README.md @@ -2,9 +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**. -It can also read Secrets from a **Secret Provider**, currently 1Password, through **Secret Grants** the user approves, and save a typed Secret into one. +It can also read Secrets from a **Secret Provider**, 1Password or the project's **Env Files**, 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. +Requires Node `>=22.19.0` and Pi `>=1.1.0`. The 1Password Secret Provider needs the [1Password CLI](https://developer.1password.com/docs/cli/) (`op`), signed in; Env Files need nothing else. > **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. @@ -32,14 +32,45 @@ To write either form as literal text, such as in documentation or tests, put a b ### 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. +Pi Secret has two Secret Providers, each decided once at load: -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. +- **1Password** (`1password`), when the 1Password CLI (`op`) is on `PATH`. +- **Env files** (`dotenv`), when Pi starts in a directory with a `.env` or `.env.*` file at its top level, or when global settings list Env Files or set `enabled: true` (see [Env Files](#env-files)). Global `enabled: false` turns it off. + +With at least one, Pi Secret adds `secret_ask_grant` and `secret_save`, and one line of guidance to the system prompt naming the providers loaded. 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`, or an Env File locator such as `dotenv://.env.local#API_KEY`. 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. +3. Pi Secret reads each allowed grant once, so a wrong locator, a missing key, 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 its Secret Provider on every tool call that references it, so one-time passwords (`?attribute=otp`) are always current and a value rotated, revoked, or edited 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 somewhere new, after the user approves it in a dialog, and never changes anything that exists: + +- Into a **new** 1Password item, such as `op://Dev/OpenAI API/credential`. The category defaults to API Credential. It refuses a title the vault already has. The value reaches `op` on stdin, never on its command line. The grant's locator uses the new item's ID, so it survives renames. +- As a **new key** in an Env File, such as `dotenv://.env.local#OPENAI_API_KEY`. It refuses a key the file already defines, and `category`. Pi Secret appends one `KEY=value` line itself, quoted so the file reads back exactly the value, and refuses a value no quoting can round-trip. A missing `.env` or `.env.*` file is created with mode `0600`, inside the project or at a listed path only. + +The Secret then becomes a grant on the saved locator, and it keeps its original expiry. + +### Env Files + +An **Env File** is a file of `NAME=value` lines named `.env` or `.env.*` (not `.envrc`) anywhere in the project, the session's working directory. `secret_list` searches the whole project for them, gitignored ones included, but skips `.git` and `node_modules`. Each Env File is an item, shown in a vault named after the project directory, and its keys are its fields. + +- **Locators** are `dotenv://#`. A path in the project is relative to it, such as `dotenv://packages/api/.env#DATABASE_URL`; a path outside is absolute, such as `dotenv:///Users/me/.config/tool/.env#TOKEN`, and `~/` is accepted. A relative locator in a resumed grant resolves against the session's project directory. +- **Values** are parsed as Node's `util.parseEnv` does: the last definition of a key wins, and `${VAR}` is kept literally, never expanded. +- **Links**: an Env File that links outside the project is used only when its target is listed in settings. +- **Git**: when git does not ignore a project Env File, the grant and save dialogs say "not ignored by git". This is a warning only; approving still grants or saves ([ADR-0003](docs/adr/0003-save-typed-secrets-into-env-files.md)). + +To use other files, such as one in your home directory, list their exact paths, absolute or `~/…`, in global settings: + +```json +{ + "secret": { + "providers": { "dotenv": { "files": ["~/.config/tool/.env"] } } + } +} +``` -`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. +Only global settings may list files outside the project; a trusted project's settings may list files inside it, and any entry outside it is ignored with a warning. `enabled: true` turns the provider on without a top-level Env File, and `enabled: false` turns it off; both work in global settings only, because Pi decides the tool set before it knows the project. 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. @@ -50,7 +81,7 @@ A Secret Name holds one Secret at a time: a newer `secret_ask`, `secret_ask_gran | `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_save` | `name`, `locator`, `category?` | The new item's or key'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. @@ -83,7 +114,7 @@ The 1Password Secret Provider uses `op`'s default account and every vault `op` c } ``` -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 Provider's project settings replace that provider's global settings only; listed Env Files from both add up. Invalid settings fall back to a 15-minute lifetime, no 1Password vaults, and only the project's own Env Files, with a warning, so a mistyped 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: @@ -91,7 +122,7 @@ 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. -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. +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. Saving into an Env File writes the value to that file in plaintext; when the Secret Lifetime ends, the grant ends, but the key stays in the file. 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)). @@ -100,7 +131,9 @@ Child Agents, such as Minimal Subagents children in the same Pi process, can use - **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. +- **Reads cost time**: each tool call using a granted 1Password Secret runs `op read` first, typically well under a second, and fails while 1Password is locked or `op` is signed out. +- **Env Files are plaintext**: Pi Secret reads them only through grants the user approves, but the agent's own tools can read an Env File unless Pi Guardian or another policy stops them. `secret_list` sends Env File paths and key names, never values, to the model provider. +- **Large projects**: `secret_list` walks the whole project for Env Files, outside `.git` and `node_modules`, on each search; a very large tree makes it slow. - **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. diff --git a/packages/pi-secret/docs/adr/0003-save-typed-secrets-into-env-files.md b/packages/pi-secret/docs/adr/0003-save-typed-secrets-into-env-files.md new file mode 100644 index 00000000..1dbb3d68 --- /dev/null +++ b/packages/pi-secret/docs/adr/0003-save-typed-secrets-into-env-files.md @@ -0,0 +1,13 @@ +# Save typed Secrets into Env Files, warning but not blocking when git does not ignore them + +The Env File Secret Provider reads Secret Grants from `.env*` files in the project and from files listed in global settings, and it also saves typed Secrets into them, even though Pi Guardian treats `.env*` files as sensitive and a project Env File may be committed. Users keep project credentials in Env Files already, so a read-only provider would leave a typed Secret with no durable home except 1Password. Saving only ever adds a key: it refuses a key the file already defines, and creates a missing file with owner-only permissions. When git does not ignore a project Env File, the approval dialog says so, but the save or grant still goes ahead if the user approves. Only global settings may list Env Files outside the project, because a trusted project's own settings could otherwise point the agent at credentials anywhere in the user's home directory. + +## Considered Options + +- **Read-only Env Files**: rejected because users who keep credentials in Env Files would have to copy a typed value there by hand, and the agent can already write a value into any file through a Secret Reference. +- **Refuse files git does not ignore**: rejected because the user chose to accept that risk; the dialog warning makes it visible when they approve. +- **Let project settings list any Env File**: rejected for the reason above; this matches how the 1Password vault allowlist fails closed. + +## Consequences + +- A saved value lands in a plaintext file the user owns. Its Secret Lifetime still ends the grant, but it does not delete the key from the file. diff --git a/packages/pi-secret/skills/pi-secret/SKILL.md b/packages/pi-secret/skills/pi-secret/SKILL.md index 9fcc168d..0fbe51be 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 Grants from 1Password, 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 or Env Files, Secret References, redaction, the Secret Lifetime setting, or restored Secrets are missing or incorrect. license: MIT disable-model-invocation: true --- @@ -12,7 +12,8 @@ disable-model-invocation: true 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_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. +6. If an Env File grant or save fails, read the error: the file or key may be gone, the file may not be a `.env` or `.env.*` file in the project (outside `.git` and `node_modules`), or it may link outside the project or sit outside it without being listed in global `secret.providers.dotenv.files`. If the tools are missing although Env Files exist, Pi did not start in a directory with a top-level Env File, or global `secret.providers.dotenv.enabled` is `false`; a project's `enabled` and project-listed files outside the project are ignored with a warning at session start. A save refuses a key the file already defines, `category`, and a value no quoting reads back exactly. "not ignored by git" in a dialog is a warning only. +7. 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. +8. 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. +9. 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. +10. 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 index a69a25ac..faa1e03a 100644 --- a/packages/pi-secret/src/approval-dialog.ts +++ b/packages/pi-secret/src/approval-dialog.ts @@ -19,6 +19,8 @@ export type ApprovalDialogResult = export interface ApprovalRow { readonly name: string; readonly detail: string; + /** A risk to see before approving, such as an Env File git does not ignore. */ + readonly warning?: string; } export interface ApprovalDialogOptions { @@ -110,6 +112,12 @@ export class ApprovalDialog implements Focusable { // 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}`)); + if (row.warning !== undefined) + for (const line of wrapTextWithAnsi( + theme.fg("warning", row.warning), + Math.max(1, inner - 4), + )) + lines.push(pad(` ${line}`)); }); lines.push(""); lines.push( diff --git a/packages/pi-secret/src/env-file-provider.ts b/packages/pi-secret/src/env-file-provider.ts new file mode 100644 index 00000000..fab66e60 --- /dev/null +++ b/packages/pi-secret/src/env-file-provider.ts @@ -0,0 +1,423 @@ +import { execFile } from "node:child_process"; +import { readdirSync } from "node:fs"; +import { lstat, open, readdir, readFile, realpath, stat } from "node:fs/promises"; +import { homedir } from "node:os"; +import { basename, dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; +import { parseEnv } from "node:util"; +import type { + AvailableEntry, + LocatorDescription, + ProviderListing, + ProviderSearch, + SaveRequest, + SecretProvider, +} from "./secret-provider.js"; + +/** An extra Env File listed in settings, with the settings scope that listed it. */ +export interface ListedEnvFile { + /** Absolute, or relative to the home directory with `~/`. */ + readonly path: string; + /** Only a file listed in global settings may be outside the project. */ + readonly scope: "global" | "project"; +} + +/** The Env File Secret Provider's settings, as read at session start. */ +export interface EnvFileSettings { + readonly files: readonly ListedEnvFile[]; +} + +/** + * Whether git ignores a path: `true` or `false`, or undefined outside a git repository or when + * git cannot tell. + */ +export type GitIgnoreCheck = (path: string, signal?: AbortSignal) => Promise; + +const GIT_TIMEOUT_MS = 10_000; + +/** Ask `git check-ignore` from the path's directory, so the repository holding it answers. */ +export const gitIgnores: GitIgnoreCheck = (path, signal) => + new Promise((done) => { + execFile( + "git", + ["check-ignore", "--quiet", "--", path], + { cwd: dirname(path), signal, timeout: GIT_TIMEOUT_MS }, + (error) => { + if (error === null) done(true); + // Exit 1: not ignored. Anything else, such as 128 outside a repository: unknown. + else done(error.code === 1 ? false : undefined); + }, + ); + }); + +/** `.env` or `.env.*`, such as `.env.local`; not `.envrc`. */ +export function isEnvFileName(name: string): boolean { + return name === ".env" || name.startsWith(".env."); +} + +/** Directories never searched for Env Files, nor read from without being listed. */ +const SKIPPED_DIRECTORIES = new Set([".git", "node_modules"]); + +/** Whether a directory has an Env File at its top level, checked once at load. */ +export function hasTopLevelEnvFile(directory: string): boolean { + try { + return readdirSync(directory, { withFileTypes: true }).some( + (entry) => (entry.isFile() || entry.isSymbolicLink()) && isEnvFileName(entry.name), + ); + } catch { + return false; + } +} + +/** `~` and `~/…` → the full path under the home directory; other paths are unchanged. */ +export function expandHome(path: string): string { + if (path === "~") return homedir(); + if (path.startsWith("~/") || path.startsWith(`~${sep}`)) return join(homedir(), path.slice(2)); + return path; +} + +/** `path` relative to `directory` when strictly inside it, else undefined; both absolute. */ +function inside(directory: string, path: string): string | undefined { + const relation = relative(directory, path); + if (relation === "" || isAbsolute(relation)) return undefined; + if (relation === ".." || relation.startsWith(`..${sep}`)) return undefined; + return relation; +} + +/** Whether a path, absolute or with `~/`, is inside the project directory, without following links. */ +export function isInsideProject(projectDirectory: string, path: string): boolean { + return inside(resolve(projectDirectory), resolve(expandHome(path))) !== undefined; +} + +function isMissing(cause: unknown): boolean { + return cause instanceof Error && "code" in cause && cause.code === "ENOENT"; +} + +const SCHEME = "dotenv://"; +const LOCATOR_HELP = "use dotenv://#, such as dotenv://.env#API_KEY"; +/** Keys secret_save writes: environment-variable style, which every Env File reader accepts. */ +const SAVE_KEY = /^[A-Za-z_][A-Za-z0-9_]*$/; +/** Values written without quotes; anything else is quoted. */ +const PLAIN_VALUE = /^[\w.\-/+=:@%,~]+$/; + +/** The parts of an Env File locator: `dotenv://#`. */ +interface ParsedLocator { + readonly path: string; + readonly key: string; +} + +function parseLocator(locator: string): ParsedLocator { + if (!locator.startsWith(SCHEME)) + throw new Error(`${JSON.stringify(locator)} is not an Env File locator: ${LOCATOR_HELP}`); + const rest = locator.slice(SCHEME.length); + const hash = rest.lastIndexOf("#"); + const path = rest.slice(0, Math.max(0, hash)); + const key = rest.slice(hash + 1); + if (hash < 0 || path.trim() === "" || key.trim() === "") + throw new Error(`${JSON.stringify(locator)} is not an Env File locator: ${LOCATOR_HELP}`); + return { path, key }; +} + +/** An Env File path, resolved against the project directory. */ +interface EnvFilePath { + /** Absolute, as named: links are not followed. */ + readonly absolute: string; + /** Relative to the project when inside it, else absolute: how a locator names it. */ + readonly shown: string; + /** Relative to the project, when the named path is inside it. */ + readonly inProject: string | undefined; +} + +/** An Env File the provider may read or save into. */ +interface PermittedFile extends EnvFilePath { + /** Where the file really is, links followed: the only path read or written. */ + readonly real: string; + readonly exists: boolean; +} + +function notFound(file: EnvFilePath, cause: unknown): Error { + if (isMissing(cause)) return new Error(`${file.shown} does not exist`, { cause }); + const message = cause instanceof Error ? cause.message : String(cause); + return new Error(`cannot read ${file.shown}: ${message}`, { cause }); +} + +/** The text of a key's new line, quoted so `util.parseEnv` reads back exactly `value`. */ +function appendedLine(existing: string, key: string, value: string): string { + const prefix = existing === "" || existing.endsWith("\n") ? "" : "\n"; + const before = parseEnv(existing); + const candidates = PLAIN_VALUE.test(value) ? [value] : []; + candidates.push(`'${value}'`, `"${value}"`, `\`${value}\``); + for (const quoted of candidates) { + const line = `${prefix}${key}=${quoted}\n`; + const after = parseEnv(existing + line); + const unchanged = Object.entries(before).every(([name, old]) => after[name] === old); + const added = Object.keys(after).length === Object.keys(before).length + 1; + if (after[key] === value && unchanged && added) return line; + } + throw new Error( + "Pi Secret cannot write this value so that the Env File reads it back exactly; save it to another Secret Provider", + ); +} + +export interface EnvFileProviderOptions { + /** The session's project directory, which relative locators resolve against. */ + readonly projectDirectory: () => string; + readonly settings: () => EnvFileSettings; + readonly gitIgnores?: GitIgnoreCheck; +} + +/** + * The Env File Secret Provider: `.env*` files in the project, and files listed in settings, + * read again at each use. Saving only ever adds a key (ADR-0003). + */ +export class EnvFileProvider implements SecretProvider { + readonly id = "dotenv"; + readonly label = "Env files"; + readonly scheme = "dotenv"; + readonly locatorExample = "the Env File locator dotenv://.env#API_KEY"; + readonly saveExample = "dotenv://.env.local#NEW_KEY, for a key the file does not define yet"; + private readonly checkIgnored: GitIgnoreCheck; + + constructor(private readonly options: EnvFileProviderOptions) { + this.checkIgnored = options.gitIgnores ?? gitIgnores; + } + + private project(): string { + return resolve(this.options.projectDirectory()); + } + + private path(path: string): EnvFilePath { + const project = this.project(); + const absolute = resolve(project, expandHome(path)); + const inProject = inside(project, absolute); + return { absolute, shown: inProject ?? absolute, inProject }; + } + + /** The vault an Env File is shown in: the project directory's name, or `outside project`. */ + private vault(file: EnvFilePath): string { + return file.inProject === undefined ? "outside project" : basename(this.project()); + } + + /** + * Check that Pi Secret may use a file, following links: a `.env*` file whose real path is in + * the project, or a listed file. A project-listed file must really be in the project too. + */ + private async permit(file: EnvFilePath, creating = false): Promise { + let real: string; + let exists = true; + try { + real = await realpath(file.absolute); + } catch (cause) { + if (!isMissing(cause) || !creating) throw notFound(file, cause); + // A dangling link would make the write land wherever it points. + if ( + await lstat(file.absolute).then( + () => true, + () => false, + ) + ) + throw new Error(`${file.shown} is a link to a file that does not exist`); + try { + real = join(await realpath(dirname(file.absolute)), basename(file.absolute)); + } catch (inner) { + throw new Error(`the directory of ${file.shown} does not exist`, { cause: inner }); + } + exists = false; + } + if (exists && !(await stat(real)).isFile()) throw new Error(`${file.shown} is not a file`); + if (!exists && !isEnvFileName(basename(file.absolute))) + throw new Error( + `secret_save creates only files named .env or .env.*, not ${basename(file.absolute)}`, + ); + const realProject = await realpath(this.project()).catch(() => this.project()); + const inRealProject = inside(realProject, real) !== undefined; + for (const listed of this.options.settings().files) { + const path = resolve(expandHome(listed.path)); + const listedReal = await realpath(path).catch(() => path); + if (path !== file.absolute && listedReal !== real) continue; + if (listed.scope === "global") return { ...file, real, exists }; + if (inside(this.project(), path) !== undefined && inRealProject) + return { ...file, real, exists }; + } + const segments = file.inProject?.split(sep) ?? []; + if ( + file.inProject !== undefined && + inRealProject && + isEnvFileName(basename(file.absolute)) && + !segments.some((segment) => SKIPPED_DIRECTORIES.has(segment)) + ) + return { ...file, real, exists }; + throw new Error( + `${file.shown} is not an Env File Pi Secret may use: use a .env or .env.* file in the project, outside .git and node_modules, or list the file in global secret.providers.dotenv.files`, + ); + } + + private async keys( + file: PermittedFile, + signal?: AbortSignal, + ): Promise> { + if (!file.exists) return {}; + return parseEnv(await readFile(file.real, { encoding: "utf8", signal })); + } + + /** "not ignored by git" for a project Env File git does not ignore; nothing otherwise. */ + private async gitWarning( + file: PermittedFile, + signal?: AbortSignal, + ): Promise<{ warning?: string }> { + if (file.inProject === undefined) return {}; + return (await this.checkIgnored(file.absolute, signal)) === false + ? { warning: "not ignored by git" } + : {}; + } + + async describe(locator: string, signal?: AbortSignal): Promise { + const { path, key } = parseLocator(locator); + const file = await this.permit(this.path(path)); + if (!Object.hasOwn(await this.keys(file, signal), key)) + throw new Error(`${file.shown} has no key ${JSON.stringify(key)}`); + return { + locator, + vault: this.vault(file), + item: file.shown, + field: key, + ...(await this.gitWarning(file, signal)), + }; + } + + async describeSave( + request: Omit, + signal?: AbortSignal, + ): Promise { + if (request.category !== undefined) + throw new Error("Env Files have no item categories; call secret_save without category"); + const { path, key } = parseLocator(request.locator); + if (!SAVE_KEY.test(key)) + throw new Error( + `${JSON.stringify(key)} is not a key secret_save writes: use letters, digits, and underscores, not starting with a digit`, + ); + const file = await this.permit(this.path(path), true); + if (Object.hasOwn(await this.keys(file, signal), key)) + throw new Error( + `${file.shown} already defines ${key}; secret_save only adds keys, so choose another key or file`, + ); + return { + locator: request.locator, + vault: this.vault(file), + item: file.shown, + field: key, + fieldType: file.exists ? "new key" : "new key, new file", + ...(await this.gitWarning(file, signal)), + }; + } + + async read(locator: string, signal?: AbortSignal): Promise { + // Checked at every read, so a file no longer permitted stops grants already held or restored. + const { path, key } = parseLocator(locator); + const file = await this.permit(this.path(path)); + const keys = await this.keys(file, signal); + const value = Object.hasOwn(keys, key) ? keys[key] : undefined; + if (value === undefined) throw new Error(`${file.shown} has no key ${JSON.stringify(key)}`); + if (value === "") throw new Error(`${key} in ${file.shown} is empty`); + return value; + } + + async save(request: SaveRequest, signal?: AbortSignal): Promise { + await this.describeSave(request, signal); + const { path, key } = parseLocator(request.locator); + const file = await this.permit(this.path(path), true); + const existing = file.exists ? await readFile(file.real, { encoding: "utf8", signal }) : ""; + const line = appendedLine(existing, key, request.value); + // Appended, never rewritten; a new file is created exclusively, readable by its owner only. + const handle = await open(file.real, file.exists ? "a" : "ax", 0o600); + try { + await handle.appendFile(line, "utf8"); + } finally { + await handle.close(); + } + return `${SCHEME}${file.shown}#${key}`; + } + + /** Every Env File in the project, outside skipped directories, without following links. */ + private async projectFiles(signal?: AbortSignal): Promise { + const found: string[] = []; + const visit = async (directory: string): Promise => { + signal?.throwIfAborted(); + let entries; + try { + entries = await readdir(directory, { withFileTypes: true }); + } catch { + return; + } + entries.sort((a, b) => a.name.localeCompare(b.name)); + for (const entry of entries) { + const path = join(directory, entry.name); + if (entry.isDirectory()) { + if (!SKIPPED_DIRECTORIES.has(entry.name)) await visit(path); + } else if ((entry.isFile() || entry.isSymbolicLink()) && isEnvFileName(entry.name)) + found.push(path); + } + }; + await visit(this.project()); + return found; + } + + /** Every Env File Pi Secret may use: the project's, then listed ones, each once. */ + private async files(signal?: AbortSignal): Promise { + const candidates = [ + ...(await this.projectFiles(signal)), + ...this.options.settings().files.map((listed) => listed.path), + ]; + const files = new Map(); + for (const candidate of candidates) { + const path = this.path(candidate); + if (files.has(path.absolute)) continue; + try { + files.set(path.absolute, await this.permit(path)); + } catch { + // Missing, or not permitted: not listed. + } + } + return [...files.values()]; + } + + async search(search: ProviderSearch, signal?: AbortSignal): Promise { + const query = search.query?.toLowerCase(); + const matches = (text: string) => query === undefined || text.toLowerCase().includes(query); + const inVault = (file: EnvFilePath) => + search.vault === undefined || this.vault(file).toLowerCase() === search.vault.toLowerCase(); + const entries: AvailableEntry[] = []; + if (search.item !== undefined) { + const file = await this.permit(this.path(search.item)); + if (inVault(file)) + // Values are dropped here: only key names and locators leave this method. + for (const key of Object.keys(await this.keys(file, signal)).toSorted()) + if (matches(key)) + entries.push({ + kind: "field", + provider: this.id, + item: file.shown, + vault: this.vault(file), + label: key, + type: "key", + locator: `${SCHEME}${file.shown}#${key}`, + }); + } else { + for (const file of await this.files(signal)) { + if (!inVault(file)) continue; + if (!matches(file.shown)) { + const keys = await this.keys(file, signal).catch(() => ({})); + if (!Object.keys(keys).some(matches)) continue; + } + entries.push({ + kind: "item", + provider: this.id, + id: file.shown, + title: file.shown, + vault: this.vault(file), + category: "env file", + }); + } + } + return { entries: entries.slice(0, search.limit), truncated: entries.length > search.limit }; + } +} diff --git a/packages/pi-secret/src/one-password-provider.ts b/packages/pi-secret/src/one-password-provider.ts index bc17afec..5717281c 100644 --- a/packages/pi-secret/src/one-password-provider.ts +++ b/packages/pi-secret/src/one-password-provider.ts @@ -167,6 +167,9 @@ export class OnePasswordProvider implements SecretProvider { readonly id = "1password"; readonly label = "1Password"; readonly scheme = "op"; + readonly locatorExample = "the 1Password Reference op://vault/item/field"; + readonly saveExample = + "op://vault/new item title/credential, for an item that does not exist yet"; constructor( private readonly settings: () => OnePasswordSettings, diff --git a/packages/pi-secret/src/pi-secret-extension.ts b/packages/pi-secret/src/pi-secret-extension.ts index 7ec0000b..a0b9e101 100644 --- a/packages/pi-secret/src/pi-secret-extension.ts +++ b/packages/pi-secret/src/pi-secret-extension.ts @@ -32,7 +32,8 @@ import { import { noticeText } from "@ian-pascoe/pi-utils/ui"; import { Type, type Static } from "typebox"; import { Value } from "typebox/value"; -import { ApprovalDialog, type ApprovalDialogResult } from "./approval-dialog.js"; +import { ApprovalDialog, type ApprovalDialogResult, type ApprovalRow } from "./approval-dialog.js"; +import { EnvFileProvider, hasTopLevelEnvFile } from "./env-file-provider.js"; import { OnePasswordProvider } from "./one-password-provider.js"; import { lifetimeText, @@ -56,6 +57,7 @@ import { } from "./secret-references.js"; import { DEFAULT_LIFETIME_MINUTES, + globalEnvFileSwitch, readSecretSettings, type SecretSettings, } from "./secret-settings.js"; @@ -76,9 +78,15 @@ 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."; +/** Added, once at load, only when a Secret Provider is available; names the ones loaded. */ +export function secretProviderGuideline(providers: readonly SecretProvider[]): string { + return `When a Secret may already be in a Secret Provider such as ${providerNames(providers)}, find it with secret_list and request it with secret_ask_grant instead of asking the user to type it.`; +} + +/** `1Password`, or `1Password or Env files`. */ +function providerNames(providers: readonly SecretProvider[]): string { + return providers.map((provider) => provider.label).join(" or "); +} const SECRET_TOOLS: readonly string[] = [ "secret_ask", @@ -88,16 +96,30 @@ const SECRET_TOOLS: readonly string[] = [ "secret_forget", ]; +/** What a Secret Provider is created with, once at load. */ +export interface ProviderContext { + /** Secret Provider settings, read at each session start. */ + settings(): SecretSettings["providers"]; + /** The session's project directory: its cwd, once a session starts. */ + projectDirectory(): string; +} + +/** Where the default Secret Providers look at load; tests isolate it. */ +export interface ProviderEnvironment { + /** The directory Pi was started in. */ + cwd: string; + agentDirectory: string; + /** Whether an executable is on `PATH`. */ + onPath(command: string): boolean; +} + 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[]; + /** The Secret Providers, created once at load; {@link defaultSecretProviders} by default. */ + providers?: (context: ProviderContext) => SecretProvider[]; } /** How a held Secret got its value: typed by the user, or granted from a Secret Provider. */ @@ -141,11 +163,6 @@ 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, @@ -158,17 +175,24 @@ 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" }), - ), -}); +/** Built once at load from the providers loaded, so it never changes within a session. */ +function askGrantParameters(providers: readonly SecretProvider[]) { + const locator = Type.String({ + minLength: 1, + description: `Secret Locator, such as ${providers.map((provider) => provider.locatorExample).join(" or ")}`, + }); + return Type.Object({ + grants: Type.Array(Type.Object({ name: SecretNameSchema, locator }), { + 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( @@ -200,17 +224,19 @@ const ListParameters = Type.Object({ ), }); -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)" }), - ), -}); +/** Built once at load from the providers loaded, so it never changes within a session. */ +function saveParameters(providers: readonly SecretProvider[]) { + return Type.Object({ + name: SecretNameSchema, + locator: Type.String({ + minLength: 1, + description: `Secret Locator to save to, such as ${providers.map((provider) => provider.saveExample).join(", or ")}`, + }), + category: Type.Optional( + Type.String({ description: "Category of a new 1Password item (default API Credential)" }), + ), + }); +} const ForgetParameters = Type.Object({ names: Type.Array(SecretNameSchema, { @@ -299,8 +325,29 @@ function onPath(command: string): boolean { return false; } -function defaultProviders(settings: () => SecretSettings["providers"]): SecretProvider[] { - return onPath("op") ? [new OnePasswordProvider(() => settings()["1password"] ?? {})] : []; +/** + * The 1Password Secret Provider when `op` is on `PATH` at load, and the Env File Secret Provider + * when global settings turn it on, or, unless they turn it off, when Pi starts in a directory with + * an Env File at its top level. + */ +export function defaultSecretProviders( + context: ProviderContext, + environment: ProviderEnvironment, +): SecretProvider[] { + const providers: SecretProvider[] = []; + if (environment.onPath("op")) + providers.push(new OnePasswordProvider(() => context.settings()["1password"] ?? {})); + const envFiles = + globalEnvFileSwitch(environment.cwd, environment.agentDirectory) ?? + hasTopLevelEnvFile(environment.cwd); + if (envFiles) + providers.push( + new EnvFileProvider({ + projectDirectory: () => context.projectDirectory(), + settings: () => context.settings().dotenv ?? { files: [] }, + }), + ); + return providers; } /** The fields through which Pi's tools point at a full output file. */ @@ -420,10 +467,18 @@ const outcomesResult = (outcomes: SecretOutcome[], lead?: string) => : { outcomes, available: [], notes: [], lead }, ); -function describeLocation(provider: SecretProvider, description: LocatorDescription): string { +/** An approval dialog row: the Secret Name, where it points, and any warning. */ +function approvalRow( + name: string, + provider: SecretProvider, + description: LocatorDescription, +): ApprovalRow { 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}`; + const detail = `${provider.label}${account} · ${description.vault} / ${description.item} / ${description.field}${type}`; + return description.warning === undefined + ? { name, detail } + : { name, detail, warning: description.warning }; } function shortValueWarning(value: string): string | undefined { @@ -442,10 +497,21 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio const live = new Map(); let lifetimeMinutes = DEFAULT_LIFETIME_MINUTES; let providerSettings: SecretSettings["providers"] = {}; + let projectDirectory = process.cwd(); let generation = 0; const vault = () => new SecretVault(keyFilePath(getAgentDirectory()), now); + const providerContext: ProviderContext = { + settings: () => providerSettings, + projectDirectory: () => projectDirectory, + }; // Decided once at load, so the tool set and system prompt never change within a session. - const providers = (options.providers ?? defaultProviders)(() => providerSettings); + const providers = + options.providers?.(providerContext) ?? + defaultSecretProviders(providerContext, { + cwd: process.cwd(), + agentDirectory: getAgentDirectory(), + onPath, + }); /** * 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. @@ -581,15 +647,19 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio noticeText("Pi Secret", `${message}. Run /skill:pi-secret to diagnose.`), "warning", ); - const { settings, warning } = readSecretSettings({ - settingsManager: SettingsManager.create(context.cwd, getAgentDirectory(), { - projectTrusted: context.isProjectTrusted(), - }), - sessionManager: context.sessionManager, - }); + const { settings, warnings } = readSecretSettings( + { + settingsManager: SettingsManager.create(context.cwd, getAgentDirectory(), { + projectTrusted: context.isProjectTrusted(), + }), + sessionManager: context.sessionManager, + }, + context.cwd, + ); lifetimeMinutes = settings.lifetimeMinutes; providerSettings = settings.providers; - if (warning !== undefined) warn(warning); + projectDirectory = context.cwd; + for (const warning of warnings) warn(warning); // 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()) { @@ -801,7 +871,7 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio context: Parameters[4], signal: AbortSignal | undefined, dialog: { title: string; reason: string | undefined; explanation: string }, - rows: { name: string; detail: string }[], + rows: ApprovalRow[], ): Promise | undefined> { const answer = await context.ui.custom( (tui, theme, keybindings, done) => { @@ -825,10 +895,9 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio 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, + description: `Ask the user to grant Secrets from a Secret Provider such as ${providerNames(providers)}, each a Secret Name and the Secret Locator its value is read from at each use. Find locators with secret_list. Returns how to reference each one; never its value.`, + promptGuidelines: [secretProviderGuideline(providers)], + parameters: askGrantParameters(providers), annotations: { readOnlyHint: false, destructiveHint: false, @@ -889,10 +958,9 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio 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), - })), + pending.map(({ index, provider, description }) => + approvalRow(params.grants[index]?.name ?? "", provider, description), + ), ); if (approved === undefined) { for (const { index } of pending) { @@ -941,7 +1009,9 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio name: "secret_list", label: "Secret List", description: - "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.", + providers.length === 0 + ? "List the Secrets this session can use and when each expires; never values." + : `List the Secrets this session can use and when each expires, and search Secret Providers such as ${providerNames(providers)} for items and fields to request with secret_ask_grant; never values.`, parameters: ListParameters, annotations: { readOnlyHint: true, @@ -1016,9 +1086,8 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio 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, + description: `Save a Secret the user typed into this session into a Secret Provider such as ${providerNames(providers)}, after the user approves, as a new item or key; nothing existing is changed. The Secret is then read from there at each use, with its original expiry.`, + parameters: saveParameters(providers), annotations: { readOnlyHint: false, destructiveHint: false, @@ -1055,9 +1124,9 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio { 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)}.`, + explanation: `Pi Secret will save this value to ${provider.label}, at the new location below. The Secret is then read from there at each use, and still expires at ${iso(expiresAt)}.`, }, - [{ name: params.name, detail: describeLocation(provider, description) }], + [approvalRow(params.name, provider, description)], ); if (approved === undefined || !approved.has(0)) return outcomesResult([ diff --git a/packages/pi-secret/src/secret-provider.ts b/packages/pi-secret/src/secret-provider.ts index f0b332bb..e7d2867d 100644 --- a/packages/pi-secret/src/secret-provider.ts +++ b/packages/pi-secret/src/secret-provider.ts @@ -15,6 +15,8 @@ export interface LocatorDescription { readonly fieldType?: string; /** The account the provider reads from, when it names one. */ readonly account?: string; + /** A risk the user should see before approving, such as an Env File git does not ignore. */ + readonly warning?: string; } /** A Secret Provider search; every filter is optional and case-insensitive. */ @@ -53,7 +55,7 @@ export interface ProviderListing { readonly truncated: boolean; } -/** A typed Secret to save into a new item. */ +/** A typed Secret to save into a new item or key. */ export interface SaveRequest { readonly locator: string; readonly value: string; @@ -67,9 +69,13 @@ export interface SecretProvider { readonly label: string; /** The Secret Locator scheme this provider owns, without `://`, such as `op`. */ readonly scheme: string; + /** A locator to read from, for tool descriptions, such as `the 1Password Reference op://…`. */ + readonly locatorExample: string; + /** A locator to save to, for tool descriptions, with what it must not exist as yet. */ + readonly saveExample: 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. */ + /** Check that a save can create what the locator names, and say where it would go. */ describeSave( request: Omit, signal?: AbortSignal, @@ -77,7 +83,7 @@ export interface SecretProvider { /** 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 the value somewhere new, and return the locator to read it from. */ save(request: SaveRequest, signal?: AbortSignal): Promise; } diff --git a/packages/pi-secret/src/secret-settings.ts b/packages/pi-secret/src/secret-settings.ts index bf5bff93..17e43a40 100644 --- a/packages/pi-secret/src/secret-settings.ts +++ b/packages/pi-secret/src/secret-settings.ts @@ -1,6 +1,10 @@ -import type { SessionManager, SettingsManager } from "@earendil-works/pi-coding-agent"; -import { defineLayeredSettings } from "@ian-pascoe/pi-utils/layered-settings"; -import { Type } from "typebox"; +import { SettingsManager, type SessionManager } from "@earendil-works/pi-coding-agent"; +import { + defineLayeredSettings, + type LayeredSettingScope, +} from "@ian-pascoe/pi-utils/layered-settings"; +import { Type, type Static } from "typebox"; +import { isInsideProject, type EnvFileSettings, type ListedEnvFile } from "./env-file-provider.js"; import type { OnePasswordSettings } from "./one-password-provider.js"; /** Pi Secret's settings: the `secret` namespace of Pi's global and trusted-project settings. */ @@ -8,52 +12,134 @@ 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 }; + providers: { "1password"?: OnePasswordSettings; dotenv?: EnvFileSettings }; } export const DEFAULT_LIFETIME_MINUTES = 15; +const SettingsSchema = 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 })), + }), + ), + dotenv: Type.Optional( + Type.Object({ + enabled: Type.Optional(Type.Boolean()), + files: Type.Optional( + Type.Array( + // An exact path: absolute, or under the home directory with `~/`. + Type.String({ pattern: "^(?:~[\\\\/]|[\\\\/]|[A-Za-z]:[\\\\/])" }), + ), + ), + }), + ), + }), + ), +}); + +/** Each provider's settings replace that provider's only; listed Env Files add up. */ +function mergeProviders( + current: SecretSettings["providers"], + next: NonNullable["providers"]>, + source: LayeredSettingScope, +): SecretSettings["providers"] { + const merged = { ...current }; + if (next["1password"] !== undefined) merged["1password"] = next["1password"]; + const files = next.dotenv?.files; + if (files !== undefined) { + // Anything but global settings is held to the project, failing closed. + const scope: ListedEnvFile["scope"] = source === "global" ? "global" : "project"; + merged.dotenv = { + files: [...(current.dotenv?.files ?? []), ...files.map((path) => ({ path, scope }))], + }; + } + return merged; +} + const layered = defineLayeredSettings({ namespace: "secret", 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 })), - }), - ), - }), - ), - }), + schema: SettingsSchema, defaults: { lifetimeMinutes: DEFAULT_LIFETIME_MINUTES, providers: {} } satisfies SecretSettings, + merge: { providers: mergeProviders }, }); -/** Settings as read, with why they fell back to the defaults, if they did. */ +/** Settings as read, with why any part of them was ignored. */ export interface ReadSecretSettings { settings: SecretSettings; - warning?: string; + warnings: string[]; } -/** Effective settings, or the defaults with a warning when the authored ones are invalid. */ -export function readSecretSettings(session: { - settingsManager: SettingsManager; - sessionManager: Pick; -}): ReadSecretSettings { +/** + * Effective settings, or the defaults with a warning when the authored ones are invalid. Project + * settings may not list Env Files outside the project, nor switch the Env File provider. + */ +export function readSecretSettings( + session: { + settingsManager: SettingsManager; + sessionManager: Pick; + }, + projectDirectory: string, +): ReadSecretSettings { + const authored = layered.readLayers(session.settingsManager); + let settings: SecretSettings; try { - return { settings: layered.readSettings(session).settings }; + settings = layered.readSettings(session, authored).settings; } catch (cause) { const message = cause instanceof Error ? cause.message : String(cause); return { - // Fail closed: an unreadable vault allowlist must not widen to every vault. + // Fail closed: an unreadable vault allowlist must not widen to every vault, and an + // unreadable file list reads only the project's own Env Files. settings: { lifetimeMinutes: DEFAULT_LIFETIME_MINUTES, - providers: { "1password": { vaults: [] } }, + providers: { "1password": { vaults: [] }, dotenv: { files: [] } }, }, - warning: `${message}; using a ${DEFAULT_LIFETIME_MINUTES}-minute Secret Lifetime and no 1Password vaults until it is fixed`, + warnings: [ + `${message}; using a ${DEFAULT_LIFETIME_MINUTES}-minute Secret Lifetime, no 1Password vaults, and only the project's own Env Files until it is fixed`, + ], }; } + const warnings: string[] = []; + const project = authored.project; + if ( + session.settingsManager.isProjectTrusted() && + !(project instanceof Error) && + project.providers?.dotenv?.enabled !== undefined + ) + warnings.push( + "secret.providers.dotenv.enabled in project settings is ignored; only global settings can turn the Env File Secret Provider on or off", + ); + const dotenv = settings.providers.dotenv; + if (dotenv !== undefined) { + const outside = dotenv.files.filter( + (file) => file.scope === "project" && !isInsideProject(projectDirectory, file.path), + ); + for (const file of outside) + warnings.push( + `ignoring ${file.path} in project secret.providers.dotenv.files; only global settings may list Env Files outside the project`, + ); + settings.providers.dotenv = { files: dotenv.files.filter((file) => !outside.includes(file)) }; + } + return { settings, warnings }; +} + +/** + * Whether global settings turn the Env File Secret Provider on (`true`) or off (`false`), or + * leave it to the project directory (undefined). Read once at load, from global settings only: + * Pi tells extensions neither the session directory nor project trust before a session starts. + */ +export function globalEnvFileSwitch(cwd: string, agentDirectory: string): boolean | undefined { + const global = layered.readLayers( + SettingsManager.create(cwd, agentDirectory, { projectTrusted: false }), + ).global; + // Invalid settings list no files, so only a project Env File can turn the provider on. + if (global instanceof Error) return undefined; + const dotenv = global.providers?.dotenv; + if (dotenv?.enabled !== undefined) return dotenv.enabled; + return (dotenv?.files?.length ?? 0) > 0 ? true : undefined; } diff --git a/packages/pi-secret/test/env-file-grants.test.ts b/packages/pi-secret/test/env-file-grants.test.ts new file mode 100644 index 00000000..c5d6ed59 --- /dev/null +++ b/packages/pi-secret/test/env-file-grants.test.ts @@ -0,0 +1,352 @@ +import { execFileSync } from "node:child_process"; +import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { parseEnv } from "node:util"; +import type { JsonValue } from "@earendil-works/pi-ai"; +import { recordToolRegistrations } from "@ian-pascoe/pi-utils/tool-testing"; +import { describe, expect, it, onTestFinished } from "vitest"; +import { + createPiSecretExtension, + defaultSecretProviders, + type ProviderEnvironment, +} from "../src/pi-secret-extension.js"; +import { + createSecretHarness, + keys, + reply, + toolCalls, + type DialogScript, + type SecretHarnessOptions, +} from "./fixtures/secret-harness.js"; + +async function workspace(files: Record = {}): Promise { + const dir = await mkdtemp(join(tmpdir(), "pi-secret-env-")); + onTestFinished(async () => { + await rm(dir, { recursive: true, force: true }); + }); + for (const [path, text] of Object.entries(files)) { + await mkdir(dirname(join(dir, path)), { recursive: true }); + await writeFile(join(dir, path), text); + } + return dir; +} + +/** Pi started in `dir`, without `op` on `PATH`. */ +const startedIn = (dir: string): ProviderEnvironment => ({ + cwd: dir, + agentDirectory: dir, + onPath: () => false, +}); + +interface EnvHarnessOptions { + files?: Record; + settings?: Record; + projectSettings?: Record; + dialogs?: DialogScript[]; + git?: boolean; +} + +/** A session in a project with Env Files, through the default Secret Providers. */ +async function envHarness(options: EnvHarnessOptions = {}) { + const dir = await workspace(options.files); + if (options.git) execFileSync("git", ["init", "--quiet"], { cwd: dir, stdio: "ignore" }); + const harnessOptions: SecretHarnessOptions = { + dir, + dialogs: options.dialogs ?? [], + providers: (context) => defaultSecretProviders(context, startedIn(dir)), + }; + if (options.settings !== undefined) harnessOptions.settings = options.settings; + if (options.projectSettings !== undefined) + harnessOptions.projectSettings = options.projectSettings; + return createSecretHarness(harnessOptions); +} + +describe("Env File Secret Provider activation", () => { + async function registered(files: Record, settings?: Record) { + const dir = await workspace(files); + if (settings !== undefined) + await writeFile(join(dir, "settings.json"), JSON.stringify(settings)); + const { pi, tools } = recordToolRegistrations(); + void createPiSecretExtension({ + getAgentDirectory: () => dir, + providers: (context) => defaultSecretProviders(context, startedIn(dir)), + })(pi); + return tools; + } + const names = (tools: Awaited>) => tools.map(({ name }) => name); + const withProviders = [ + "secret_ask", + "secret_ask_grant", + "secret_list", + "secret_save", + "secret_forget", + ]; + const withoutProviders = ["secret_ask", "secret_list", "secret_forget"]; + + it("turns on with a top-level Env File and no op on PATH, and names Env files in the prompt", async () => { + const tools = await registered({ ".env": "KEY=value\n" }); + expect(names(tools)).toEqual(withProviders); + const grant = tools.find(({ name }) => name === "secret_ask_grant"); + expect(grant).toMatchObject({ + promptGuidelines: [expect.stringContaining("in a Secret Provider such as Env files")], + }); + expect(JSON.stringify(grant?.parameters)).toContain("dotenv://.env#API_KEY"); + }); + + it("stays off without an Env File, op, or settings, leaving today's tool set", async () => { + const tools = await registered({ "packages/api/.env": "NESTED=1\n", ".envrc": "x" }); + expect(names(tools)).toEqual(withoutProviders); + expect(tools.find(({ name }) => name === "secret_list")).toMatchObject({ + description: "List the Secrets this session can use and when each expires; never values.", + }); + }); + + it("follows global settings: enabled false wins over Env Files, and files or enabled true turn it on", async () => { + const dotenv = (value: JsonValue) => ({ secret: { providers: { dotenv: value } } }); + expect(names(await registered({ ".env": "K=v\n" }, dotenv({ enabled: false })))).toEqual( + withoutProviders, + ); + expect( + names(await registered({ ".env": "K=v\n" }, dotenv({ enabled: false, files: ["/x/.env"] }))), + ).toEqual(withoutProviders); + expect(names(await registered({}, dotenv({ files: ["~/.env.tool"] })))).toEqual(withProviders); + expect(names(await registered({}, dotenv({ enabled: true })))).toEqual(withProviders); + // Invalid settings list no files. + expect(names(await registered({}, dotenv({ files: ["relative/.env"] })))).toEqual( + withoutProviders, + ); + }); +}); + +describe("Env File Secret Grants", () => { + it("resolves a granted key through both reference forms, follows edits, and blocks once it is gone", async () => { + const harness = await envHarness({ + files: { ".env": "API_TOKEN=env-token-0a1b2c3d\nOTHER=${API_TOKEN}\n" }, + dialogs: [[keys.enter]], + }); + harness.responses.push( + toolCalls([ + "secret_ask_grant", + { + grants: [ + { name: "API_TOKEN", locator: "dotenv://.env#API_TOKEN" }, + { name: "LITERAL", locator: "dotenv://.env#OTHER" }, + ], + }, + "call-grant", + ]), + toolCalls(["echo", { text: "a={{secret:API_TOKEN}} b={{secret:LITERAL}}" }, "call-echo-1"]), + ); + await harness.session.prompt("Use the token."); + + expect(harness.result("call-grant")).toMatchObject({ + details: { + secrets: [ + { name: "API_TOKEN", status: "granted", locator: "dotenv://.env#API_TOKEN" }, + { name: "LITERAL", status: "granted" }, + ], + }, + }); + expect(harness.frames[0]?.join("\n")).toContain("Env files · "); + expect(harness.frames[0]?.join("\n")).toContain(" / .env / API_TOKEN"); + expect(harness.received).toEqual([ + { tool: "echo", args: { text: "a=env-token-0a1b2c3d b=${API_TOKEN}" } }, + ]); + + await writeFile(join(harness.dir, ".env"), "API_TOKEN=edited-token-4e5f6a7b\n"); + harness.responses.push( + toolCalls(["echo", { text: "$PI_SECRET_API_TOKEN" }, "call-echo-2"]), + toolCalls(["echo", { text: "{{secret:LITERAL}}" }, "call-echo-3"]), + reply("Done."), + ); + await harness.session.prompt("Again."); + + expect(process.env["PI_SECRET_API_TOKEN"]).toBe("edited-token-4e5f6a7b"); + expect(harness.resultText("call-echo-3")).toContain("could not read"); + expect(harness.resultText("call-echo-3")).toContain('.env has no key "OTHER"'); + expect(await harness.sessionFileText()).not.toContain("env-token-0a1b2c3d"); + expect(harness.requests.join("\n")).not.toContain("edited-token-4e5f6a7b"); + }); + + it("lists nested Env Files and their keys, never their values", async () => { + const harness = await envHarness({ + files: { + ".env": "ROOT=root-value-11223344\n", + "packages/api/.env": "DATABASE_URL=postgres://value-55667788\n", + "node_modules/dep/.env": "DEP=dep\n", + }, + }); + harness.responses.push( + toolCalls(["secret_list", { provider: "dotenv" }, "call-files"]), + toolCalls(["secret_list", { item: join("packages", "api", ".env") }, "call-keys"]), + reply("Done."), + ); + await harness.session.prompt("List."); + + const files = harness.resultText("call-files") ?? ""; + expect(files).toContain(`- .env (env file) in ${harness.dir.split("/").at(-1)}`); + expect(files).toContain(join("packages", "api", ".env")); + expect(files).not.toContain("node_modules"); + expect(harness.resultText("call-keys")).toContain( + `- DATABASE_URL (key) of ${join("packages", "api", ".env")}: dotenv://${join("packages", "api", ".env")}#DATABASE_URL`, + ); + const everything = JSON.stringify(harness.session.messages); + expect(everything).not.toContain("root-value-11223344"); + expect(everything).not.toContain("value-55667788"); + }); +}); + +describe("Env File settings", () => { + it("grants from a global file outside the project, but drops one listed in project settings with a warning", async () => { + const elsewhere = await workspace({ + "global/.env": "GLOBAL_TOKEN=global-token-1a2b\n", + "project/.env": "PROJECT_TOKEN=project-token-3c4d\n", + }); + const harness = await envHarness({ + files: { ".env": "LOCAL=1\n" }, + settings: { secret: { providers: { dotenv: { files: [join(elsewhere, "global/.env")] } } } }, + projectSettings: { + secret: { + providers: { dotenv: { enabled: false, files: [join(elsewhere, "project/.env")] } }, + }, + }, + dialogs: [[keys.enter]], + }); + harness.responses.push( + toolCalls(["secret_list", { vault: "outside project" }, "call-list"]), + toolCalls([ + "secret_ask_grant", + { + grants: [ + { + name: "GLOBAL_TOKEN", + locator: `dotenv://${join(elsewhere, "global/.env")}#GLOBAL_TOKEN`, + }, + { + name: "PROJECT_TOKEN", + locator: `dotenv://${join(elsewhere, "project/.env")}#PROJECT_TOKEN`, + }, + ], + }, + "call-grant", + ]), + reply("Done."), + ); + await harness.session.prompt("Grant."); + + const notices = harness.notices.join("\n"); + expect(notices).toContain("secret.providers.dotenv.enabled in project settings is ignored"); + expect(notices).toContain( + `ignoring ${join(elsewhere, "project/.env")} in project secret.providers.dotenv.files`, + ); + const listed = harness.resultText("call-list") ?? ""; + expect(listed).toContain(join(elsewhere, "global/.env")); + expect(listed).not.toContain(join(elsewhere, "project/.env")); + expect(harness.result("call-grant")).toMatchObject({ + details: { + secrets: [ + { name: "GLOBAL_TOKEN", status: "granted" }, + { name: "PROJECT_TOKEN", status: "failed" }, + ], + }, + }); + expect(harness.frames[0]?.join("\n")).toContain("outside project"); + }); + + it("keeps the project's Env Files readable when the settings are invalid, with a warning", async () => { + const elsewhere = await workspace({ ".env": "OUTSIDE=outside\n" }); + const harness = await envHarness({ + files: { ".env": "LOCAL_TOKEN=local-token-5e6f\n" }, + settings: { + secret: { + lifetimeMinutes: 0, + providers: { dotenv: { enabled: true, files: [join(elsewhere, ".env")] } }, + }, + }, + dialogs: [[keys.enter]], + }); + harness.responses.push( + toolCalls(["secret_list", { provider: "dotenv" }, "call-list"]), + toolCalls([ + "secret_ask_grant", + { grants: [{ name: "LOCAL_TOKEN", locator: "dotenv://.env#LOCAL_TOKEN" }] }, + "call-grant", + ]), + reply("Done."), + ); + await harness.session.prompt("Grant."); + + expect(harness.notices.join("\n")).toContain("only the project's own Env Files"); + expect(harness.resultText("call-list")).not.toContain(elsewhere); + expect(harness.result("call-grant")).toMatchObject({ + details: { secrets: [{ name: "LOCAL_TOKEN", status: "granted" }] }, + }); + }); +}); + +describe("secret_save into Env Files", () => { + it("appends a typed Secret after approval, warning that git does not ignore the file, and grants it with its original expiry", async () => { + const harness = await envHarness({ + files: { ".env": "EXISTING=1\n" }, + git: true, + dialogs: [[keys.paste("placeholder-0000"), keys.enter], [keys.enter]], + }); + harness.responses.push( + toolCalls(["secret_ask", { names: ["NEW_TOKEN"] }, "call-ask"]), + toolCalls([ + "secret_save", + { name: "NEW_TOKEN", locator: "dotenv://.env#NEW_TOKEN" }, + "call-save", + ]), + toolCalls(["echo", { text: "{{secret:NEW_TOKEN}}" }, "call-echo"]), + reply("Done."), + ); + const expiresAt = new Date(harness.clock.now + 15 * 60_000).toISOString(); + await harness.session.prompt("Save it."); + + expect(harness.result("call-save")).toMatchObject({ + details: { + secrets: [ + { + name: "NEW_TOKEN", + status: "saved", + locator: "dotenv://.env#NEW_TOKEN", + expires_at: expiresAt, + }, + ], + }, + }); + const dialog = harness.frames.at(-1)?.join("\n") ?? ""; + expect(dialog).toContain("Save Secret"); + expect(dialog).toContain(".env / NEW_TOKEN · new"); + expect(dialog).toContain("not ignored by git"); + expect(parseEnv(await readFile(join(harness.dir, ".env"), "utf8"))).toEqual({ + EXISTING: "1", + NEW_TOKEN: "placeholder-0000", + }); + expect(harness.received).toEqual([{ tool: "echo", args: { text: "placeholder-0000" } }]); + }); + + it("shows no git warning for an ignored Env File", async () => { + const harness = await envHarness({ + files: { ".env.local": "TOKEN=ignored-token-7a8b\n", ".gitignore": ".env.local\n" }, + git: true, + dialogs: [[keys.enter]], + }); + harness.responses.push( + toolCalls([ + "secret_ask_grant", + { grants: [{ name: "TOKEN", locator: "dotenv://.env.local#TOKEN" }] }, + "call-grant", + ]), + reply("Done."), + ); + await harness.session.prompt("Grant."); + + expect(harness.frames[0]?.join("\n")).not.toContain("not ignored by git"); + expect(harness.result("call-grant")).toMatchObject({ + details: { secrets: [{ name: "TOKEN", status: "granted" }] }, + }); + }); +}); diff --git a/packages/pi-secret/test/env-file-provider.test.ts b/packages/pi-secret/test/env-file-provider.test.ts new file mode 100644 index 00000000..42ba3fc4 --- /dev/null +++ b/packages/pi-secret/test/env-file-provider.test.ts @@ -0,0 +1,361 @@ +import { execFileSync } from "node:child_process"; +import { mkdir, mkdtemp, readFile, rm, stat, symlink, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { parseEnv } from "node:util"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + EnvFileProvider, + type EnvFileSettings, + type ListedEnvFile, +} from "../src/env-file-provider.js"; + +let root: string; +let project: string; +let outside: string; + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), "pi-secret-env-")); + project = join(root, "project"); + outside = join(root, "outside"); + await mkdir(project); + await mkdir(outside); +}); + +afterEach(async () => { + vi.unstubAllEnvs(); + await rm(root, { recursive: true, force: true }); +}); + +async function put(path: string, text: string): Promise { + await mkdir(dirname(path), { recursive: true }); + await writeFile(path, text); +} + +function provider(files: ListedEnvFile[] = [], gitIgnores?: () => Promise) { + const settings: EnvFileSettings = { files }; + return new EnvFileProvider({ + projectDirectory: () => project, + settings: () => settings, + // Outside a repository unless a test says otherwise. + gitIgnores: gitIgnores ?? (async () => undefined), + }); +} + +describe("EnvFileProvider search", () => { + it("lists nested Env Files, including gitignored ones, but none under .git or node_modules", async () => { + await put(join(project, ".env"), "ROOT_KEY=root-value-0001\n"); + await put(join(project, ".env.local"), "LOCAL_KEY=local-value-0001\n"); + await put(join(project, "packages/api/.env"), "DATABASE_URL=postgres://db\n"); + await put(join(project, ".gitignore"), ".env*\n"); + await put(join(project, ".envrc"), "export NOT_AN_ENV_FILE=1\n"); + await put(join(project, "node_modules/dep/.env"), "DEP_KEY=dep\n"); + await put(join(project, ".git/.env"), "GIT_KEY=git\n"); + + const listing = await provider().search({ limit: 20 }); + + expect(listing.entries.map((entry) => entry.kind === "item" && entry.title)).toEqual([ + ".env", + ".env.local", + join("packages", "api", ".env"), + ]); + expect(listing.entries[0]).toEqual({ + kind: "item", + provider: "dotenv", + id: ".env", + title: ".env", + vault: "project", + category: "env file", + }); + expect(JSON.stringify(listing)).not.toContain("value-0001"); + }); + + it("lists one Env File's keys with their locators, never their values", async () => { + await put(join(project, "packages/api/.env"), "DATABASE_URL=postgres://secret\nAPI_KEY=abc\n"); + const path = join("packages", "api", ".env"); + + const listing = await provider().search({ item: path, limit: 20 }); + + expect(listing.entries).toEqual([ + expect.objectContaining({ label: "API_KEY", locator: `dotenv://${path}#API_KEY` }), + { + kind: "field", + provider: "dotenv", + item: path, + vault: "project", + label: "DATABASE_URL", + type: "key", + locator: `dotenv://${path}#DATABASE_URL`, + }, + ]); + expect(JSON.stringify(listing)).not.toContain("postgres://secret"); + }); + + it("matches query against file paths and key names, and filters by vault", async () => { + await put(join(project, ".env"), "STRIPE_KEY=a\n"); + await put(join(project, "web/.env.production"), "OTHER=b\n"); + await put(join(outside, ".env"), "OUTSIDE=c\n"); + const env = provider([{ path: join(outside, ".env"), scope: "global" }]); + + const titles = async (search: { query?: string; vault?: string }) => + (await env.search({ ...search, limit: 20 })).entries.map( + (entry) => entry.kind === "item" && entry.title, + ); + + expect(await titles({ query: "stripe" })).toEqual([".env"]); + expect(await titles({ query: "production" })).toEqual([join("web", ".env.production")]); + expect(await titles({ vault: "outside project" })).toEqual([join(outside, ".env")]); + expect(await titles({ vault: "PROJECT" })).toEqual([".env", join("web", ".env.production")]); + }); + + it("reports truncation past the limit", async () => { + await put(join(project, ".env"), "A=1\n"); + await put(join(project, ".env.local"), "B=1\n"); + expect(await provider().search({ limit: 1 })).toMatchObject({ truncated: true }); + }); +}); + +describe("EnvFileProvider reads", () => { + it("reads values without expansion, the last definition winning, and again after an edit", async () => { + await put(join(project, ".env"), "TOKEN=first\nPLAIN=${HOME}/x\nTOKEN=second\n"); + const env = provider(); + + expect(await env.read("dotenv://.env#TOKEN")).toBe("second"); + expect(await env.read("dotenv://.env#PLAIN")).toBe("${HOME}/x"); + await writeFile(join(project, ".env"), "TOKEN=edited\n"); + expect(await env.read("dotenv://.env#TOKEN")).toBe("edited"); + }); + + it("fails a read when the key or file is gone", async () => { + await put(join(project, ".env"), "TOKEN=value\n"); + const env = provider(); + + await expect(env.read("dotenv://.env#MISSING")).rejects.toThrow('.env has no key "MISSING"'); + await rm(join(project, ".env")); + await expect(env.read("dotenv://.env#TOKEN")).rejects.toThrow(".env does not exist"); + }); + + it("rejects malformed locators", async () => { + const env = provider(); + for (const locator of ["dotenv://.env", "dotenv://#KEY", "op://a/b/c"]) + await expect(env.read(locator)).rejects.toThrow("is not an Env File locator"); + }); + + it("describes a locator for the approval dialog", async () => { + await put(join(project, "packages/api/.env"), "DATABASE_URL=x\n"); + const locator = "dotenv://packages/api/.env#DATABASE_URL"; + + expect(await provider().describe(locator)).toEqual({ + locator, + vault: "project", + item: join("packages", "api", ".env"), + field: "DATABASE_URL", + }); + }); +}); + +describe("EnvFileProvider path rules", () => { + it("refuses files outside the project, and non-Env Files inside it, unless they are listed", async () => { + await put(join(outside, ".env"), "OUT=outside-value\n"); + await put(join(project, "config/secrets.txt"), "IN=inside-value\n"); + await put(join(project, "node_modules/dep/.env"), "DEP=dep-value\n"); + + for (const locator of [ + "dotenv://../outside/.env#OUT", + `dotenv://${join(outside, ".env")}#OUT`, + "dotenv://config/secrets.txt#IN", + "dotenv://node_modules/dep/.env#DEP", + ]) + await expect(provider().read(locator)).rejects.toThrow( + "is not an Env File Pi Secret may use", + ); + + const listed = provider([ + { path: join(outside, ".env"), scope: "global" }, + { path: join(project, "config/secrets.txt"), scope: "project" }, + ]); + expect(await listed.read(`dotenv://${join(outside, ".env")}#OUT`)).toBe("outside-value"); + expect(await listed.read("dotenv://../outside/.env#OUT")).toBe("outside-value"); + expect(await listed.read("dotenv://config/secrets.txt#IN")).toBe("inside-value"); + }); + + it("only lets global settings list files outside the project", async () => { + await put(join(outside, ".env"), "OUT=outside-value\n"); + const env = provider([{ path: join(outside, ".env"), scope: "project" }]); + + await expect(env.read(`dotenv://${join(outside, ".env")}#OUT`)).rejects.toThrow( + "is not an Env File Pi Secret may use", + ); + expect((await env.search({ limit: 20 })).entries).toEqual([]); + }); + + it("expands ~ to the home directory", async () => { + vi.stubEnv("HOME", outside); + await put(join(outside, ".config/tool/.env"), "TOKEN=home-value\n"); + const env = provider([{ path: "~/.config/tool/.env", scope: "global" }]); + + expect(await env.read("dotenv://~/.config/tool/.env#TOKEN")).toBe("home-value"); + expect((await env.search({ limit: 20 })).entries).toEqual([ + expect.objectContaining({ title: join(outside, ".config/tool/.env") }), + ]); + }); + + it("reads a linked Env File whose target is outside the project only when the target is listed", async () => { + await put(join(outside, "real.env"), "LINKED=linked-value\n"); + await symlink(join(outside, "real.env"), join(project, ".env")); + await put(join(project, "inner.env"), "INNER=inner-value\n"); + await symlink(join(project, "inner.env"), join(project, ".env.inner")); + + await expect(provider().read("dotenv://.env#LINKED")).rejects.toThrow( + "is not an Env File Pi Secret may use", + ); + expect((await provider().search({ limit: 20 })).entries).toEqual([ + expect.objectContaining({ title: ".env.inner" }), + ]); + expect(await provider().read("dotenv://.env.inner#INNER")).toBe("inner-value"); + const listed = provider([{ path: join(outside, "real.env"), scope: "global" }]); + expect(await listed.read("dotenv://.env#LINKED")).toBe("linked-value"); + }); +}); + +describe("EnvFileProvider saves", () => { + const tricky = [ + "plain-value_0123", + "with spaces inside", + "hash # inside", + "single ' quote", + 'double " quote', + "both ' and \" quotes", + "line one\nline two", + " padded ", + "back\\slash\\n", + "${NOT_EXPANDED}", + ]; + + it.each(tricky)("appends %j so util.parseEnv reads it back exactly", async (value) => { + await put(join(project, ".env"), "EXISTING=keep # comment\nUNTERMINATED_NEWLINE=yes"); + const env = provider(); + + const locator = await env.save({ locator: "dotenv://.env#NEW_KEY", value }); + + expect(locator).toBe("dotenv://.env#NEW_KEY"); + const text = await readFile(join(project, ".env"), "utf8"); + expect(text.startsWith("EXISTING=keep # comment\nUNTERMINATED_NEWLINE=yes\n")).toBe(true); + expect(parseEnv(text)).toEqual({ + EXISTING: "keep", + UNTERMINATED_NEWLINE: "yes", + NEW_KEY: value, + }); + expect(await env.read(locator)).toBe(value); + }); + + it("refuses a key the file already defines, a category, and a value that cannot round-trip", async () => { + await put(join(project, ".env"), "TOKEN=old\n"); + const env = provider(); + + await expect(env.save({ locator: "dotenv://.env#TOKEN", value: "new" })).rejects.toThrow( + ".env already defines TOKEN", + ); + await expect( + env.save({ locator: "dotenv://.env#OTHER", value: "new", category: "Password" }), + ).rejects.toThrow("Env Files have no item categories"); + await expect(env.save({ locator: "dotenv://.env#OTHER", value: "a'b\"c`d" })).rejects.toThrow( + "cannot write this value", + ); + await expect(env.save({ locator: "dotenv://.env#1BAD", value: "new" })).rejects.toThrow( + "is not a key secret_save writes", + ); + expect(await readFile(join(project, ".env"), "utf8")).toBe("TOKEN=old\n"); + }); + + it("creates a missing Env File with owner-only permissions", async () => { + await mkdir(join(project, "packages/api"), { recursive: true }); + + const locator = await provider().save({ + locator: "dotenv://packages/api/.env.local#TOKEN", + value: "created-value", + }); + + expect(locator).toBe(`dotenv://${join("packages", "api", ".env.local")}#TOKEN`); + const path = join(project, "packages/api/.env.local"); + expect(await readFile(path, "utf8")).toBe("TOKEN=created-value\n"); + expect((await stat(path)).mode & 0o777).toBe(0o600); + }); + + it("refuses to create a file not named .env*, outside the project, or in a missing directory", async () => { + const env = provider(); + await expect( + env.save({ locator: "dotenv://secrets.txt#TOKEN", value: "value" }), + ).rejects.toThrow("creates only files named .env or .env.*"); + await expect( + env.save({ locator: `dotenv://${join(outside, ".env")}#TOKEN`, value: "value" }), + ).rejects.toThrow("is not an Env File Pi Secret may use"); + await expect( + env.save({ locator: "dotenv://missing/.env#TOKEN", value: "value" }), + ).rejects.toThrow("the directory of"); + }); + + it("saves to a listed file outside the project, naming it by its full path", async () => { + vi.stubEnv("HOME", outside); + const env = provider([{ path: "~/.env.tool", scope: "global" }]); + + const locator = await env.save({ locator: "dotenv://~/.env.tool#TOKEN", value: "value" }); + + expect(locator).toBe(`dotenv://${join(outside, ".env.tool")}#TOKEN`); + expect(await env.read(locator)).toBe("value"); + }); + + it("refuses to write through a dangling link", async () => { + await symlink(join(outside, "nowhere.env"), join(project, ".env")); + await expect( + provider().save({ locator: "dotenv://.env#TOKEN", value: "value" }), + ).rejects.toThrow("is a link to a file that does not exist"); + }); + + it("describes a save, saying when the file is new", async () => { + await put(join(project, ".env"), "OLD=1\n"); + const env = provider(); + + expect(await env.describeSave({ locator: "dotenv://.env#NEW" })).toMatchObject({ + item: ".env", + field: "NEW", + fieldType: "new key", + }); + expect(await env.describeSave({ locator: "dotenv://.env.local#NEW" })).toMatchObject({ + item: ".env.local", + fieldType: "new key, new file", + }); + }); +}); + +describe("EnvFileProvider git warning", () => { + const git = (...args: string[]) => execFileSync("git", args, { cwd: project, stdio: "ignore" }); + + it("warns about a project Env File git does not ignore, and not about an ignored one", async () => { + git("init", "--quiet"); + await put(join(project, ".gitignore"), ".env.local\n"); + await put(join(project, ".env"), "TOKEN=a\n"); + await put(join(project, ".env.local"), "TOKEN=b\n"); + const env = new EnvFileProvider({ + projectDirectory: () => project, + settings: () => ({ files: [] }), + }); + + expect(await env.describe("dotenv://.env#TOKEN")).toMatchObject({ + warning: "not ignored by git", + }); + expect(await env.describe("dotenv://.env.local#TOKEN")).not.toHaveProperty("warning"); + expect(await env.describeSave({ locator: "dotenv://.env#NEW" })).toMatchObject({ + warning: "not ignored by git", + }); + }); + + it("warns about nothing outside a git repository", async () => { + await put(join(project, ".env"), "TOKEN=a\n"); + const env = new EnvFileProvider({ + projectDirectory: () => project, + settings: () => ({ files: [] }), + }); + expect(await env.describe("dotenv://.env#TOKEN")).not.toHaveProperty("warning"); + }); +}); diff --git a/packages/pi-secret/test/fixtures/fake-provider.ts b/packages/pi-secret/test/fixtures/fake-provider.ts index c9242e41..239963d1 100644 --- a/packages/pi-secret/test/fixtures/fake-provider.ts +++ b/packages/pi-secret/test/fixtures/fake-provider.ts @@ -12,6 +12,8 @@ export class FakeProvider implements SecretProvider { readonly id = "fake"; readonly label = "Fake Vault"; readonly scheme = "fake"; + readonly locatorExample = "fake://vault/item/field"; + readonly saveExample = "fake://vault/new item/field"; /** 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. */ diff --git a/packages/pi-secret/test/fixtures/secret-harness.ts b/packages/pi-secret/test/fixtures/secret-harness.ts index c1f5d109..c8957eaa 100644 --- a/packages/pi-secret/test/fixtures/secret-harness.ts +++ b/packages/pi-secret/test/fixtures/secret-harness.ts @@ -1,4 +1,4 @@ -import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { onTestFinished } from "vitest"; @@ -28,7 +28,7 @@ import { 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 { createPiSecretExtension, type ProviderContext } 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. */ @@ -55,6 +55,8 @@ export interface SecretHarnessOptions { headless?: boolean; /** Global Pi settings written to the agent directory. */ settings?: Record; + /** Project Pi settings written to the workspace's `.pi/settings.json`. */ + projectSettings?: Record; /** Reuse a workspace (cwd and agent directory) from an earlier harness. */ dir?: string; /** Open an existing session file instead of creating one. */ @@ -64,8 +66,11 @@ 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[]; + /** + * Secret Providers, or a factory given the extension's provider context; none by default, so + * tests never reach a real 1Password CLI. + */ + providers?: SecretProvider[] | ((context: ProviderContext) => SecretProvider[]); } /** What a test tool received, after every `tool_call` handler ran. */ @@ -143,6 +148,10 @@ export async function createSecretHarness(options: SecretHarnessOptions = {}) { ...options.settings, }), ); + if (options.projectSettings !== undefined) { + await mkdir(join(dir, ".pi"), { recursive: true }); + await writeFile(join(dir, ".pi", "settings.json"), JSON.stringify(options.projectSettings)); + } // Starts at the real time: other sessions in the process judge expiry by the wall clock. const clock = options.clock ?? { now: Date.now() }; const received: ReceivedCall[] = []; @@ -164,7 +173,10 @@ export async function createSecretHarness(options: SecretHarnessOptions = {}) { createPiSecretExtension({ getAgentDirectory: () => dir, now: () => clock.now, - providers: () => options.providers ?? [], + providers: (context) => { + const providers = options.providers ?? []; + return Array.isArray(providers) ? providers : providers(context); + }, }), ], 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 7a197833..bb05f693 100644 --- a/packages/pi-secret/test/null-optional-arguments.test.ts +++ b/packages/pi-secret/test/null-optional-arguments.test.ts @@ -3,7 +3,7 @@ import { recordToolRegistrations, } from "@ian-pascoe/pi-utils/tool-testing"; import { expect, test } from "vitest"; -import { createPiSecretExtension, SECRET_PROVIDER_GUIDELINE } from "../src/pi-secret-extension.js"; +import { createPiSecretExtension, secretProviderGuideline } from "../src/pi-secret-extension.js"; import { FakeProvider } from "./fixtures/fake-provider.js"; function registeredTools(withProvider = true) { @@ -29,7 +29,9 @@ test("registers the Secret Provider tools only when a Secret Provider is availab 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] }); + const guideline = secretProviderGuideline([new FakeProvider()]); + expect(guideline).toContain("in a Secret Provider such as Fake Vault, find it with secret_list"); + expect(grant).toMatchObject({ promptGuidelines: [guideline] }); }); test.each([ From eb7ef8b2544c0bb723f8ca20110fb76edf6e52c2 Mon Sep 17 00:00:00 2001 From: Ian Pascoe Date: Tue, 20 Oct 2026 12:00:54 -0400 Subject: [PATCH 2/2] fix(secret): address review of the Env File Secret Provider - An invalid global settings file with dotenv.enabled: false keeps the provider off - Show a project link to an outside file as outside the project, and list it once - Record grants under the provider's canonical locator (~ expanded, project-relative) - Append with O_NOFOLLOW, and check a save once - Keep secret_list's description unchanged without Secret Providers --- packages/pi-secret/src/env-file-provider.ts | 79 +++++++++++++------ packages/pi-secret/src/pi-secret-extension.ts | 16 ++-- packages/pi-secret/src/secret-provider.ts | 2 +- packages/pi-secret/src/secret-settings.ts | 19 +++-- .../pi-secret/test/env-file-grants.test.ts | 20 ++++- .../pi-secret/test/env-file-provider.test.ts | 11 +++ 6 files changed, 104 insertions(+), 43 deletions(-) diff --git a/packages/pi-secret/src/env-file-provider.ts b/packages/pi-secret/src/env-file-provider.ts index fab66e60..0deaca31 100644 --- a/packages/pi-secret/src/env-file-provider.ts +++ b/packages/pi-secret/src/env-file-provider.ts @@ -1,5 +1,5 @@ import { execFile } from "node:child_process"; -import { readdirSync } from "node:fs"; +import { constants, readdirSync } from "node:fs"; import { lstat, open, readdir, readFile, realpath, stat } from "node:fs/promises"; import { homedir } from "node:os"; import { basename, dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; @@ -105,6 +105,10 @@ interface ParsedLocator { readonly key: string; } +function formatLocator(path: string, key: string): string { + return `${SCHEME}${path}#${key}`; +} + function parseLocator(locator: string): ParsedLocator { if (!locator.startsWith(SCHEME)) throw new Error(`${JSON.stringify(locator)} is not an Env File locator: ${LOCATOR_HELP}`); @@ -131,9 +135,21 @@ interface EnvFilePath { interface PermittedFile extends EnvFilePath { /** Where the file really is, links followed: the only path read or written. */ readonly real: string; + /** Whether the real path is in the project, so a link out of it counts as outside. */ + readonly inRealProject: boolean; readonly exists: boolean; } +/** A checked save: where it goes, and the file's text before it. */ +interface PlannedSave { + readonly file: PermittedFile; + readonly key: string; + readonly existing: string; +} + +/** Append, never truncate, and never through a link swapped in after the check. */ +const APPEND = constants.O_WRONLY | constants.O_APPEND | (constants.O_NOFOLLOW ?? 0); + function notFound(file: EnvFilePath, cause: unknown): Error { if (isMissing(cause)) return new Error(`${file.shown} does not exist`, { cause }); const message = cause instanceof Error ? cause.message : String(cause); @@ -193,8 +209,8 @@ export class EnvFileProvider implements SecretProvider { } /** The vault an Env File is shown in: the project directory's name, or `outside project`. */ - private vault(file: EnvFilePath): string { - return file.inProject === undefined ? "outside project" : basename(this.project()); + private vault(file: PermittedFile): string { + return file.inRealProject ? basename(this.project()) : "outside project"; } /** @@ -229,23 +245,23 @@ export class EnvFileProvider implements SecretProvider { `secret_save creates only files named .env or .env.*, not ${basename(file.absolute)}`, ); const realProject = await realpath(this.project()).catch(() => this.project()); - const inRealProject = inside(realProject, real) !== undefined; + const inRealProject = file.inProject !== undefined && inside(realProject, real) !== undefined; + const permitted: PermittedFile = { ...file, real, inRealProject, exists }; for (const listed of this.options.settings().files) { const path = resolve(expandHome(listed.path)); const listedReal = await realpath(path).catch(() => path); if (path !== file.absolute && listedReal !== real) continue; - if (listed.scope === "global") return { ...file, real, exists }; - if (inside(this.project(), path) !== undefined && inRealProject) - return { ...file, real, exists }; + if (listed.scope === "global") return permitted; + if (inside(this.project(), path) !== undefined && inside(realProject, real) !== undefined) + return permitted; } const segments = file.inProject?.split(sep) ?? []; if ( - file.inProject !== undefined && inRealProject && isEnvFileName(basename(file.absolute)) && !segments.some((segment) => SKIPPED_DIRECTORIES.has(segment)) ) - return { ...file, real, exists }; + return permitted; throw new Error( `${file.shown} is not an Env File Pi Secret may use: use a .env or .env.* file in the project, outside .git and node_modules, or list the file in global secret.providers.dotenv.files`, ); @@ -264,7 +280,7 @@ export class EnvFileProvider implements SecretProvider { file: PermittedFile, signal?: AbortSignal, ): Promise<{ warning?: string }> { - if (file.inProject === undefined) return {}; + if (!file.inRealProject) return {}; return (await this.checkIgnored(file.absolute, signal)) === false ? { warning: "not ignored by git" } : {}; @@ -276,7 +292,8 @@ export class EnvFileProvider implements SecretProvider { if (!Object.hasOwn(await this.keys(file, signal), key)) throw new Error(`${file.shown} has no key ${JSON.stringify(key)}`); return { - locator, + // The full path for `~`, and relative in the project, so a grant follows the project. + locator: formatLocator(file.shown, key), vault: this.vault(file), item: file.shown, field: key, @@ -284,10 +301,11 @@ export class EnvFileProvider implements SecretProvider { }; } - async describeSave( + /** Check a save: a new key, in a permitted file that exists or may be created. */ + private async planSave( request: Omit, signal?: AbortSignal, - ): Promise { + ): Promise { if (request.category !== undefined) throw new Error("Env Files have no item categories; call secret_save without category"); const { path, key } = parseLocator(request.locator); @@ -296,12 +314,21 @@ export class EnvFileProvider implements SecretProvider { `${JSON.stringify(key)} is not a key secret_save writes: use letters, digits, and underscores, not starting with a digit`, ); const file = await this.permit(this.path(path), true); - if (Object.hasOwn(await this.keys(file, signal), key)) + const existing = file.exists ? await readFile(file.real, { encoding: "utf8", signal }) : ""; + if (Object.hasOwn(parseEnv(existing), key)) throw new Error( `${file.shown} already defines ${key}; secret_save only adds keys, so choose another key or file`, ); + return { file, key, existing }; + } + + async describeSave( + request: Omit, + signal?: AbortSignal, + ): Promise { + const { file, key } = await this.planSave(request, signal); return { - locator: request.locator, + locator: formatLocator(file.shown, key), vault: this.vault(file), item: file.shown, field: key, @@ -322,19 +349,17 @@ export class EnvFileProvider implements SecretProvider { } async save(request: SaveRequest, signal?: AbortSignal): Promise { - await this.describeSave(request, signal); - const { path, key } = parseLocator(request.locator); - const file = await this.permit(this.path(path), true); - const existing = file.exists ? await readFile(file.real, { encoding: "utf8", signal }) : ""; + const { file, key, existing } = await this.planSave(request, signal); const line = appendedLine(existing, key, request.value); // Appended, never rewritten; a new file is created exclusively, readable by its owner only. - const handle = await open(file.real, file.exists ? "a" : "ax", 0o600); + const flags = file.exists ? APPEND : APPEND | constants.O_CREAT | constants.O_EXCL; + const handle = await open(file.real, flags, 0o600); try { await handle.appendFile(line, "utf8"); } finally { await handle.close(); } - return `${SCHEME}${file.shown}#${key}`; + return formatLocator(file.shown, key); } /** Every Env File in the project, outside skipped directories, without following links. */ @@ -367,15 +392,17 @@ export class EnvFileProvider implements SecretProvider { ...(await this.projectFiles(signal)), ...this.options.settings().files.map((listed) => listed.path), ]; + // By real path, so a project link to a listed file is listed once, under the link. const files = new Map(); for (const candidate of candidates) { - const path = this.path(candidate); - if (files.has(path.absolute)) continue; + let file: PermittedFile; try { - files.set(path.absolute, await this.permit(path)); + file = await this.permit(this.path(candidate)); } catch { // Missing, or not permitted: not listed. + continue; } + if (!files.has(file.real)) files.set(file.real, file); } return [...files.values()]; } @@ -383,7 +410,7 @@ export class EnvFileProvider implements SecretProvider { async search(search: ProviderSearch, signal?: AbortSignal): Promise { const query = search.query?.toLowerCase(); const matches = (text: string) => query === undefined || text.toLowerCase().includes(query); - const inVault = (file: EnvFilePath) => + const inVault = (file: PermittedFile) => search.vault === undefined || this.vault(file).toLowerCase() === search.vault.toLowerCase(); const entries: AvailableEntry[] = []; if (search.item !== undefined) { @@ -399,7 +426,7 @@ export class EnvFileProvider implements SecretProvider { vault: this.vault(file), label: key, type: "key", - locator: `${SCHEME}${file.shown}#${key}`, + locator: formatLocator(file.shown, key), }); } else { for (const file of await this.files(signal)) { diff --git a/packages/pi-secret/src/pi-secret-extension.ts b/packages/pi-secret/src/pi-secret-extension.ts index a0b9e101..2150cb77 100644 --- a/packages/pi-secret/src/pi-secret-extension.ts +++ b/packages/pi-secret/src/pi-secret-extension.ts @@ -972,7 +972,7 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio "The user cancelled the Grant Request. Do not ask again unless they want to.", ); } - for (const [row, { index, provider }] of pending.entries()) { + for (const [row, { index, provider, description }] of pending.entries()) { const outcome = outcomes[index]; const grant = params.grants[index]; if (outcome === undefined || grant === undefined) continue; @@ -983,16 +983,19 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio 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); + value = await provider.read(description.locator, signal); } catch (cause) { outcome.error = errorMessage(cause); continue; } + // The provider's own form of the locator, such as a full path for `~`. + const { locator } = description; const expiresAt = now() + lifetimeMinutes * 60_000; - recordGrant(grant.name, grant.locator, expiresAt); + recordGrant(grant.name, locator, expiresAt); await discard(grant.name); - adoptGrant(grant.name, grant.locator, provider, expiresAt, value); + adoptGrant(grant.name, locator, provider, expiresAt, value); Object.assign(outcome, { + locator, status: "granted", reference: secretReference(grant.name), env: secretEnvName(grant.name), @@ -1009,9 +1012,8 @@ export function createPiSecretExtension(options: PiSecretOptions = {}): Extensio name: "secret_list", label: "Secret List", description: - providers.length === 0 - ? "List the Secrets this session can use and when each expires; never values." - : `List the Secrets this session can use and when each expires, and search Secret Providers such as ${providerNames(providers)} for items and fields to request with secret_ask_grant; never values.`, + // Without a provider, today's text, so that tool set stays exactly as it was. + `List the Secrets this session can use and when each expires, and search Secret Providers such as ${providers.length === 0 ? "1Password" : providerNames(providers)} for items and fields to request with secret_ask_grant; never values.`, parameters: ListParameters, annotations: { readOnlyHint: true, diff --git a/packages/pi-secret/src/secret-provider.ts b/packages/pi-secret/src/secret-provider.ts index e7d2867d..a229cbfe 100644 --- a/packages/pi-secret/src/secret-provider.ts +++ b/packages/pi-secret/src/secret-provider.ts @@ -6,7 +6,7 @@ /** 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. */ + /** The Secret Locator, in the form a grant records it, such as with `~` expanded. */ readonly locator: string; readonly vault: string; readonly item: string; diff --git a/packages/pi-secret/src/secret-settings.ts b/packages/pi-secret/src/secret-settings.ts index 17e43a40..756772c1 100644 --- a/packages/pi-secret/src/secret-settings.ts +++ b/packages/pi-secret/src/secret-settings.ts @@ -4,6 +4,7 @@ import { type LayeredSettingScope, } from "@ian-pascoe/pi-utils/layered-settings"; import { Type, type Static } from "typebox"; +import { Value } from "typebox/value"; import { isInsideProject, type EnvFileSettings, type ListedEnvFile } from "./env-file-provider.js"; import type { OnePasswordSettings } from "./one-password-provider.js"; @@ -128,17 +129,25 @@ export function readSecretSettings( return { settings, warnings }; } +/** A raw settings document with `secret.providers.dotenv.enabled: false`, valid or not otherwise. */ +const DisabledSchema = Type.Object({ + secret: Type.Object({ + providers: Type.Object({ dotenv: Type.Object({ enabled: Type.Literal(false) }) }), + }), +}); + /** * Whether global settings turn the Env File Secret Provider on (`true`) or off (`false`), or * leave it to the project directory (undefined). Read once at load, from global settings only: * Pi tells extensions neither the session directory nor project trust before a session starts. */ export function globalEnvFileSwitch(cwd: string, agentDirectory: string): boolean | undefined { - const global = layered.readLayers( - SettingsManager.create(cwd, agentDirectory, { projectTrusted: false }), - ).global; - // Invalid settings list no files, so only a project Env File can turn the provider on. - if (global instanceof Error) return undefined; + const manager = SettingsManager.create(cwd, agentDirectory, { projectTrusted: false }); + const global = layered.readLayers(manager).global; + // Invalid settings list no files, so only a project Env File can turn the provider on, and + // an `enabled: false` among them still turns it off: an unrelated typo must not re-enable it. + if (global instanceof Error) + return Value.Check(DisabledSchema, manager.getGlobalSettings()) ? false : undefined; const dotenv = global.providers?.dotenv; if (dotenv?.enabled !== undefined) return dotenv.enabled; return (dotenv?.files?.length ?? 0) > 0 ? true : undefined; diff --git a/packages/pi-secret/test/env-file-grants.test.ts b/packages/pi-secret/test/env-file-grants.test.ts index c5d6ed59..63d03158 100644 --- a/packages/pi-secret/test/env-file-grants.test.ts +++ b/packages/pi-secret/test/env-file-grants.test.ts @@ -98,7 +98,8 @@ describe("Env File Secret Provider activation", () => { const tools = await registered({ "packages/api/.env": "NESTED=1\n", ".envrc": "x" }); expect(names(tools)).toEqual(withoutProviders); expect(tools.find(({ name }) => name === "secret_list")).toMatchObject({ - description: "List the Secrets this session can use and when each expires; never values.", + description: + "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.", }); }); @@ -112,10 +113,18 @@ describe("Env File Secret Provider activation", () => { ).toEqual(withoutProviders); expect(names(await registered({}, dotenv({ files: ["~/.env.tool"] })))).toEqual(withProviders); expect(names(await registered({}, dotenv({ enabled: true })))).toEqual(withProviders); - // Invalid settings list no files. + // Invalid settings list no files, and an `enabled: false` among them still turns it off. expect(names(await registered({}, dotenv({ files: ["relative/.env"] })))).toEqual( withoutProviders, ); + expect( + names( + await registered( + { ".env": "K=v\n" }, + { secret: { lifetimeMinutes: 0, providers: { dotenv: { enabled: false } } } }, + ), + ), + ).toEqual(withoutProviders); }); }); @@ -131,7 +140,7 @@ describe("Env File Secret Grants", () => { { grants: [ { name: "API_TOKEN", locator: "dotenv://.env#API_TOKEN" }, - { name: "LITERAL", locator: "dotenv://.env#OTHER" }, + { name: "LITERAL", locator: `dotenv://${join(harness.dir, ".env")}#OTHER` }, ], }, "call-grant", @@ -144,11 +153,14 @@ describe("Env File Secret Grants", () => { details: { secrets: [ { name: "API_TOKEN", status: "granted", locator: "dotenv://.env#API_TOKEN" }, - { name: "LITERAL", status: "granted" }, + // Recorded relative to the project, so the grant follows it. + { name: "LITERAL", status: "granted", locator: "dotenv://.env#OTHER" }, ], }, }); expect(harness.frames[0]?.join("\n")).toContain("Env files · "); + // Outside a git repository, no git warning. + expect(harness.frames[0]?.join("\n")).not.toContain("not ignored by git"); expect(harness.frames[0]?.join("\n")).toContain(" / .env / API_TOKEN"); expect(harness.received).toEqual([ { tool: "echo", args: { text: "a=env-token-0a1b2c3d b=${API_TOKEN}" } }, diff --git a/packages/pi-secret/test/env-file-provider.test.ts b/packages/pi-secret/test/env-file-provider.test.ts index 42ba3fc4..79e2e45b 100644 --- a/packages/pi-secret/test/env-file-provider.test.ts +++ b/packages/pi-secret/test/env-file-provider.test.ts @@ -195,6 +195,9 @@ describe("EnvFileProvider path rules", () => { const env = provider([{ path: "~/.config/tool/.env", scope: "global" }]); expect(await env.read("dotenv://~/.config/tool/.env#TOKEN")).toBe("home-value"); + expect(await env.describe("dotenv://~/.config/tool/.env#TOKEN")).toMatchObject({ + locator: `dotenv://${join(outside, ".config/tool/.env")}#TOKEN`, + }); expect((await env.search({ limit: 20 })).entries).toEqual([ expect.objectContaining({ title: join(outside, ".config/tool/.env") }), ]); @@ -215,6 +218,14 @@ describe("EnvFileProvider path rules", () => { expect(await provider().read("dotenv://.env.inner#INNER")).toBe("inner-value"); const listed = provider([{ path: join(outside, "real.env"), scope: "global" }]); expect(await listed.read("dotenv://.env#LINKED")).toBe("linked-value"); + // Shown as outside the project, where the value really is, and listed once. + expect(await listed.describe("dotenv://.env#LINKED")).toMatchObject({ + vault: "outside project", + }); + const titles = (await listed.search({ limit: 20 })).entries.map( + (entry) => entry.kind === "item" && entry.title, + ); + expect(titles).toEqual([".env", ".env.inner"]); }); });