diff --git a/config/knip.config.ts b/config/knip.config.ts index f15a95642ec8..631526dcd4bc 100644 --- a/config/knip.config.ts +++ b/config/knip.config.ts @@ -365,6 +365,11 @@ const config = { // Focused tests consume these diagnostic/test seams; production code uses // the surrounding runtime helpers rather than importing the exports. "extensions/signal/src/setup-core.ts": ["exports"], + // The resolver's executable-path validation is covered through focused tests; + // production imports only the narrower op resolver. + "extensions/onepassword/onepassword-op-path.js": ["exports"], + // Focused CLI tests exercise plan construction through this explicit test seam. + "extensions/onepassword/src/secret-ref-cli.ts": ["exports"], "src/infra/heartbeat-wake.ts": ["exports"], }, workspaces: { @@ -706,6 +711,10 @@ const config = { "tts.ts!", "usage.ts!", ]), + [`${BUNDLED_PLUGIN_ROOT_DIR}/onepassword`]: bundledPluginWorkspace([ + // Shipped resolver child process declared as a static plugin artifact. + "onepassword-secret-ref-resolver.js!", + ]), [`${BUNDLED_PLUGIN_ROOT_DIR}/opencode`]: bundledPluginWorkspace([ // Session catalog and provider helpers are plugin-owned runtime surfaces. "media-understanding-provider.ts!", diff --git a/docs/.generated/plugin-sdk-api-baseline.sha256 b/docs/.generated/plugin-sdk-api-baseline.sha256 index 970711f63a05..4d3cb1813032 100644 --- a/docs/.generated/plugin-sdk-api-baseline.sha256 +++ b/docs/.generated/plugin-sdk-api-baseline.sha256 @@ -115,7 +115,7 @@ b6b8edc50ecab8386c9acd8f374a207212b5a99c8f518538bbcf0c458dda3881 module/runtime 0d8f2c5f3a3325d7d190d2c395835f58ac57f1bea9b6fa516c6942b96d9bf605 module/runtime-store 44adc2205f926172fcd3762ca8a96c1485beabcb1bef8b9acfd2233cefea2a6a module/secret-input 57dcb1462d4c4f9a98d934c4ca975b163d704758af9821a64001ff3ac05637c3 module/secret-input-runtime -17a6a199714ba8308e62928c0491bcf9fd214c923c3879aa99a047a30138253d module/secret-ref-runtime +193c492b30aee96be362adeefddfc5b22d79cb0aee1d323d2102684398327986 module/secret-ref-runtime 596a315d426121c9620b314e3a9a7f523840b46e007d94d0d5e83cdedf789d15 module/security-runtime 31b785e74f1f8f56241b7756ef6a5d86199c5ce177cbb1c234a261866972f270 module/session-discussion 32fb6d253abf22440bc76c7a68d1f35fc0ef369b0ad738aedba9c3054a76e48e module/session-store-runtime diff --git a/docs/docs_map.md b/docs/docs_map.md index df8de6de2dd8..dc4f8fc158dc 100644 --- a/docs/docs_map.md +++ b/docs/docs_map.md @@ -3161,8 +3161,8 @@ Do not edit it by hand; run `pnpm docs:map:gen`. - H2: Requirements - H2: Resolve config secrets with the plugin - H2: The 1password skill for agents - - H2: Browser sign-in with 1Password for Claude - H2: Official 1Password MCP server + - H2: Browser sign-in with 1Password for Claude - H2: Security notes - H2: Troubleshooting diff --git a/docs/gateway/1password.md b/docs/gateway/1password.md index eb2ed07aa041..5efc1a7ebc1d 100644 --- a/docs/gateway/1password.md +++ b/docs/gateway/1password.md @@ -45,12 +45,18 @@ openclaw onepassword secretref setup \ --anthropic-id op://Automation/Anthropic/credential \ --plan-out ./openclaw-1password-secrets-plan.json +openclaw onepassword secretref status openclaw secrets apply --from ./openclaw-1password-secrets-plan.json --dry-run --allow-exec openclaw secrets apply --from ./openclaw-1password-secrets-plan.json --allow-exec openclaw secrets audit --check --allow-exec openclaw secrets reload ``` +The setup command requires at least one target. Before the plan is applied, +status may report that the provider is not configured while still reporting +`prerequisites ready: yes`; after apply, `ready: yes` confirms the provider, +trusted `op` executable, and accepted non-empty token file are all ready. + The plugin accepts native `op:////` and `op:////
/` references. It resolves only @@ -112,8 +118,11 @@ One-time passcodes are filled by 1Password on the same page; never relay verific desktop approval or macOS permission dialogs. - Before passing the service-account token, the plugin resolves the `op` executable and rejects paths that are writable by another local account or - have unverifiable Windows ACLs. An absolute `CLAW_1PASSWORD_OP` override is - subject to the same check. + have unverifiable Windows ACLs or ownership. An absolute + `CLAW_1PASSWORD_OP` override is subject to the same check. +- A resolver request is limited to 32 references. Reads run four at a time with + a seven-second per-read timeout; the provider-wide 90-second timeout covers + the full supported batch plus process and permission-check overhead. - Never place secret values in `openclaw.json`, logs, or chat. Scope the service account to only the vaults and items OpenClaw needs. diff --git a/docs/plugins/onepassword.md b/docs/plugins/onepassword.md index 7d5f4c42e732..76c1e588bc5b 100644 --- a/docs/plugins/onepassword.md +++ b/docs/plugins/onepassword.md @@ -91,15 +91,23 @@ openclaw onepassword secretref setup \ Use `--provider-key ` for another model provider, or `--target ` for any registered [SecretRef credential target](/reference/secretref-credential-surface). -The command writes a plan; inspect it, then apply and reload: +The command requires at least one target and writes a plan. Inspect it, check +the local `op` and token-file prerequisites, then apply and reload: ```bash +openclaw onepassword secretref status openclaw secrets apply --from ./openclaw-1password-secrets-plan.json --dry-run --allow-exec openclaw secrets apply --from ./openclaw-1password-secrets-plan.json --allow-exec openclaw secrets audit --check --allow-exec openclaw secrets reload ``` +Before apply, status can report that the provider itself is not configured yet; +`prerequisites ready: yes` confirms that the trusted `op` executable and an +accepted non-empty token file are ready. After apply, `ready: yes` confirms both the +provider wiring and prerequisites. Missing or unsafe prerequisites produce +actionable next steps without printing the token or raw resolver errors. + Manual provider configuration uses the existing plugin id: ```json5 @@ -141,12 +149,13 @@ OpenClaw's shared exec-id grammar in a plugin-local opaque form and decodes them only inside the resolver. Very long references should use stable 1Password IDs; they are shorter and reduce the number of 1Password API requests. -The SecretRef resolver accepts at most 16 IDs per request, runs at most four -`op read` processes concurrently, never uses desktop-app integration, and does -not expose an agent tool for arbitrary reads. Before passing the service-account -token, both plugin surfaces resolve the executable and reject paths that another -local account can replace; Windows ACL verification must also succeed. Check its -provider wiring with: +The SecretRef resolver runs at most four `op read` processes concurrently, +disables the 1Password CLI cache so reloads observe rotated values, never uses +desktop-app integration, and does not expose an agent tool for arbitrary reads. +Before passing the service-account token, both plugin surfaces +resolve the executable and reject paths that another local account can replace; +Windows ACL verification must also succeed. Check provider wiring and local +readiness with: ```bash openclaw onepassword secretref status --json diff --git a/docs/plugins/sdk-subpaths.md b/docs/plugins/sdk-subpaths.md index 952d77302dfd..6c08bbf0cacf 100644 --- a/docs/plugins/sdk-subpaths.md +++ b/docs/plugins/sdk-subpaths.md @@ -199,7 +199,7 @@ usage endpoint failed or returned no usable usage data. | `plugin-sdk/channel-secret-runtime` | Deprecated broad secret-contract surface (`collectSimpleChannelFieldAssignments`, `getChannelSurface`, `pushAssignment`, secret target types); prefer the focused subpaths below | | `plugin-sdk/channel-secret-basic-runtime` | Narrow secret-contract exports and target-registry builders for non-TTS channel/plugin secret surfaces | | `plugin-sdk/channel-secret-tts-runtime` | Private-local after July 2026; Narrow nested channel TTS secret assignment helpers | - | `plugin-sdk/secret-ref-runtime` | Narrow SecretRef typing, resolution, and plan-target path lookup for secret-contract/config parsing | + | `plugin-sdk/secret-ref-runtime` | Narrow SecretRef typing, resolution, and shared setup-plan construction for plugin-owned secret providers | | `plugin-sdk/security-runtime` | Deprecated broad barrel for trust, DM gating, root-bounded file/path helpers including create-only writes, sync/async atomic file replacement, sibling temp writes, cross-device move fallback, private file-store helpers, symlink-parent guards, external-content, sensitive text redaction, constant-time secret comparison, and secret-collection helpers; prefer focused security/SSRF/secret subpaths | | `plugin-sdk/ssrf-policy` | Host allowlist and private-network SSRF policy helpers | | `plugin-sdk/ssrf-dispatcher` | Private-local after July 2026; Narrow pinned-dispatcher helpers without the broad infra runtime surface | diff --git a/extensions/onepassword/index.ts b/extensions/onepassword/index.ts index 2916ed70cb72..905da53c28d4 100644 --- a/extensions/onepassword/index.ts +++ b/extensions/onepassword/index.ts @@ -102,7 +102,7 @@ export default definePluginEntry({ resolveOpClient: resolveCurrentOpClient, auditStore: audit, registerAdditionalCommands: (command) => - registerOnePasswordSecretRefCommands({ command, config }), + registerOnePasswordSecretRefCommands({ command, config, tokenFile }), }); }, { diff --git a/extensions/onepassword/onepassword-op-path.d.ts b/extensions/onepassword/onepassword-op-path.d.ts index 9c45a98d7d44..5ef5c31c5b97 100644 --- a/extensions/onepassword/onepassword-op-path.d.ts +++ b/extensions/onepassword/onepassword-op-path.d.ts @@ -1,5 +1,9 @@ export function resolveTrustedExecutablePath(targetPath: string): Promise; +export function resolveTrustedWindowsSystemExecutablePath(targetPath: string): Promise; + +export function resolveTrustedOnePasswordDirectoryPath(targetPath: string): Promise; + export function resolveTrustedOnePasswordCli(options?: { configuredPath?: string; pathEnv?: string; diff --git a/extensions/onepassword/onepassword-op-path.js b/extensions/onepassword/onepassword-op-path.js index 6a471da596fb..3926aee0ba53 100644 --- a/extensions/onepassword/onepassword-op-path.js +++ b/extensions/onepassword/onepassword-op-path.js @@ -124,16 +124,7 @@ async function readShebangInterpreter(targetPath) { } } -async function assertTrustedPath(targetPath, validatedScripts = new Set()) { - const resolvedPath = await fs.realpath(targetPath); - const targetStat = await fs.stat(resolvedPath); - if (!targetStat.isFile()) { - throw new Error(`path is not a regular file: ${resolvedPath}`); - } - await fs.access(resolvedPath, fsSync.constants.X_OK); - - // The CLI receives the service-account token. Validate its resolved parent chain so another - // local account cannot replace the executable between discovery and a later secret read. +async function assertTrustedPathChain(resolvedPath, targetType, options = {}) { const validatedEntries = []; let currentPath = resolvedPath; let first = true; @@ -157,12 +148,14 @@ async function assertTrustedPath(targetPath, validatedScripts = new Set()) { ) { throw new Error(`path changed during permission verification: ${currentPath}`); } - if ((first && stat.isDir) || (!first && !stat.isDir)) { + const expectedDirectory = !first || targetType === "directory"; + if (stat.isDir !== expectedDirectory) { throw new Error(`unexpected path type: ${currentPath}`); } - // TrustedInstaller legitimately owns Windows system directories. The requested executable - // itself still needs fs-safe's local-owner verdict. - const allowWindowsTrustedInstaller = !first; + // TrustedInstaller legitimately owns Windows system paths. Targets remain strict unless a + // caller validates a pinned system executable through the dedicated resolver below. + const allowWindowsTrustedInstaller = + !first || (first && options.allowWindowsTargetTrustedInstaller === true); if (!isTrustedOwner(stat, permissions, process.platform, allowWindowsTrustedInstaller)) { throw new Error(`path is not owned by the current user or root: ${currentPath}`); } @@ -174,7 +167,7 @@ async function assertTrustedPath(targetPath, validatedScripts = new Set()) { stat.isDir && isSafeWindowsDirectoryAclSummary( permissions.aclSummary, - currentPath !== path.dirname(resolvedPath), + targetType === "directory" || currentPath !== path.dirname(resolvedPath), ); // Windows directories commonly allow adding new children or carry inherit-only full // control for each child's eventual owner. Higher ancestors cannot replace the checked @@ -200,6 +193,19 @@ async function assertTrustedPath(targetPath, validatedScripts = new Set()) { throw new Error(`path changed after permission verification: ${entry.path}`); } } +} + +async function assertTrustedPath(targetPath, validatedScripts = new Set(), options = {}) { + const resolvedPath = await fs.realpath(targetPath); + const targetStat = await fs.stat(resolvedPath); + if (!targetStat.isFile()) { + throw new Error(`path is not a regular file: ${resolvedPath}`); + } + await fs.access(resolvedPath, fsSync.constants.X_OK); + + // The CLI receives the service-account token. Validate its resolved parent chain so another + // local account cannot replace the executable between discovery and a later secret read. + await assertTrustedPathChain(resolvedPath, "file", options); if (process.platform === "win32" && path.extname(resolvedPath).toLowerCase() !== ".exe") { throw new Error(`Windows executable must be an .exe file: ${resolvedPath}`); } @@ -228,6 +234,24 @@ export async function resolveTrustedExecutablePath(targetPath) { return await assertTrustedPath(targetPath); } +export async function resolveTrustedWindowsSystemExecutablePath(targetPath) { + if (!path.isAbsolute(targetPath)) { + throw new Error(`Executable path must be absolute: ${targetPath}`); + } + return await assertTrustedPath(targetPath, new Set(), { + allowWindowsTargetTrustedInstaller: true, + }); +} + +export async function resolveTrustedOnePasswordDirectoryPath(targetPath) { + if (!path.isAbsolute(targetPath)) { + throw new Error(`Directory path must be absolute: ${targetPath}`); + } + const resolvedPath = await fs.realpath(targetPath); + await assertTrustedPathChain(resolvedPath, "directory"); + return resolvedPath; +} + export async function resolveTrustedOnePasswordCli(options = {}) { const configuredPath = options.configuredPath?.trim(); if (configuredPath && !path.isAbsolute(configuredPath)) { diff --git a/extensions/onepassword/onepassword-secret-id.d.ts b/extensions/onepassword/onepassword-secret-id.d.ts index effcc8b17343..969ead5eb877 100644 --- a/extensions/onepassword/onepassword-secret-id.d.ts +++ b/extensions/onepassword/onepassword-secret-id.d.ts @@ -1,3 +1,2 @@ export function encodeOnePasswordSecretId(value: string): string; -export function decodeOnePasswordSecretId(value: string): string; export function resolveOnePasswordSecretReference(value: string): string; diff --git a/extensions/onepassword/onepassword-secret-id.js b/extensions/onepassword/onepassword-secret-id.js index ee4f89e94ac0..0c1128e38d55 100644 --- a/extensions/onepassword/onepassword-secret-id.js +++ b/extensions/onepassword/onepassword-secret-id.js @@ -63,7 +63,7 @@ export function encodeOnePasswordSecretId(value) { return encoded; } -export function decodeOnePasswordSecretId(value) { +function decodeOnePasswordSecretId(value) { if (!value.startsWith(ENCODED_SECRET_ID_PREFIX)) { return value; } diff --git a/extensions/onepassword/onepassword-secret-ref-resolver.js b/extensions/onepassword/onepassword-secret-ref-resolver.js index cbc1a7c5c52e..895081fc30f9 100644 --- a/extensions/onepassword/onepassword-secret-ref-resolver.js +++ b/extensions/onepassword/onepassword-secret-ref-resolver.js @@ -3,7 +3,7 @@ import fsSync from "node:fs"; import os from "node:os"; import path from "node:path"; -import { tryReadSecretFileSync } from "@openclaw/fs-safe/secret"; +import { DEFAULT_SECRET_FILE_MAX_BYTES, tryReadSecretFileSync } from "@openclaw/fs-safe/secret"; import { execa } from "execa"; import { resolveTrustedOnePasswordCli } from "./onepassword-op-path.js"; import { resolveOnePasswordSecretReference } from "./onepassword-secret-id.js"; @@ -12,7 +12,6 @@ const OP_READ_CONCURRENCY = 4; const OP_READ_TIMEOUT_MS = 7_000; const MAX_SECRET_REFS_PER_REQUEST = 32; const MAX_SECRET_VALUE_BYTES = 64 * 1024; -const MAX_TOKEN_BYTES = 16 * 1024; function readStdin() { return new Promise((resolve, reject) => { @@ -131,7 +130,7 @@ function readServiceAccountToken() { ); try { const token = tryReadSecretFileSync(tokenFile, "1Password service account token", { - maxBytes: MAX_TOKEN_BYTES, + maxBytes: DEFAULT_SECRET_FILE_MAX_BYTES, rejectHardlinks: false, rejectSymlink: true, }); diff --git a/extensions/onepassword/openclaw.plugin.json b/extensions/onepassword/openclaw.plugin.json index c78da5a69268..eecc3ee30afe 100644 --- a/extensions/onepassword/openclaw.plugin.json +++ b/extensions/onepassword/openclaw.plugin.json @@ -26,7 +26,7 @@ "args": ["./onepassword-secret-ref-resolver.js"], "timeoutMs": 90000, "noOutputTimeoutMs": 90000, - "maxOutputBytes": 8388608, + "maxOutputBytes": 16777216, "passEnv": [ "HOME", "USERPROFILE", diff --git a/extensions/onepassword/src/private-plan-file.ts b/extensions/onepassword/src/private-plan-file.ts new file mode 100644 index 000000000000..072dd525a873 --- /dev/null +++ b/extensions/onepassword/src/private-plan-file.ts @@ -0,0 +1,327 @@ +import { randomUUID } from "node:crypto"; +import path from "node:path"; +import { runExec } from "openclaw/plugin-sdk/process-runtime"; +import { + resolveTrustedExecutablePath, + resolveTrustedOnePasswordDirectoryPath, + resolveTrustedWindowsSystemExecutablePath, +} from "../onepassword-op-path.js"; + +type WindowsPrivatePlanFileDependencies = { + resolveCompilerTempDir?: (env: NodeJS.ProcessEnv) => Promise; + resolveTrustedExecutable?: typeof resolveTrustedExecutablePath; + run?: typeof runExec; +}; + +const WINDOWS_PLAN_FILE_EXISTS_MARKER = "ONEPASSWORD_PRIVATE_PLAN_FILE_EXISTS"; +const WINDOWS_PRIVATE_PLAN_FILE_NATIVE_SOURCE = ` +using System; +using System.Runtime.InteropServices; +using Microsoft.Win32.SafeHandles; + +public sealed class OpenClawPrivatePlanFile : IDisposable +{ + [StructLayout(LayoutKind.Sequential)] + private struct SecurityAttributes + { + public int Length; + public IntPtr SecurityDescriptor; + public int InheritHandle; + } + + [DllImport("advapi32.dll", CharSet = CharSet.Unicode, SetLastError = true)] + private static extern bool ConvertStringSecurityDescriptorToSecurityDescriptorW( + string securityDescriptor, + uint revision, + out IntPtr convertedSecurityDescriptor, + out uint convertedSecurityDescriptorSize); + + [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)] + private static extern SafeFileHandle CreateFileW( + string fileName, + uint desiredAccess, + uint shareMode, + ref SecurityAttributes securityAttributes, + uint creationDisposition, + uint flagsAndAttributes, + IntPtr templateFile); + + [DllImport("kernel32.dll")] + private static extern IntPtr LocalFree(IntPtr memory); + + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool WriteFile( + SafeFileHandle file, + byte[] buffer, + uint bytesToWrite, + out uint bytesWritten, + IntPtr overlapped); + + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool FlushFileBuffers(SafeFileHandle file); + + [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)] + private static extern bool MoveFileExW( + string existingFileName, + string newFileName, + uint flags); + + [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)] + private static extern bool DeleteFileW(string fileName); + + [StructLayout(LayoutKind.Sequential)] + private struct FileDispositionInfo + { + [MarshalAs(UnmanagedType.Bool)] + public bool DeleteFile; + } + + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool SetFileInformationByHandle( + SafeFileHandle file, + int fileInformationClass, + ref FileDispositionInfo fileInformation, + uint bufferSize); + + private readonly string stagingPath; + private readonly string finalPath; + private SafeFileHandle handle; + + private OpenClawPrivatePlanFile( + string stagingPath, + string finalPath, + SafeFileHandle handle) + { + this.stagingPath = stagingPath; + this.finalPath = finalPath; + this.handle = handle; + } + + private static bool SetDeleteOnClose(SafeFileHandle handle, bool enabled) + { + var disposition = new FileDispositionInfo { DeleteFile = enabled }; + return SetFileInformationByHandle( + handle, + 4, + ref disposition, + (uint)Marshal.SizeOf(typeof(FileDispositionInfo))); + } + + public static OpenClawPrivatePlanFile Open( + string stagingPath, + string finalPath, + string securityDescriptor, + out int errorCode) + { + errorCode = 0; + IntPtr descriptor; + uint descriptorSize; + if (!ConvertStringSecurityDescriptorToSecurityDescriptorW( + securityDescriptor, + 1, + out descriptor, + out descriptorSize)) + { + errorCode = Marshal.GetLastWin32Error(); + return null; + } + + try + { + var attributes = new SecurityAttributes + { + Length = Marshal.SizeOf(typeof(SecurityAttributes)), + SecurityDescriptor = descriptor, + InheritHandle = 0, + }; + var handle = CreateFileW(stagingPath, 0x40010000, 0, ref attributes, 1, 0x80, IntPtr.Zero); + if (handle.IsInvalid) + { + errorCode = Marshal.GetLastWin32Error(); + handle.Dispose(); + return null; + } + return new OpenClawPrivatePlanFile(stagingPath, finalPath, handle); + } + finally + { + LocalFree(descriptor); + } + } + + public int ArmDeleteOnClose() + { + if (handle == null || handle.IsInvalid || handle.IsClosed) + { + return 6; + } + if (SetDeleteOnClose(handle, true)) + { + return 0; + } + var dispositionError = Marshal.GetLastWin32Error(); + handle.Dispose(); + handle = null; + DeleteFileW(stagingPath); + return dispositionError == 0 ? 29 : dispositionError; + } + + public int WriteAndPublish(byte[] content) + { + if (handle == null || handle.IsInvalid || handle.IsClosed) + { + return 6; + } + uint written; + if (content.Length > 0 && + (!WriteFile(handle, content, (uint)content.Length, out written, IntPtr.Zero) || + written != (uint)content.Length)) + { + var writeError = Marshal.GetLastWin32Error(); + return writeError == 0 ? 29 : writeError; + } + if (!FlushFileBuffers(handle)) + { + var flushError = Marshal.GetLastWin32Error(); + return flushError == 0 ? 29 : flushError; + } + if (!SetDeleteOnClose(handle, false)) + { + var dispositionError = Marshal.GetLastWin32Error(); + return dispositionError == 0 ? 29 : dispositionError; + } + handle.Dispose(); + handle = null; + if (MoveFileExW(stagingPath, finalPath, 0x8)) + { + return 0; + } + var moveError = Marshal.GetLastWin32Error(); + DeleteFileW(stagingPath); + return moveError == 0 ? 29 : moveError; + } + + public void Dispose() + { + if (handle != null) + { + var openHandle = handle; + handle = null; + var deletePending = SetDeleteOnClose(openHandle, true); + openHandle.Dispose(); + if (!deletePending) + { + DeleteFileW(stagingPath); + } + } + } +} +`; + +function readWindowsEnv(env: NodeJS.ProcessEnv, name: string): string | undefined { + const lower = name.toLowerCase(); + return Object.entries(env).find(([key]) => key.toLowerCase() === lower)?.[1]; +} + +async function resolvePrivateWindowsCompilerTempDir(env: NodeJS.ProcessEnv): Promise { + const candidate = readWindowsEnv(env, "TEMP") ?? readWindowsEnv(env, "TMP"); + if (!candidate || !path.win32.isAbsolute(candidate)) { + throw new Error( + "Unable to resolve an absolute Windows temp directory for private plan creation.", + ); + } + return await resolveTrustedOnePasswordDirectoryPath(candidate); +} + +export async function createPrivateWindowsPlanFile( + filePath: string, + content: string, + env: NodeJS.ProcessEnv = process.env, + dependencies: WindowsPrivatePlanFileDependencies = {}, +): Promise { + const resolveTrustedExecutable = + dependencies.resolveTrustedExecutable ?? resolveTrustedWindowsSystemExecutablePath; + const resolveCompilerTempDir = + dependencies.resolveCompilerTempDir ?? resolvePrivateWindowsCompilerTempDir; + const run = dependencies.run ?? runExec; + const systemRoot = + readWindowsEnv(env, "SYSTEMROOT") ?? readWindowsEnv(env, "WINDIR") ?? "C:\\Windows"; + if (!path.win32.isAbsolute(systemRoot)) { + throw new Error("Unable to resolve the Windows system directory for private plan creation."); + } + const resolvedPath = path.resolve(filePath); + const stagingPath = path.join(path.dirname(resolvedPath), `.openclaw-plan-${randomUUID()}.tmp`); + const command = [ + "$ErrorActionPreference = 'Stop'", + "$payloadJson = [Text.Encoding]::UTF8.GetString([Convert]::FromBase64String([Console]::In.ReadToEnd()))", + "$payload = $payloadJson | ConvertFrom-Json", + "Add-Type -TypeDefinition $payload.nativeSource -Language CSharp", + "$finalPath = $payload.finalPath", + "$stagingPath = $payload.stagingPath", + "$current = [System.Security.Principal.WindowsIdentity]::GetCurrent().User", + "$security = New-Object System.Security.AccessControl.FileSecurity", + "$security.SetAccessRuleProtection($true, $false)", + "$security.SetOwner($current)", + "$expected = @($current.Value, 'S-1-5-18') | Sort-Object -Unique", + "foreach ($sidValue in $expected) { $sid = New-Object System.Security.Principal.SecurityIdentifier($sidValue); $rule = New-Object System.Security.AccessControl.FileSystemAccessRule($sid, [System.Security.AccessControl.FileSystemRights]::FullControl, [System.Security.AccessControl.AccessControlType]::Allow); [void]$security.AddAccessRule($rule) }", + "$sections = [System.Security.AccessControl.AccessControlSections]::Owner -bor [System.Security.AccessControl.AccessControlSections]::Access", + "$sddl = $security.GetSecurityDescriptorSddlForm($sections)", + "$content = [Convert]::FromBase64String($payload.content)", + "$openError = 0", + "$native = [OpenClawPrivatePlanFile]::Open($stagingPath, $finalPath, $sddl, [ref]$openError)", + "$errorCode = $openError", + "if ($null -ne $native) { try { $actual = Get-Acl -LiteralPath $stagingPath; $rules = @($actual.GetAccessRules($true, $true, [System.Security.Principal.SecurityIdentifier])); if (!$actual.AreAccessRulesProtected -or $rules.Count -ne $expected.Count) { throw 'private plan ACL verification failed' }; foreach ($rule in $rules) { if ($rule.AccessControlType -ne [System.Security.AccessControl.AccessControlType]::Allow -or $expected -notcontains $rule.IdentityReference.Value -or ($rule.FileSystemRights -band [System.Security.AccessControl.FileSystemRights]::FullControl) -ne [System.Security.AccessControl.FileSystemRights]::FullControl) { throw 'private plan ACL verification failed' } }; $errorCode = $native.ArmDeleteOnClose(); if ($errorCode -eq 0) { $errorCode = $native.WriteAndPublish($content) } } finally { $native.Dispose() } }", + `if ($errorCode -eq 80 -or $errorCode -eq 183) { throw '${WINDOWS_PLAN_FILE_EXISTS_MARKER}' }`, + "if ($errorCode -ne 0) { $exception = New-Object System.ComponentModel.Win32Exception($errorCode); throw $exception }", + ].join("; "); + const powershellCandidate = path.win32.join( + systemRoot, + "System32", + "WindowsPowerShell", + "v1.0", + "powershell.exe", + ); + const powershell = await resolveTrustedExecutable(powershellCandidate); + const compilerTempDir = await resolveCompilerTempDir(env); + const input = Buffer.from( + JSON.stringify({ + content: Buffer.from(content, "utf8").toString("base64"), + finalPath: path.toNamespacedPath(resolvedPath), + nativeSource: WINDOWS_PRIVATE_PLAN_FILE_NATIVE_SOURCE, + stagingPath: path.toNamespacedPath(stagingPath), + }), + "utf8", + ).toString("base64"); + try { + await run( + powershell, + [ + "-NoLogo", + "-NoProfile", + "-NonInteractive", + "-EncodedCommand", + Buffer.from(command, "utf16le").toString("base64"), + ], + { + baseEnv: {}, + env: { + SYSTEMROOT: systemRoot, + TEMP: compilerTempDir, + TMP: compilerTempDir, + WINDIR: systemRoot, + }, + input, + logOutput: false, + maxBuffer: 64 * 1024, + timeoutMs: 10_000, + }, + ); + } catch (error) { + if (String(error).includes(WINDOWS_PLAN_FILE_EXISTS_MARKER)) { + const existsError = new Error(`Private plan file already exists: ${filePath}`); + (existsError as NodeJS.ErrnoException).code = "EEXIST"; + throw existsError; + } + throw new Error(`Unable to create private Windows plan file: ${filePath}`, { cause: error }); + } +} diff --git a/extensions/onepassword/src/secret-ref-cli.test.ts b/extensions/onepassword/src/secret-ref-cli.test.ts index b78d7d532a4a..f8529fa5e968 100644 --- a/extensions/onepassword/src/secret-ref-cli.test.ts +++ b/extensions/onepassword/src/secret-ref-cli.test.ts @@ -1,8 +1,10 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; +import { inspectPathPermissions } from "@openclaw/fs-safe/permissions"; import { Command } from "commander"; import type { OpenClawConfig } from "openclaw/plugin-sdk/plugin-entry"; +import type { runExec } from "openclaw/plugin-sdk/process-runtime"; import { afterEach, describe, expect, it, vi } from "vitest"; import { encodeOnePasswordSecretId } from "../onepassword-secret-id.js"; import { registerOnePasswordSecretRefCommands, testing } from "./secret-ref-cli.js"; @@ -19,7 +21,12 @@ function captureStdout() { function createProgram(config: OpenClawConfig): Command { const program = new Command().exitOverride(); const onepassword = program.command("onepassword"); - registerOnePasswordSecretRefCommands({ command: onepassword, config }); + registerOnePasswordSecretRefCommands({ + command: onepassword, + config, + tokenFile: path.join(os.tmpdir(), "openclaw-onepassword-missing-token"), + env: { PATH: "" }, + }); return program; } @@ -159,6 +166,52 @@ describe("1Password CLI helpers", () => { ]); }); + it.each([ + ["posix", "/tmp/plan.json", "/tmp/plan.json"], + ["posix", "/tmp/plan with spaces.json", "'/tmp/plan with spaces.json'"], + ["posix", "/tmp/plan'$(touch pwn).json", "'/tmp/plan'\\''$(touch pwn).json'"], + ["powershell", String.raw`C:\$env:TEMP\plan';.json`, String.raw`'C:\$env:TEMP\plan'';.json'`], + ["cmd", String.raw`C:\Users\Jane Doe\plan.json`, String.raw`"C:\Users\Jane Doe\plan.json"`], + ] satisfies Array<["cmd" | "posix" | "powershell", string, string]>)( + "shell-quotes %s command arguments for %j", + (shell, value, expected) => { + expect(testing.quoteCliArg(value, shell)).toBe(expected); + }, + ); + + it("rejects line breaks in generated command arguments", () => { + expect(() => testing.quoteCliArg("plan.json\nopenclaw secrets reload", "posix")).toThrow( + /cannot contain CR or LF/, + ); + expect(() => testing.quoteCliArg("plan.json\r& whoami", "cmd")).toThrow( + /cannot contain CR or LF/, + ); + }); + + it("renders native follow-up commands for both Windows shells", () => { + expect(testing.renderApplyCommands(String.raw`C:\Users\Jane Doe\plan;.json`, "win32")).toEqual([ + "PowerShell:", + String.raw` openclaw secrets apply --from 'C:\Users\Jane Doe\plan;.json' --dry-run --allow-exec`, + String.raw` openclaw secrets apply --from 'C:\Users\Jane Doe\plan;.json' --allow-exec`, + "Command Prompt:", + String.raw` openclaw secrets apply --from "C:\Users\Jane Doe\plan;.json" --dry-run --allow-exec`, + String.raw` openclaw secrets apply --from "C:\Users\Jane Doe\plan;.json" --allow-exec`, + ]); + }); + + it("omits unsafe interactive Command Prompt commands", () => { + const commands = testing.renderApplyCommands(String.raw`C:\%TEMP%\plan!.json`, "win32"); + expect(commands).toContain( + "Command Prompt: unavailable for paths containing % or !; use PowerShell.", + ); + expect(commands.filter((command) => command.includes("openclaw secrets apply"))).toHaveLength( + 2, + ); + expect(() => testing.quoteCliArg(String.raw`C:\%TEMP%\plan!.json`, "cmd")).toThrow( + /cannot safely quote/, + ); + }); + it("parses config target mappings", () => { expect( testing.parseConfigTargetMappings([ @@ -178,6 +231,14 @@ describe("1Password CLI helpers", () => { ]); }); + it("rejects non-canonical auth-profile agent ids", () => { + expect(() => + testing.parseConfigTargetMappings([ + "auth-profiles:../main:profiles.openai.key=op://openclaw/OpenAI/credential", + ]), + ).toThrow("Invalid --target auth-profiles target for 1Password"); + }); + it("rejects duplicate model providers", () => { expect(() => testing.collectProviderSecrets({ @@ -187,6 +248,70 @@ describe("1Password CLI helpers", () => { ).toThrow("Duplicate model provider id in 1Password setup: openai"); }); + it("rejects setup plans without targets", () => { + expect(() => + testing.buildPlan({ + providerAlias: "onepassword", + providerConfig: testing.buildProviderConfig(), + providerSecrets: [], + }), + ).toThrow("No SecretRef targets selected"); + }); + + it("reports trusted executable and token prerequisites without exposing the token", async () => { + const resolveTrustedCli = vi.fn(async () => "/trusted/op"); + const readTokenFile = vi.fn(() => "not-a-real-service-account-token"); + await expect( + testing.inspectSecretRefReadiness( + { + env: { CLAW_1PASSWORD_OP: "/trusted/op", PATH: "/bin" }, + tokenFile: "/state/credentials/onepassword/service-account-token", + }, + { resolveTrustedCli, readTokenFile }, + ), + ).resolves.toEqual({ + opCommand: "/trusted/op", + opBinaryPath: "/trusted/op", + opStatus: "ready", + tokenFile: "/state/credentials/onepassword/service-account-token", + tokenFileStatus: "ready", + prerequisitesReady: true, + }); + expect(resolveTrustedCli).toHaveBeenCalledWith({ + configuredPath: "/trusted/op", + pathEnv: "/bin", + }); + expect(readTokenFile).toHaveBeenCalledWith( + "/state/credentials/onepassword/service-account-token", + ); + }); + + it("reports untrusted op and unsafe token prerequisites", async () => { + await expect( + testing.inspectSecretRefReadiness( + { + env: { CLAW_1PASSWORD_OP: "op", PATH: "/bin" }, + tokenFile: "/missing-token", + }, + { + resolveTrustedCli: async () => { + throw new Error("unsafe path detail"); + }, + readTokenFile: () => { + throw new Error("unsafe token detail"); + }, + }, + ), + ).resolves.toEqual({ + opCommand: "op", + opBinaryPath: null, + opStatus: "untrusted", + tokenFile: "/missing-token", + tokenFileStatus: "missing-or-unsafe", + prerequisitesReady: false, + }); + }); + it("rejects traversal segments in SecretRef ids", () => { expect(() => testing.parseProviderKeyMappings(["openai=op://openclaw/../credential"])).toThrow( "Invalid --provider-key openai 1Password SecretRef id", @@ -203,10 +328,16 @@ describe("1Password CLI helpers", () => { it("rejects unsupported config target paths", () => { expect(() => - testing.createConfigSecretTarget({ + testing.buildPlan({ providerAlias: "onepassword", - path: "secrets.github_pat", - secretId: "op://openclaw/GitHub/pat", + providerConfig: testing.buildProviderConfig(), + providerSecrets: [], + configTargetSecrets: [ + { + path: "secrets.github_pat", + secretId: "op://openclaw/GitHub/pat", + }, + ], }), ).toThrow("Unknown or unsupported 1Password setup target path: secrets.github_pat"); }); @@ -238,11 +369,29 @@ describe("1Password CLI helpers", () => { const plan = testing.buildPlan({ providerAlias: "onepassword", providerConfig: testing.buildProviderConfig(), - providerSecrets: [], + providerSecrets: [ + { + providerId: "openai", + secretId: "op://openclaw/OpenAI/credential", + }, + ], }); try { await testing.writePlanFile(plan, planPath); - expect((await fs.stat(planPath)).mode & 0o777).toBe(0o600); + if (process.platform !== "win32") { + expect((await fs.stat(planPath)).mode & 0o777).toBe(0o600); + } else { + const permissions = await inspectPathPermissions(planPath); + expect(permissions).toMatchObject({ + ok: true, + source: "windows-acl", + ownerTrusted: true, + groupReadable: false, + groupWritable: false, + worldReadable: false, + worldWritable: false, + }); + } await expect(testing.writePlanFile(plan, planPath)).rejects.toThrow( "Plan path already exists", ); @@ -256,6 +405,199 @@ describe("1Password CLI helpers", () => { await fs.rm(tempDir, { recursive: true, force: true }); } }); + + it.skipIf(process.platform === "win32")( + "rejects plan output in a directory writable by another account", + async () => { + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-1password-plan-test-")); + const planPath = path.join(tempDir, "plan.json"); + const plan = testing.buildPlan({ + providerAlias: "onepassword", + providerConfig: testing.buildProviderConfig(), + providerSecrets: [ + { + providerId: "openai", + secretId: "op://openclaw/OpenAI/credential", + }, + ], + }); + try { + await fs.chmod(tempDir, 0o777); + await expect(testing.writePlanFile(plan, planPath)).rejects.toThrow( + "path is writable by another user", + ); + await expect(fs.stat(planPath)).rejects.toMatchObject({ code: "ENOENT" }); + } finally { + await fs.chmod(tempDir, 0o700); + await fs.rm(tempDir, { recursive: true, force: true }); + } + }, + ); + + it.skipIf(process.platform === "win32")( + "writes through the canonical directory instead of a replaceable alias", + async () => { + const trustedDir = await fs.mkdtemp( + path.join(os.tmpdir(), "openclaw-1password-plan-trusted-"), + ); + const aliasParent = await fs.mkdtemp( + path.join(os.tmpdir(), "openclaw-1password-plan-alias-"), + ); + const aliasDir = path.join(aliasParent, "output"); + const canonicalPlanPath = path.join(await fs.realpath(trustedDir), "plan.json"); + const plan = testing.buildPlan({ + providerAlias: "onepassword", + providerConfig: testing.buildProviderConfig(), + providerSecrets: [ + { + providerId: "openai", + secretId: "op://openclaw/OpenAI/credential", + }, + ], + }); + try { + await fs.symlink(trustedDir, aliasDir); + await fs.chmod(aliasParent, 0o777); + await expect(testing.writePlanFile(plan, path.join(aliasDir, "plan.json"))).resolves.toBe( + canonicalPlanPath, + ); + expect(JSON.parse(await fs.readFile(canonicalPlanPath, "utf8"))).toMatchObject({ + version: 1, + }); + } finally { + await fs.chmod(aliasParent, 0o700); + await fs.rm(aliasParent, { recursive: true, force: true }); + await fs.rm(trustedDir, { recursive: true, force: true }); + } + }, + ); + + it.skipIf(process.platform === "win32")( + "rejects unrenderable plan paths before creating a file", + async () => { + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-1password-plan-test-")); + const planPath = path.join(tempDir, "plan\n.json"); + const plan = testing.buildPlan({ + providerAlias: "onepassword", + providerConfig: testing.buildProviderConfig(), + providerSecrets: [ + { + providerId: "openai", + secretId: "op://openclaw/OpenAI/credential", + }, + ], + }); + try { + await expect(testing.writePlanFile(plan, planPath)).rejects.toThrow( + "Command argument cannot contain CR or LF", + ); + await expect(fs.stat(planPath)).rejects.toMatchObject({ code: "ENOENT" }); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } + }, + ); + + it("writes a Windows plan through the atomic private-file primitive", async () => { + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-1password-plan-test-")); + const planPath = path.join(tempDir, "plan.json"); + const plan = testing.buildPlan({ + providerAlias: "onepassword", + providerConfig: testing.buildProviderConfig(), + providerSecrets: [ + { + providerId: "openai", + secretId: "op://openclaw/OpenAI/credential", + }, + ], + }); + const createPrivateWindowsFile = vi.fn(async (filePath: string, content: string) => { + await fs.writeFile(filePath, content, { flag: "wx" }); + }); + const resolveTrustedPlanDirectory = vi.fn(async (directoryPath: string) => directoryPath); + try { + await testing.writePlanFile(plan, planPath, { + platform: "win32", + createPrivateWindowsFile, + resolveTrustedPlanDirectory, + }); + expect(resolveTrustedPlanDirectory).toHaveBeenCalledWith(path.resolve(tempDir)); + expect(createPrivateWindowsFile).toHaveBeenCalledWith(planPath, expect.any(String)); + expect(JSON.parse(await fs.readFile(planPath, "utf8"))).toMatchObject({ version: 1 }); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } + }); + + it("resolves trusted PowerShell and passes plan content through stdin", async () => { + let observedInput: string | Uint8Array | undefined; + const run = vi.fn(async (_command, _args, options) => { + observedInput = typeof options === "object" ? options.input : undefined; + return { stdout: "", stderr: "" }; + }); + const powershell = "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe"; + const resolveTrustedExecutable = vi.fn(async () => powershell); + const content = '{"version":1}\n'; + await testing.createPrivateWindowsPlanFile( + "C:\\plans\\plan.json", + content, + { SYSTEMROOT: "C:\\Windows" }, + { + resolveCompilerTempDir: async () => "C:\\Users\\me\\AppData\\Local\\Temp", + resolveTrustedExecutable, + run, + }, + ); + expect(resolveTrustedExecutable).toHaveBeenCalledWith(powershell); + expect(run).toHaveBeenCalledOnce(); + expect(run).toHaveBeenCalledWith( + powershell, + expect.any(Array), + expect.objectContaining({ + baseEnv: {}, + input: expect.any(String), + logOutput: false, + }), + ); + const payload = JSON.parse(Buffer.from(String(observedInput), "base64").toString("utf8")) as { + content: string; + finalPath: string; + stagingPath: string; + }; + expect(Buffer.from(payload.content, "base64").toString("utf8")).toBe(content); + expect(payload.finalPath).toContain("plan.json"); + expect(path.win32.basename(payload.stagingPath)).toMatch(/^\.openclaw-plan-[^.]+\.tmp$/u); + }); + + it("prints the readiness check before plan application", async () => { + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-1password-setup-test-")); + const planPath = path.join(tempDir, "plan with spaces.json"); + const canonicalPlanPath = path.join(await fs.realpath(tempDir), "plan with spaces.json"); + const output = captureStdout(); + try { + await createProgram({}).parseAsync( + [ + "onepassword", + "secretref", + "setup", + "--openai-id", + "op://openclaw/OpenAI/credential", + "--plan-out", + planPath, + ], + { from: "user" }, + ); + expect(output()).toContain("openclaw onepassword secretref status"); + expect(output()).toContain( + `openclaw secrets apply --from '${canonicalPlanPath}' --dry-run --allow-exec`, + ); + expect(output()).toContain( + `openclaw secrets apply --from '${canonicalPlanPath}' --allow-exec`, + ); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } + }); }); describe("1Password CLI status", () => { @@ -271,6 +613,30 @@ describe("1Password CLI status", () => { }, }); expect(result.providerAlias).toBe("corp-onepassword"); + expect(result).toMatchObject({ + providerReady: true, + opStatus: "not-found", + tokenFileStatus: "missing-or-unsafe", + prerequisitesReady: false, + ready: false, + issues: ["op-not-found", "token-file-missing-or-unsafe"], + }); + }); + + it("prefers the managed integration when the default alias is unrelated", async () => { + const result = await runStatus({ + secrets: { + providers: { + onepassword: { source: "exec", command: "/legacy/resolver" }, + "corp-onepassword": { + source: "exec", + pluginIntegration: { pluginId: "onepassword", integrationId: "onepassword" }, + }, + }, + }, + }); + expect(result.providerAlias).toBe("corp-onepassword"); + expect(result.providerReady).toBe(true); }); it("requires an explicit alias when multiple providers are configured", async () => { diff --git a/extensions/onepassword/src/secret-ref-cli.ts b/extensions/onepassword/src/secret-ref-cli.ts index d3442077b94d..78ab030515f7 100644 --- a/extensions/onepassword/src/secret-ref-cli.ts +++ b/extensions/onepassword/src/secret-ref-cli.ts @@ -3,9 +3,18 @@ import fs from "node:fs/promises"; import path from "node:path"; import { createInterface } from "node:readline/promises"; import type { OpenClawConfig } from "openclaw/plugin-sdk/plugin-entry"; -import { resolveSecretPlanTargetByPath } from "openclaw/plugin-sdk/secret-ref-runtime"; +import { + DEFAULT_SECRET_FILE_MAX_BYTES, + tryReadSecretFileSync, +} from "openclaw/plugin-sdk/secret-file-runtime"; +import { pluginSecretRefSetup } from "openclaw/plugin-sdk/secret-ref-runtime"; import { resolvePreferredOpenClawTmpDir } from "openclaw/plugin-sdk/temp-path"; +import { + resolveTrustedOnePasswordCli, + resolveTrustedOnePasswordDirectoryPath, +} from "../onepassword-op-path.js"; import { encodeOnePasswordSecretId } from "../onepassword-secret-id.js"; +import { createPrivateWindowsPlanFile } from "./private-plan-file.js"; type CommandLike = { command(name: string): CommandLike; @@ -19,22 +28,6 @@ type CommandLike = { action(fn: (options: TOptions) => void | Promise): CommandLike; }; -type SecretRef = { - source: "exec"; - provider: string; - id: string; -}; - -type SecretsPlanTarget = { - type: string; - path: string; - pathSegments: string[]; - agentId?: string; - providerId?: string; - accountId?: string; - ref: SecretRef; -}; - type OnePasswordExecProviderConfig = { source: "exec"; pluginIntegration: { @@ -54,18 +47,13 @@ type ConfigTargetSecretMapping = { secretId: string; }; -type SecretsApplyPlan = { - version: 1; - protocolVersion: 1; - generatedAt: string; - generatedBy: "manual"; - providerUpserts: Record; - targets: SecretsPlanTarget[]; -}; +type SecretsApplyPlan = ReturnType; type RegisterOnePasswordSecretRefCommandsParams = { command: CommandLike; config: OpenClawConfig; + tokenFile: string; + env?: NodeJS.ProcessEnv; }; type StatusOptions = { @@ -93,10 +81,27 @@ type ProviderStatus = { }; }; +type SecretRefReadiness = { + opCommand: string; + opBinaryPath: string | null; + opStatus: "ready" | "not-found" | "untrusted"; + tokenFile: string; + tokenFileStatus: "ready" | "missing-or-unsafe"; + prerequisitesReady: boolean; +}; + +type ReadinessDependencies = { + resolveTrustedCli?: typeof resolveTrustedOnePasswordCli; + readTokenFile?: (filePath: string) => string | undefined; +}; + +type WritePlanFileDependencies = { + platform?: NodeJS.Platform; + createPrivateWindowsFile?: (filePath: string, content: string) => Promise; + resolveTrustedPlanDirectory?: typeof resolveTrustedOnePasswordDirectoryPath; +}; + const ONEPASSWORD_PROVIDER_ALIAS = "onepassword"; -const SECRET_PROVIDER_ALIAS_PATTERN = /^[a-z][a-z0-9_-]{0,63}$/; -const MODEL_PROVIDER_ID_PATTERN = /^[A-Za-z0-9][A-Za-z0-9._:-]{0,127}$/; -const FORBIDDEN_PATH_SEGMENTS = new Set(["__proto__", "prototype", "constructor"]); function writeLine(message = ""): void { process.stdout.write(`${message}\n`); @@ -114,29 +119,56 @@ function normalizeOptionalString(value: unknown): string | undefined { return typeof value === "string" && value.trim() ? value.trim() : undefined; } -function parseDotPath(pathname: string): string[] { - return pathname - .split(".") - .map((segment) => segment.trim()) - .filter((segment) => segment.length > 0); +type CommandShell = "cmd" | "posix" | "powershell"; + +function quoteCliArg(value: string, shell: CommandShell): string { + if (/\r|\n/u.test(value)) { + throw new Error("Command argument cannot contain CR or LF"); + } + if (shell === "cmd") { + if (/[%!]/u.test(value)) { + throw new Error("Interactive Command Prompt cannot safely quote paths containing % or !"); + } + const escaped = value.replaceAll('"', '\\"'); + return /[ \t"&|<>^()]/u.test(value) ? `"${escaped}"` : escaped || '""'; + } + if (shell === "powershell") { + return `'${value.replaceAll("'", "''")}'`; + } + if (/^[A-Za-z0-9_/:=.,@%+-]+$/.test(value)) { + return value; + } + return `'${value.replaceAll("'", "'\\''")}'`; } -function toDotPath(segments: string[]): string { - return segments.join("."); +function renderApplyCommands( + planPath: string, + platform: NodeJS.Platform = process.platform, +): string[] { + const render = (shell: CommandShell, extraIndent = "") => { + const quotedPlanPath = quoteCliArg(planPath, shell); + return [ + `${extraIndent}openclaw secrets apply --from ${quotedPlanPath} --dry-run --allow-exec`, + `${extraIndent}openclaw secrets apply --from ${quotedPlanPath} --allow-exec`, + ]; + }; + if (platform !== "win32") { + return render("posix"); + } + // Windows cannot reveal which parent shell will receive these copy-paste commands. + // Print native variants instead of emitting syntax that is unsafe in the other shell. + const powershellCommands = ["PowerShell:", ...render("powershell", " ")]; + if (/[%!]/u.test(planPath)) { + return [ + ...powershellCommands, + "Command Prompt: unavailable for paths containing % or !; use PowerShell.", + ]; + } + return [...powershellCommands, "Command Prompt:", ...render("cmd", " ")]; } function assertValidProviderAlias(value: string): void { - if (!SECRET_PROVIDER_ALIAS_PATTERN.test(value)) { - throw new Error( - `Invalid provider alias "${value}". Use lowercase letters, numbers, underscores, or hyphens.`, - ); - } -} - -function assertValidModelProviderId(label: string, value: string): void { - if (!MODEL_PROVIDER_ID_PATTERN.test(value)) { - throw new Error(`Invalid ${label} model provider id: ${value}`); - } + pluginSecretRefSetup.assertValidProviderAlias(value); } function normalizeOnePasswordSecretId(label: string, value: string): string { @@ -188,9 +220,6 @@ function resolveStatusProviderAlias(config: OpenClawConfig, requestedAlias?: str assertValidProviderAlias(explicitAlias); return explicitAlias; } - if (readProviderStatus(config, ONEPASSWORD_PROVIDER_ALIAS).configured) { - return ONEPASSWORD_PROVIDER_ALIAS; - } const configuredAliases = Object.entries(config.secrets?.providers ?? {}) .filter(([, provider]) => isOnePasswordIntegrationProvider(provider)) .map(([alias]) => alias) @@ -203,24 +232,52 @@ function resolveStatusProviderAlias(config: OpenClawConfig, requestedAlias?: str return configuredAliases[0] ?? ONEPASSWORD_PROVIDER_ALIAS; } -function resolveOpCommand(): string { - return normalizeOptionalString(process.env.CLAW_1PASSWORD_OP) ?? "op"; -} +async function inspectSecretRefReadiness( + params: { env: NodeJS.ProcessEnv; tokenFile: string }, + dependencies: ReadinessDependencies = {}, +): Promise { + const resolveTrustedCli = dependencies.resolveTrustedCli ?? resolveTrustedOnePasswordCli; + const readTokenFile = + dependencies.readTokenFile ?? + ((filePath: string) => + tryReadSecretFileSync(filePath, "1Password service account token", { + maxBytes: DEFAULT_SECRET_FILE_MAX_BYTES, + rejectHardlinks: false, + rejectSymlink: true, + })); + const configuredOpCommand = normalizeOptionalString(params.env.CLAW_1PASSWORD_OP); + const opCommand = configuredOpCommand ?? "op"; + const { opBinaryPath, opStatus } = await (async () => { + try { + const resolvedPath = + (await resolveTrustedCli({ + ...(configuredOpCommand ? { configuredPath: configuredOpCommand } : {}), + pathEnv: params.env.PATH, + })) ?? null; + return { + opBinaryPath: resolvedPath, + opStatus: resolvedPath ? ("ready" as const) : ("not-found" as const), + }; + } catch { + return { opBinaryPath: null, opStatus: "untrusted" as const }; + } + })(); -async function pathExists(filePath: string): Promise { - try { - await fs.access(filePath); - return true; - } catch { - return false; - } -} - -async function isConfiguredOpCommandAvailable(command: string): Promise { - if (!path.isAbsolute(command)) { - return undefined; - } - return pathExists(command); + const tokenFileStatus: SecretRefReadiness["tokenFileStatus"] = (() => { + try { + return readTokenFile(params.tokenFile) ? "ready" : "missing-or-unsafe"; + } catch { + return "missing-or-unsafe"; + } + })(); + return { + opCommand, + opBinaryPath, + opStatus, + tokenFile: params.tokenFile, + tokenFileStatus, + prerequisitesReady: opStatus === "ready" && tokenFileStatus === "ready", + }; } function buildProviderConfig(): OnePasswordExecProviderConfig { @@ -233,79 +290,11 @@ function buildProviderConfig(): OnePasswordExecProviderConfig { }; } -function createModelApiKeyTarget(params: { - providerAlias: string; - providerId: string; - secretId: string; -}): SecretsPlanTarget { - assertValidModelProviderId("target", params.providerId); - return { - type: "models.providers.apiKey", - path: `models.providers.${params.providerId}.apiKey`, - pathSegments: ["models", "providers", params.providerId, "apiKey"], - providerId: params.providerId, - ref: { - source: "exec", - provider: params.providerAlias, - id: params.secretId, - }, - }; -} - function parseTargetSpecifier(value: string): { path: string; agentId?: string; } { - if (value.startsWith("auth-profiles:")) { - const remainder = value.slice("auth-profiles:".length); - const separatorIndex = remainder.indexOf(":"); - const agentId = separatorIndex >= 0 ? remainder.slice(0, separatorIndex) : ""; - const targetPath = separatorIndex >= 0 ? remainder.slice(separatorIndex + 1) : ""; - if (!agentId || !targetPath) { - throw new Error(`Invalid --target auth-profiles target: ${value}`); - } - return { agentId, path: targetPath }; - } - return { - path: value.startsWith("openclaw:") ? value.slice("openclaw:".length) : value, - }; -} - -function createConfigSecretTarget(params: { - providerAlias: string; - path: string; - agentId?: string; - secretId: string; -}): SecretsPlanTarget { - const pathSegments = parseDotPath(params.path); - const normalizedPath = toDotPath(pathSegments); - if ( - pathSegments.length === 0 || - normalizedPath !== params.path || - pathSegments.some((segment) => FORBIDDEN_PATH_SEGMENTS.has(segment)) - ) { - throw new Error(`Invalid --target config path: ${params.path}`); - } - const resolved = resolveSecretPlanTargetByPath({ - configFile: params.agentId ? "auth-profiles.json" : "openclaw.json", - pathSegments, - }); - if (!resolved) { - throw new Error(`Unknown or unsupported 1Password setup target path: ${params.path}`); - } - return { - type: resolved.targetType, - path: normalizedPath, - pathSegments, - ...(params.agentId ? { agentId: params.agentId } : {}), - ...(resolved.providerId ? { providerId: resolved.providerId } : {}), - ...(resolved.accountId ? { accountId: resolved.accountId } : {}), - ref: { - source: "exec", - provider: params.providerAlias, - id: params.secretId, - }, - }; + return pluginSecretRefSetup.parseTargetSpecifier("1Password", value); } function parseProviderKeyMappings(values: string[] | undefined): ProviderSecretMapping[] { @@ -317,7 +306,7 @@ function parseProviderKeyMappings(values: string[] | undefined): ProviderSecretM ); } const providerId = value.slice(0, separator).trim(); - assertValidModelProviderId("--provider-key", providerId); + pluginSecretRefSetup.assertValidModelProviderId("--provider-key", providerId); const secretId = normalizeOnePasswordSecretId( `--provider-key ${providerId}`, value.slice(separator + 1).trim(), @@ -375,53 +364,19 @@ function collectProviderSecrets(options: { return providerSecrets; } -function assertNoDuplicatePlanTargets(targets: SecretsPlanTarget[]): void { - const seen = new Set(); - for (const target of targets) { - const key = target.agentId - ? `auth-profiles:${target.agentId}:${target.path}` - : `openclaw:${target.path}`; - if (seen.has(key)) { - throw new Error(`Duplicate secret target path in 1Password setup: ${target.path}`); - } - seen.add(key); - } -} - function buildPlan(params: { providerAlias: string; providerConfig: OnePasswordExecProviderConfig; providerSecrets: ProviderSecretMapping[]; configTargetSecrets?: ConfigTargetSecretMapping[]; }): SecretsApplyPlan { - const targets = [ - ...params.providerSecrets.map((entry) => - createModelApiKeyTarget({ - providerAlias: params.providerAlias, - providerId: entry.providerId, - secretId: entry.secretId, - }), - ), - ...(params.configTargetSecrets ?? []).map((entry) => - createConfigSecretTarget({ - providerAlias: params.providerAlias, - path: entry.path, - ...(entry.agentId ? { agentId: entry.agentId } : {}), - secretId: entry.secretId, - }), - ), - ]; - assertNoDuplicatePlanTargets(targets); - return { - version: 1, - protocolVersion: 1, - generatedAt: new Date().toISOString(), - generatedBy: "manual", - providerUpserts: { - [params.providerAlias]: params.providerConfig, - }, - targets, - }; + const plan = pluginSecretRefSetup.buildPlan({ productName: "1Password", ...params }); + if (plan.targets.length === 0) { + throw new Error( + "No SecretRef targets selected. Pass --openai-id, --anthropic-id, --openrouter-id, --provider-key, or --target.", + ); + } + return plan; } async function promptOptionalSecretId(label: string): Promise { @@ -462,21 +417,42 @@ async function promptProviderSecrets(options: SetupOptions): Promise { +async function runStatus( + params: RegisterOnePasswordSecretRefCommandsParams, + options: StatusOptions, +): Promise { + const config = params.config; const providerAlias = resolveStatusProviderAlias(config, options.providerAlias); const provider = readProviderStatus(config, providerAlias); - const opCommand = resolveOpCommand(); + const providerReady = isOnePasswordIntegrationProvider( + config.secrets?.providers?.[providerAlias], + ); + const readiness = await inspectSecretRefReadiness({ + env: params.env ?? process.env, + tokenFile: params.tokenFile, + }); + const issues = [ + ...(providerReady + ? [] + : [provider.configured ? "provider-misconfigured" : "provider-not-configured"]), + ...(readiness.opStatus === "ready" ? [] : [`op-${readiness.opStatus}`]), + ...(readiness.tokenFileStatus === "ready" ? [] : ["token-file-missing-or-unsafe"]), + ]; const result = { providerAlias, provider, - opCommand, - opCommandAvailable: await isConfiguredOpCommandAvailable(opCommand), + providerReady, + ...readiness, + ready: providerReady && readiness.prerequisitesReady, + issues, }; if (options.json) { writeJson(result); return; } - writeLine(`1Password provider: ${provider.configured ? "configured" : "not configured"}`); + writeLine( + `1Password provider: ${providerReady ? "ready" : provider.configured ? "misconfigured" : "not configured"}`, + ); if (provider.source) { writeLine(`Source: ${provider.source}`); } @@ -488,34 +464,138 @@ async function runStatus(config: OpenClawConfig, options: StatusOptions): Promis `Plugin integration: ${provider.pluginIntegration.pluginId}:${provider.pluginIntegration.integrationId}`, ); } - writeLine(`op command: ${result.opCommand}`); - if (result.opCommandAvailable !== undefined) { - writeLine(`op command exists: ${result.opCommandAvailable ? "yes" : "no"}`); + writeLine(`op command: ${readiness.opCommand}`); + writeLine(`op status: ${readiness.opStatus}`); + if (readiness.opBinaryPath) { + writeLine(`op binary: ${readiness.opBinaryPath}`); + } + writeLine(`token file: ${readiness.tokenFileStatus}`); + writeLine(`prerequisites ready: ${readiness.prerequisitesReady ? "yes" : "no"}`); + writeLine(`ready: ${result.ready ? "yes" : "no"}`); + if (issues.length === 0) { + return; + } + writeLine(""); + writeLine("Next actions:"); + if (!providerReady) { + writeLine(" Generate and apply a 1Password SecretRef setup plan."); + } + if (readiness.opStatus === "not-found") { + writeLine(" Install the official 1Password CLI or set CLAW_1PASSWORD_OP."); + } else if (readiness.opStatus === "untrusted") { + writeLine(" Use an absolute 1Password CLI path that is not replaceable by another user."); + } + if (readiness.tokenFileStatus !== "ready") { + writeLine(` Create a non-empty service-account token file at ${readiness.tokenFile}.`); } - writeLine("Auth: onepassword service-account token file"); } -async function writePlanFile(plan: SecretsApplyPlan, requestedPath?: string): Promise { - const planPath = +async function verifyPosixPlanFilePermissions( + handle: Awaited>, +): Promise { + await handle.chmod(0o600); + if (((await handle.stat()).mode & 0o777) !== 0o600) { + throw new Error("Unable to verify owner-only permissions for the generated plan file."); + } +} + +async function closePlanHandle( + handle: Awaited> | undefined, +): Promise { + await handle?.close().catch(() => undefined); +} + +async function writePlanFile( + plan: SecretsApplyPlan, + requestedPath?: string, + dependencies: WritePlanFileDependencies = {}, +): Promise { + const requestedPlanPath = normalizeOptionalString(requestedPath) ?? path.join(resolvePreferredOpenClawTmpDir(), `openclaw-1password-secrets-${randomUUID()}.json`); + const content = `${JSON.stringify(plan, null, 2)}\n`; + const requestedPlanPathAbsolute = path.resolve(requestedPlanPath); + const planDirectory = await ( + dependencies.resolveTrustedPlanDirectory ?? resolveTrustedOnePasswordDirectoryPath + )(path.dirname(requestedPlanPathAbsolute)); + // Write through the canonical directory returned by the trust check. Reusing the requested + // alias would let another local account retarget a writable parent symlink after validation. + const planPath = path.join(planDirectory, path.basename(requestedPlanPathAbsolute)); + const platform = dependencies.platform ?? process.platform; + // Validate the exact canonical path before the exclusive write. Follow-up command rendering + // must not fail after leaving a plan behind that the next setup attempt cannot overwrite. + renderApplyCommands(planPath, platform); + if (platform === "win32") { + try { + await (dependencies.createPrivateWindowsFile ?? createPrivateWindowsPlanFile)( + planPath, + content, + ); + return planPath; + } catch (error) { + if (error && typeof error === "object" && "code" in error && error.code === "EEXIST") { + throw new Error(`Plan path already exists; choose a new --plan-out path: ${planPath}`, { + cause: error, + }); + } + throw error; + } + } + let handle: Awaited> | undefined; + let identity: { dev: bigint; ino: bigint } | undefined; try { - await fs.writeFile(planPath, `${JSON.stringify(plan, null, 2)}\n`, { - encoding: "utf8", - flag: "wx", - mode: 0o600, - }); + handle = await fs.open(planPath, "wx", 0o600); + identity = await handle.stat({ bigint: true }); + await verifyPosixPlanFilePermissions(handle); + const pathStat = await fs.lstat(planPath, { bigint: true }); + const handleStat = await handle.stat({ bigint: true }); + if ( + pathStat.isSymbolicLink() || + !sameFileIdentity(identity, handleStat) || + !sameFileIdentity(identity, pathStat) + ) { + throw new Error("Generated plan path changed during permission setup."); + } + await handle.writeFile(content, "utf8"); + await handle.sync(); } catch (error) { + await closePlanHandle(handle); if (error && typeof error === "object" && "code" in error && error.code === "EEXIST") { throw new Error(`Plan path already exists; choose a new --plan-out path: ${planPath}`, { cause: error, }); } + if (identity) { + await removePlanFileIfUnchanged(planPath, identity); + } throw error; + } finally { + await closePlanHandle(handle); } return planPath; } +function sameFileIdentity( + left: { dev: number | bigint; ino: number | bigint }, + right: { dev: number | bigint; ino: number | bigint }, +): boolean { + return left.dev === right.dev && left.ino === right.ino; +} + +async function removePlanFileIfUnchanged( + filePath: string, + identity: { dev: number | bigint; ino: number | bigint }, +): Promise { + try { + const current = await fs.lstat(filePath, { bigint: true }); + if (!current.isSymbolicLink() && sameFileIdentity(current, identity)) { + await fs.rm(filePath, { force: true }); + } + } catch { + // The original error is authoritative; cleanup is best effort. + } +} + async function runSetup(options: SetupOptions): Promise { const providerAlias = normalizeOptionalString(options.providerAlias) ?? ONEPASSWORD_PROVIDER_ALIAS; @@ -533,8 +613,10 @@ async function runSetup(options: SetupOptions): Promise { writeLine(""); writeLine("Next steps:"); writeLine(" openclaw plugins enable onepassword"); - writeLine(` openclaw secrets apply --from ${planPath} --dry-run --allow-exec`); - writeLine(` openclaw secrets apply --from ${planPath} --allow-exec`); + writeLine(" openclaw onepassword secretref status"); + for (const command of renderApplyCommands(planPath)) { + writeLine(` ${command}`); + } writeLine(" openclaw secrets audit --check --allow-exec"); writeLine(" openclaw secrets reload"); } @@ -548,7 +630,7 @@ export function registerOnePasswordSecretRefCommands( .description("Show 1Password SecretRef provider status") .option("--json", "Print JSON status") .option("--provider-alias ", "Secret provider alias to inspect") - .action((options: StatusOptions) => runStatus(params.config, options)); + .action((options: StatusOptions) => runStatus(params, options)); secretRef .command("setup") .description("Create a 1Password SecretRef setup plan") @@ -580,9 +662,11 @@ export const testing = { buildPlan, buildProviderConfig, collectProviderSecrets, - createModelApiKeyTarget, - createConfigSecretTarget, parseConfigTargetMappings, parseProviderKeyMappings, + quoteCliArg, + renderApplyCommands, + inspectSecretRefReadiness, + createPrivateWindowsPlanFile, writePlanFile, }; diff --git a/extensions/onepassword/src/secret-ref-resolver.test.ts b/extensions/onepassword/src/secret-ref-resolver.test.ts index a8ae83fb3e91..b674f3f3eb18 100644 --- a/extensions/onepassword/src/secret-ref-resolver.test.ts +++ b/extensions/onepassword/src/secret-ref-resolver.test.ts @@ -3,6 +3,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; +import { DEFAULT_SECRET_FILE_MAX_BYTES } from "openclaw/plugin-sdk/secret-file-runtime"; import { afterEach, describe, expect, it } from "vitest"; import { encodeOnePasswordSecretId } from "../onepassword-secret-id.js"; import { createTrustedNodeFixture } from "./trusted-node.test-support.js"; @@ -94,8 +95,12 @@ describe("plugin manifest", () => { const opReadConcurrency = readIntegerConstant("OP_READ_CONCURRENCY"); const opReadTimeoutMs = readIntegerConstant("OP_READ_TIMEOUT_MS"); const maxRefsPerRequest = readIntegerConstant("MAX_SECRET_REFS_PER_REQUEST"); + const maxSecretValueBytes = readIntegerConstant("MAX_SECRET_VALUE_BYTES"); const worstCaseBatchTimeoutMs = Math.ceil(maxRefsPerRequest / opReadConcurrency) * opReadTimeoutMs; + const worstCaseEscapedValueBytes = Buffer.byteLength( + JSON.stringify("\0".repeat(maxSecretValueBytes)), + ); const manifest = JSON.parse(fs.readFileSync(manifestPath, "utf8")) as { commandAliases?: Array<{ name?: string; cliCommand?: string }>; secretProviderIntegrations?: Record>; @@ -120,7 +125,7 @@ describe("plugin manifest", () => { args: ["./onepassword-secret-ref-resolver.js"], timeoutMs: 90_000, noOutputTimeoutMs: 90_000, - maxOutputBytes: 8 * 1024 * 1024, + maxOutputBytes: 16 * 1024 * 1024, passEnv: expect.arrayContaining([ "HOME", "USERPROFILE", @@ -145,6 +150,9 @@ describe("plugin manifest", () => { expect(integration).not.toHaveProperty("trustedDirs"); expect(integration?.timeoutMs).toBeGreaterThan(worstCaseBatchTimeoutMs); expect(integration?.noOutputTimeoutMs).toBeGreaterThan(worstCaseBatchTimeoutMs); + expect(integration?.maxOutputBytes).toBeGreaterThan( + maxRefsPerRequest * worstCaseEscapedValueBytes, + ); expect(resolverSource).toContain("#!/usr/bin/env node"); expect(resolverSource).toContain('from "execa"'); expect(packageJson.openclaw?.build?.staticAssets).toContainEqual({ @@ -440,6 +448,24 @@ process.stdout.write("not-a-real-value"); }); }); + it("rejects an oversized broker service-account token file", async () => { + const result = await runResolver({ + request: { + protocolVersion: 1, + provider: "onepassword", + ids: ["op://Engineering/OpenRouter/apiKey"], + }, + env: { CLAW_1PASSWORD_OP: process.execPath }, + token: "x".repeat(DEFAULT_SECRET_FILE_MAX_BYTES + 1), + }); + + expect(result).toEqual({ + code: 1, + stdout: "", + stderr: "1Password SecretRef resolver failed.\n", + }); + }); + it.runIf(process.platform !== "win32")("rejects a symlinked broker token file", async () => { const stateDir = makeTempDir(); const tokenDir = path.join(stateDir, "credentials", "onepassword"); diff --git a/extensions/vault/src/cli.test.ts b/extensions/vault/src/cli.test.ts index b169391e1831..415ffd08850e 100644 --- a/extensions/vault/src/cli.test.ts +++ b/extensions/vault/src/cli.test.ts @@ -155,6 +155,11 @@ describe("vault CLI setup plan", () => { ], "Duplicate secret target path", ], + [ + "non-canonical auth-profile agent ids", + ["--target", "auth-profiles:../main:profiles.openai.key=providers/openai/apiKey"], + "Invalid --target auth-profiles target for Vault", + ], ])("rejects %s", async (_label, args, message) => { await expect(createSetupPlan(args)).rejects.toThrow(message); }); diff --git a/extensions/vault/src/cli.ts b/extensions/vault/src/cli.ts index 66c362b9ecdc..701cd9100b4f 100644 --- a/extensions/vault/src/cli.ts +++ b/extensions/vault/src/cli.ts @@ -3,7 +3,7 @@ import path from "node:path"; import { createInterface } from "node:readline/promises"; import { fileURLToPath } from "node:url"; import type { OpenClawConfig } from "openclaw/plugin-sdk/plugin-entry"; -import { resolveSecretPlanTargetByPath } from "openclaw/plugin-sdk/secret-ref-runtime"; +import { pluginSecretRefSetup } from "openclaw/plugin-sdk/secret-ref-runtime"; import { pathExists } from "openclaw/plugin-sdk/security-runtime"; import { resolvePreferredOpenClawTmpDir } from "openclaw/plugin-sdk/temp-path"; import { parseVaultSecretId } from "../vault-secret-id.js"; @@ -20,22 +20,6 @@ type CommandLike = { action(fn: (options: TOptions) => void | Promise): CommandLike; }; -type SecretRef = { - source: "exec"; - provider: string; - id: string; -}; - -type SecretsPlanTarget = { - type: string; - path: string; - pathSegments: string[]; - agentId?: string; - providerId?: string; - accountId?: string; - ref: SecretRef; -}; - type VaultExecProviderConfig = { source: "exec"; pluginIntegration: { @@ -55,15 +39,6 @@ type ConfigTargetSecretMapping = { secretId: string; }; -type SecretsApplyPlan = { - version: 1; - protocolVersion: 1; - generatedAt: string; - generatedBy: "manual"; - providerUpserts: Record; - targets: SecretsPlanTarget[]; -}; - type RegisterVaultCommandsParams = { program: CommandLike; config: OpenClawConfig; @@ -95,9 +70,6 @@ type ProviderStatus = { }; const VAULT_PROVIDER_ALIAS = "vault"; -const SECRET_PROVIDER_ALIAS_PATTERN = /^[a-z][a-z0-9_-]{0,63}$/; -const MODEL_PROVIDER_ID_PATTERN = /^[A-Za-z0-9][A-Za-z0-9._:-]{0,127}$/; -const FORBIDDEN_PATH_SEGMENTS = new Set(["__proto__", "prototype", "constructor"]); function writeLine(message = ""): void { process.stdout.write(`${message}\n`); @@ -115,29 +87,8 @@ function normalizeOptionalString(value: unknown): string | undefined { return typeof value === "string" && value.trim() ? value.trim() : undefined; } -function parseDotPath(pathname: string): string[] { - return pathname - .split(".") - .map((segment) => segment.trim()) - .filter((segment) => segment.length > 0); -} - -function toDotPath(segments: string[]): string { - return segments.join("."); -} - function assertValidProviderAlias(value: string): void { - if (!SECRET_PROVIDER_ALIAS_PATTERN.test(value)) { - throw new Error( - `Invalid provider alias "${value}". Use lowercase letters, numbers, underscores, or hyphens.`, - ); - } -} - -function assertValidModelProviderId(label: string, value: string): void { - if (!MODEL_PROVIDER_ID_PATTERN.test(value)) { - throw new Error(`Invalid ${label} model provider id: ${value}`); - } + pluginSecretRefSetup.assertValidProviderAlias(value); } function assertValidVaultSecretId(label: string, value: string): void { @@ -233,79 +184,11 @@ function buildProviderConfig(): VaultExecProviderConfig { }; } -function createModelApiKeyTarget(params: { - providerAlias: string; - providerId: string; - secretId: string; -}): SecretsPlanTarget { - assertValidModelProviderId("target", params.providerId); - return { - type: "models.providers.apiKey", - path: `models.providers.${params.providerId}.apiKey`, - pathSegments: ["models", "providers", params.providerId, "apiKey"], - providerId: params.providerId, - ref: { - source: "exec", - provider: params.providerAlias, - id: params.secretId, - }, - }; -} - function parseTargetSpecifier(value: string): { path: string; agentId?: string; } { - if (value.startsWith("auth-profiles:")) { - const remainder = value.slice("auth-profiles:".length); - const separatorIndex = remainder.indexOf(":"); - const agentId = separatorIndex >= 0 ? remainder.slice(0, separatorIndex) : ""; - const targetPath = separatorIndex >= 0 ? remainder.slice(separatorIndex + 1) : ""; - if (!agentId || !targetPath) { - throw new Error(`Invalid --target auth-profiles target: ${value}`); - } - return { agentId, path: targetPath }; - } - return { - path: value.startsWith("openclaw:") ? value.slice("openclaw:".length) : value, - }; -} - -function createConfigSecretTarget(params: { - providerAlias: string; - path: string; - agentId?: string; - secretId: string; -}): SecretsPlanTarget { - const pathSegments = parseDotPath(params.path); - const normalizedPath = toDotPath(pathSegments); - if ( - pathSegments.length === 0 || - normalizedPath !== params.path || - pathSegments.some((segment) => FORBIDDEN_PATH_SEGMENTS.has(segment)) - ) { - throw new Error(`Invalid --target config path: ${params.path}`); - } - const resolved = resolveSecretPlanTargetByPath({ - configFile: params.agentId ? "auth-profiles.json" : "openclaw.json", - pathSegments, - }); - if (!resolved) { - throw new Error(`Unknown or unsupported Vault setup target path: ${params.path}`); - } - return { - type: resolved.targetType, - path: normalizedPath, - pathSegments, - ...(params.agentId ? { agentId: params.agentId } : {}), - ...(resolved.providerId ? { providerId: resolved.providerId } : {}), - ...(resolved.accountId ? { accountId: resolved.accountId } : {}), - ref: { - source: "exec", - provider: params.providerAlias, - id: params.secretId, - }, - }; + return pluginSecretRefSetup.parseTargetSpecifier("Vault", value); } function parseProviderKeyMappings(values: string[] | undefined): ProviderSecretMapping[] { @@ -318,7 +201,7 @@ function parseProviderKeyMappings(values: string[] | undefined): ProviderSecretM } const providerId = value.slice(0, separator).trim(); const secretId = value.slice(separator + 1).trim(); - assertValidModelProviderId("--provider-key", providerId); + pluginSecretRefSetup.assertValidModelProviderId("--provider-key", providerId); assertValidVaultSecretId(`--provider-key ${providerId}`, secretId); return { providerId, secretId }; }); @@ -371,53 +254,13 @@ function collectProviderSecrets(options: { return providerSecrets; } -function assertNoDuplicatePlanTargets(targets: SecretsPlanTarget[]): void { - const seen = new Set(); - for (const target of targets) { - const key = target.agentId - ? `auth-profiles:${target.agentId}:${target.path}` - : `openclaw:${target.path}`; - if (seen.has(key)) { - throw new Error(`Duplicate secret target path in Vault setup: ${target.path}`); - } - seen.add(key); - } -} - function buildPlan(params: { providerAlias: string; providerConfig: VaultExecProviderConfig; providerSecrets: ProviderSecretMapping[]; configTargetSecrets?: ConfigTargetSecretMapping[]; -}): SecretsApplyPlan { - const targets = [ - ...params.providerSecrets.map((entry) => - createModelApiKeyTarget({ - providerAlias: params.providerAlias, - providerId: entry.providerId, - secretId: entry.secretId, - }), - ), - ...(params.configTargetSecrets ?? []).map((entry) => - createConfigSecretTarget({ - providerAlias: params.providerAlias, - path: entry.path, - ...(entry.agentId ? { agentId: entry.agentId } : {}), - secretId: entry.secretId, - }), - ), - ]; - assertNoDuplicatePlanTargets(targets); - return { - version: 1, - protocolVersion: 1, - generatedAt: new Date().toISOString(), - generatedBy: "manual", - providerUpserts: { - [params.providerAlias]: params.providerConfig, - }, - targets, - }; +}) { + return pluginSecretRefSetup.buildPlan({ productName: "Vault", ...params }); } async function promptOptionalSecretId(label: string): Promise { diff --git a/scripts/plugin-sdk-surface-report.mjs b/scripts/plugin-sdk-surface-report.mjs index 2636ff22afe5..7b1e58e24452 100644 --- a/scripts/plugin-sdk-surface-report.mjs +++ b/scripts/plugin-sdk-surface-report.mjs @@ -177,7 +177,8 @@ export function readPluginSdkSurfaceBudgets(env = process.env) { // +3: channel DM policy factory and its account/patch callback contracts. // +1: typed owner-required error for session store path resolution. // +1: native approval messaging target resolver. - 4722, + // +1: shared plugin SecretRef setup plan helper. + 4723, env, ), publicFunctionExports: readPluginSdkSurfaceBudgetEnv( diff --git a/src/plugin-sdk/secret-ref-runtime.ts b/src/plugin-sdk/secret-ref-runtime.ts index 039284fa511c..7b4b3ea61983 100644 --- a/src/plugin-sdk/secret-ref-runtime.ts +++ b/src/plugin-sdk/secret-ref-runtime.ts @@ -1,5 +1,11 @@ // Narrow shared secret-ref helpers for plugin config and secret-contract paths. +import { + assertValidPluginModelProviderId, + assertValidPluginSecretProviderAlias, + buildPluginSecretRefSetupPlan, + parsePluginSecretTargetSpecifier, +} from "../secrets/plugin-setup-plan.js"; import { resolveSecretPlanTargetByPath as resolveSecretPlanTargetByPathInternal } from "../secrets/target-registry-query.js"; export { coerceSecretRef } from "../config/types.secrets.js"; @@ -7,6 +13,14 @@ export type { SecretInput, SecretRef } from "../config/types.secrets.js"; export { resolveSecretRefValues } from "../secrets/resolve.js"; export { applyResolvedAssignments, createResolverContext } from "../secrets/runtime-shared.js"; +/** Shared validation and apply-plan construction for plugin-owned SecretRef setup CLIs. */ +export const pluginSecretRefSetup = { + assertValidModelProviderId: assertValidPluginModelProviderId, + assertValidProviderAlias: assertValidPluginSecretProviderAlias, + buildPlan: buildPluginSecretRefSetupPlan, + parseTargetSpecifier: parsePluginSecretTargetSpecifier, +}; + export type ResolvedSecretPlanTarget = { targetType: string; providerId?: string; diff --git a/src/secrets/plugin-setup-plan.ts b/src/secrets/plugin-setup-plan.ts new file mode 100644 index 000000000000..86a9e37d4835 --- /dev/null +++ b/src/secrets/plugin-setup-plan.ts @@ -0,0 +1,180 @@ +/** Shared plan construction for plugin-owned SecretRef setup commands. */ +import { isValidAgentId } from "@openclaw/normalization-core/agent-id"; +import type { PluginIntegrationSecretProviderConfig, SecretRef } from "../config/types.secrets.js"; +import type { SecretsApplyPlan, SecretsPlanTarget } from "./plan.js"; +import { resolveSecretPlanTargetByPath } from "./target-registry-query.js"; + +type PluginSecretRefProviderMapping = { + providerId: string; + secretId: string; +}; + +type PluginSecretRefConfigTargetMapping = { + path: string; + agentId?: string; + secretId: string; +}; + +const SECRET_PROVIDER_ALIAS_PATTERN = /^[a-z][a-z0-9_-]{0,63}$/; +const MODEL_PROVIDER_ID_PATTERN = /^[A-Za-z0-9][A-Za-z0-9._:-]{0,127}$/; +const FORBIDDEN_PATH_SEGMENTS = new Set(["__proto__", "prototype", "constructor"]); + +function parseDotPath(pathname: string): string[] { + return pathname + .split(".") + .map((segment) => segment.trim()) + .filter((segment) => segment.length > 0); +} + +function toDotPath(segments: string[]): string { + return segments.join("."); +} + +export function assertValidPluginSecretProviderAlias(value: string): void { + if (!SECRET_PROVIDER_ALIAS_PATTERN.test(value)) { + throw new Error( + `Invalid provider alias "${value}". Use lowercase letters, numbers, underscores, or hyphens.`, + ); + } +} + +export function assertValidPluginModelProviderId(label: string, value: string): void { + if (!MODEL_PROVIDER_ID_PATTERN.test(value)) { + throw new Error(`Invalid ${label} model provider id: ${value}`); + } +} + +export function parsePluginSecretTargetSpecifier( + productName: string, + value: string, +): { path: string; agentId?: string } { + if (!value.startsWith("auth-profiles:")) { + return { + path: value.startsWith("openclaw:") ? value.slice("openclaw:".length) : value, + }; + } + const remainder = value.slice("auth-profiles:".length); + const separatorIndex = remainder.indexOf(":"); + const agentId = separatorIndex >= 0 ? remainder.slice(0, separatorIndex) : ""; + const targetPath = separatorIndex >= 0 ? remainder.slice(separatorIndex + 1) : ""; + if (!isValidAgentId(agentId) || !targetPath) { + throw new Error(`Invalid --target auth-profiles target for ${productName}: ${value}`); + } + return { agentId, path: targetPath }; +} + +function createPluginModelApiKeyTarget(params: { + providerAlias: string; + providerId: string; + secretId: string; +}): SecretsPlanTarget { + assertValidPluginModelProviderId("target", params.providerId); + return { + type: "models.providers.apiKey", + path: `models.providers.${params.providerId}.apiKey`, + pathSegments: ["models", "providers", params.providerId, "apiKey"], + providerId: params.providerId, + ref: { + source: "exec", + provider: params.providerAlias, + id: params.secretId, + }, + }; +} + +function createPluginConfigSecretTarget(params: { + productName: string; + providerAlias: string; + path: string; + agentId?: string; + secretId: string; +}): SecretsPlanTarget { + if (params.agentId && !isValidAgentId(params.agentId)) { + throw new Error(`Invalid ${params.productName} setup agent id: ${params.agentId}`); + } + const pathSegments = parseDotPath(params.path); + const normalizedPath = toDotPath(pathSegments); + if ( + pathSegments.length === 0 || + normalizedPath !== params.path || + pathSegments.some((segment) => FORBIDDEN_PATH_SEGMENTS.has(segment)) + ) { + throw new Error(`Invalid --target config path: ${params.path}`); + } + const resolved = resolveSecretPlanTargetByPath({ + configFile: params.agentId ? "auth-profiles.json" : "openclaw.json", + pathSegments, + }); + if (!resolved) { + throw new Error( + `Unknown or unsupported ${params.productName} setup target path: ${params.path}`, + ); + } + const ref: SecretRef = { + source: "exec", + provider: params.providerAlias, + id: params.secretId, + }; + return { + type: resolved.entry.targetType, + path: normalizedPath, + pathSegments, + ...(params.agentId ? { agentId: params.agentId } : {}), + ...(resolved.providerId ? { providerId: resolved.providerId } : {}), + ...(resolved.accountId ? { accountId: resolved.accountId } : {}), + ref, + }; +} + +export function buildPluginSecretRefSetupPlan(params: { + productName: string; + providerAlias: string; + providerConfig: PluginIntegrationSecretProviderConfig; + providerSecrets: PluginSecretRefProviderMapping[]; + configTargetSecrets?: PluginSecretRefConfigTargetMapping[]; + generatedAt?: string; +}): SecretsApplyPlan & { + providerUpserts: Record; +} { + assertValidPluginSecretProviderAlias(params.providerAlias); + const targets = [ + ...params.providerSecrets.map((entry) => + createPluginModelApiKeyTarget({ + providerAlias: params.providerAlias, + providerId: entry.providerId, + secretId: entry.secretId, + }), + ), + ...(params.configTargetSecrets ?? []).map((entry) => + createPluginConfigSecretTarget({ + productName: params.productName, + providerAlias: params.providerAlias, + path: entry.path, + ...(entry.agentId ? { agentId: entry.agentId } : {}), + secretId: entry.secretId, + }), + ), + ]; + const seen = new Set(); + for (const target of targets) { + const key = target.agentId + ? `auth-profiles:${target.agentId}:${target.path}` + : `openclaw:${target.path}`; + if (seen.has(key)) { + throw new Error( + `Duplicate secret target path in ${params.productName} setup: ${target.path}`, + ); + } + seen.add(key); + } + return { + version: 1, + protocolVersion: 1, + generatedAt: params.generatedAt ?? new Date().toISOString(), + generatedBy: "manual", + providerUpserts: { + [params.providerAlias]: params.providerConfig, + }, + targets, + }; +} diff --git a/src/secrets/resolve.ts b/src/secrets/resolve.ts index 7e36e92b2255..e4ad1143146e 100644 --- a/src/secrets/resolve.ts +++ b/src/secrets/resolve.ts @@ -529,12 +529,11 @@ async function resolveExecRefs(params: { }); } - const requestPayload = { + const input = JSON.stringify({ protocolVersion: 1, provider: params.providerName, ids, - }; - const input = JSON.stringify(requestPayload); + }); if (Buffer.byteLength(input, "utf8") > params.limits.maxBatchBytes) { throw providerResolutionError({ code: "SECRET_PROVIDER_INVALID",