From fb8589ebdb890afb6c5b203b5c782dfd90e7ea2f Mon Sep 17 00:00:00 2001 From: joshavant <830519+joshavant@users.noreply.github.com> Date: Sat, 25 Jul 2026 01:24:47 -0500 Subject: [PATCH] fix(onepassword): harden trusted op execution --- extensions/onepassword/npm-shrinkwrap.json | 443 +++++++++++++++++- .../onepassword/onepassword-op-path.d.ts | 19 + extensions/onepassword/onepassword-op-path.js | 185 +++++++- extensions/onepassword/package.json | 3 +- extensions/onepassword/src/op-client.test.ts | 18 +- extensions/onepassword/src/op-client.ts | 12 +- extensions/onepassword/src/op-path.test.ts | 164 +++++++ .../src/trusted-node.test-support.ts | 29 ++ pnpm-lock.yaml | 7 +- 9 files changed, 865 insertions(+), 15 deletions(-) create mode 100644 extensions/onepassword/src/op-path.test.ts create mode 100644 extensions/onepassword/src/trusted-node.test-support.ts diff --git a/extensions/onepassword/npm-shrinkwrap.json b/extensions/onepassword/npm-shrinkwrap.json index f078123f4269..71ec1ab20729 100644 --- a/extensions/onepassword/npm-shrinkwrap.json +++ b/extensions/onepassword/npm-shrinkwrap.json @@ -6,7 +6,448 @@ "packages": { "": { "name": "@openclaw/onepassword", - "version": "2026.7.2" + "version": "2026.7.2", + "dependencies": { + "@openclaw/fs-safe": "0.4.7", + "execa": "10.0.0" + } + }, + "node_modules/@isaacs/fs-minipass": { + "version": "4.0.1", + "resolved": "https://registry.npmjs.org/@isaacs/fs-minipass/-/fs-minipass-4.0.1.tgz", + "integrity": "sha512-wgm9Ehl2jpeqP3zw/7mo3kRHFp5MEDhqAdwy1fTGkHAwnkGOVsgpvQhL8B5n1qlb01jV3n/bI0ZfZp5lWA1k4w==", + "license": "ISC", + "optional": true, + "dependencies": { + "minipass": "^7.0.4" + }, + "engines": { + "node": ">=18.0.0" + } + }, + "node_modules/@openclaw/fs-safe": { + "version": "0.4.7", + "resolved": "https://registry.npmjs.org/@openclaw/fs-safe/-/fs-safe-0.4.7.tgz", + "integrity": "sha512-UO+FCB8vF+VJngOzZuVTdkOapX/gPml0HFEjRj2qIeEmBlCkGLHjRJ4CZWFziRtcq3g4YioWra/6J3iwEH1JaA==", + "license": "MIT", + "engines": { + "node": ">=22" + }, + "optionalDependencies": { + "jszip": "^3.10.1", + "tar": "7.5.20" + } + }, + "node_modules/@sec-ant/readable-stream": { + "version": "0.4.1", + "resolved": "https://registry.npmjs.org/@sec-ant/readable-stream/-/readable-stream-0.4.1.tgz", + "integrity": "sha512-831qok9r2t8AlxLko40y2ebgSDhenenCatLVeW/uBtnHPyhHOvG0C7TvfgecV+wHzIm5KUICgzmVpWS+IMEAeg==", + "license": "MIT" + }, + "node_modules/@sindresorhus/merge-streams": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/@sindresorhus/merge-streams/-/merge-streams-4.0.0.tgz", + "integrity": "sha512-tlqY9xq5ukxTUZBmoOp+m61cqwQD5pHJtFY3Mn8CA8ps6yghLH/Hw8UPdqg4OLmFW3IFlcXnQNmo/dh8HzXYIQ==", + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/chownr": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/chownr/-/chownr-3.0.0.tgz", + "integrity": "sha512-+IxzY9BZOQd/XuYPRmrvEVjF/nqj5kgT4kEq7VofrDoM1MxoRjEWkrCC3EtLi59TVawxTAn+orJwFQcrqEN1+g==", + "license": "BlueOak-1.0.0", + "optional": true, + "engines": { + "node": ">=18" + } + }, + "node_modules/core-util-is": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/core-util-is/-/core-util-is-1.0.3.tgz", + "integrity": "sha512-ZQBvi1DcpJ4GDqanjucZ2Hj3wEO5pZDS89BWbkcrvdxksJorwUDDZamX9ldFkp9aw2lmBDLgkObEA4DWNJ9FYQ==", + "license": "MIT", + "optional": true + }, + "node_modules/execa": { + "version": "10.0.0", + "resolved": "https://registry.npmjs.org/execa/-/execa-10.0.0.tgz", + "integrity": "sha512-Cxl6MKxB1dr1H0FHmiizJ+lavKF7pV+fcDZFyqMB8d5m7qUPm/OtZYcD5vPWePKxSnTQ57KuBd9mtdZ3oNCvyQ==", + "license": "MIT", + "dependencies": { + "@sindresorhus/merge-streams": "^4.0.0", + "figures": "^6.1.0", + "get-stream": "^9.0.1", + "human-signals": "^8.0.1", + "is-plain-obj": "^4.1.0", + "is-stream": "^4.0.1", + "npm-run-path": "^6.0.0", + "path-key": "^4.0.0", + "pretty-ms": "^9.3.0", + "signal-exit": "^4.1.0", + "strip-final-newline": "^4.0.0", + "which-command": "^0.1.0", + "yoctocolors": "^2.1.2" + }, + "engines": { + "node": ">=22" + }, + "funding": { + "url": "https://github.com/sindresorhus/execa?sponsor=1" + } + }, + "node_modules/figures": { + "version": "6.1.0", + "resolved": "https://registry.npmjs.org/figures/-/figures-6.1.0.tgz", + "integrity": "sha512-d+l3qxjSesT4V7v2fh+QnmFnUWv9lSpjarhShNTgBOfA0ttejbQUAlHLitbjkoRiDulW0OPoQPYIGhIC8ohejg==", + "license": "MIT", + "dependencies": { + "is-unicode-supported": "^2.0.0" + }, + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/get-stream": { + "version": "9.0.1", + "resolved": "https://registry.npmjs.org/get-stream/-/get-stream-9.0.1.tgz", + "integrity": "sha512-kVCxPF3vQM/N0B1PmoqVUqgHP+EeVjmZSQn+1oCRPxd2P21P2F19lIgbR3HBosbB1PUhOAoctJnfEn2GbN2eZA==", + "license": "MIT", + "dependencies": { + "@sec-ant/readable-stream": "^0.4.1", + "is-stream": "^4.0.1" + }, + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/human-signals": { + "version": "8.0.1", + "resolved": "https://registry.npmjs.org/human-signals/-/human-signals-8.0.1.tgz", + "integrity": "sha512-eKCa6bwnJhvxj14kZk5NCPc6Hb6BdsU9DZcOnmQKSnO1VKrfV0zCvtttPZUsBvjmNDn8rpcJfpwSYnHBjc95MQ==", + "license": "Apache-2.0", + "engines": { + "node": ">=18.18.0" + } + }, + "node_modules/immediate": { + "version": "3.0.6", + "resolved": "https://registry.npmjs.org/immediate/-/immediate-3.0.6.tgz", + "integrity": "sha512-XXOFtyqDjNDAQxVfYxuF7g9Il/IbWmmlQg2MYKOH8ExIT1qg6xc4zyS3HaEEATgs1btfzxq15ciUiY7gjSXRGQ==", + "license": "MIT", + "optional": true + }, + "node_modules/inherits": { + "version": "2.0.4", + "resolved": "https://registry.npmjs.org/inherits/-/inherits-2.0.4.tgz", + "integrity": "sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ==", + "license": "ISC", + "optional": true + }, + "node_modules/is-plain-obj": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/is-plain-obj/-/is-plain-obj-4.1.0.tgz", + "integrity": "sha512-+Pgi+vMuUNkJyExiMBt5IlFoMyKnr5zhJ4Uspz58WOhBF5QoIZkFyNHIbBAtHwzVAgk5RtndVNsDRN61/mmDqg==", + "license": "MIT", + "engines": { + "node": ">=12" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/is-stream": { + "version": "4.0.1", + "resolved": "https://registry.npmjs.org/is-stream/-/is-stream-4.0.1.tgz", + "integrity": "sha512-Dnz92NInDqYckGEUJv689RbRiTSEHCQ7wOVeALbkOz999YpqT46yMRIGtSNl2iCL1waAZSx40+h59NV/EwzV/A==", + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/is-unicode-supported": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/is-unicode-supported/-/is-unicode-supported-2.1.0.tgz", + "integrity": "sha512-mE00Gnza5EEB3Ds0HfMyllZzbBrmLOX3vfWoj9A9PEnTfratQ/BcaJOuMhnkhjXvb2+FkY3VuHqtAGpTPmglFQ==", + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/isarray": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/isarray/-/isarray-1.0.0.tgz", + "integrity": "sha512-VLghIWNM6ELQzo7zwmcg0NmTVyWKYjvIeM83yjp0wRDTmUnrM678fQbcKBo6n2CJEF0szoG//ytg+TKla89ALQ==", + "license": "MIT", + "optional": true + }, + "node_modules/jszip": { + "version": "3.10.1", + "resolved": "https://registry.npmjs.org/jszip/-/jszip-3.10.1.tgz", + "integrity": "sha512-xXDvecyTpGLrqFrvkrUSoxxfJI5AH7U8zxxtVclpsUtMCq4JQ290LY8AW5c7Ggnr/Y/oK+bQMbqK2qmtk3pN4g==", + "license": "(MIT OR GPL-3.0-or-later)", + "optional": true, + "dependencies": { + "lie": "~3.3.0", + "pako": "~1.0.2", + "readable-stream": "~2.3.6", + "setimmediate": "^1.0.5" + } + }, + "node_modules/lie": { + "version": "3.3.0", + "resolved": "https://registry.npmjs.org/lie/-/lie-3.3.0.tgz", + "integrity": "sha512-UaiMJzeWRlEujzAuw5LokY1L5ecNQYZKfmyZ9L7wDHb/p5etKaxXhohBcrw0EYby+G/NA52vRSN4N39dxHAIwQ==", + "license": "MIT", + "optional": true, + "dependencies": { + "immediate": "~3.0.5" + } + }, + "node_modules/minipass": { + "version": "7.1.3", + "resolved": "https://registry.npmjs.org/minipass/-/minipass-7.1.3.tgz", + "integrity": "sha512-tEBHqDnIoM/1rXME1zgka9g6Q2lcoCkxHLuc7ODJ5BxbP5d4c2Z5cGgtXAku59200Cx7diuHTOYfSBD8n6mm8A==", + "license": "BlueOak-1.0.0", + "optional": true, + "engines": { + "node": ">=16 || 14 >=14.17" + } + }, + "node_modules/minizlib": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/minizlib/-/minizlib-3.1.0.tgz", + "integrity": "sha512-KZxYo1BUkWD2TVFLr0MQoM8vUUigWD3LlD83a/75BqC+4qE0Hb1Vo5v1FgcfaNXvfXzr+5EhQ6ing/CaBijTlw==", + "license": "MIT", + "optional": true, + "dependencies": { + "minipass": "^7.1.2" + }, + "engines": { + "node": ">= 18" + } + }, + "node_modules/npm-run-path": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/npm-run-path/-/npm-run-path-6.0.0.tgz", + "integrity": "sha512-9qny7Z9DsQU8Ou39ERsPU4OZQlSTP47ShQzuKZ6PRXpYLtIFgl/DEBYEXKlvcEa+9tHVcK8CF81Y2V72qaZhWA==", + "license": "MIT", + "dependencies": { + "path-key": "^4.0.0", + "unicorn-magic": "^0.3.0" + }, + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/pako": { + "version": "1.0.11", + "resolved": "https://registry.npmjs.org/pako/-/pako-1.0.11.tgz", + "integrity": "sha512-4hLB8Py4zZce5s4yd9XzopqwVv/yGNhV1Bl8NTmCq1763HeK2+EwVTv+leGeL13Dnh2wfbqowVPXCIO0z4taYw==", + "license": "(MIT AND Zlib)", + "optional": true + }, + "node_modules/parse-ms": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/parse-ms/-/parse-ms-4.0.0.tgz", + "integrity": "sha512-TXfryirbmq34y8QBwgqCVLi+8oA3oWx2eAnSn62ITyEhEYaWRlVZ2DvMM9eZbMs/RfxPu/PK/aBLyGj4IrqMHw==", + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/path-key": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/path-key/-/path-key-4.0.0.tgz", + "integrity": "sha512-haREypq7xkM7ErfgIyA0z+Bj4AGKlMSdlQE2jvJo6huWD1EdkKYV+G/T4nq0YEF2vgTT8kqMFKo1uHn950r4SQ==", + "license": "MIT", + "engines": { + "node": ">=12" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/pretty-ms": { + "version": "9.3.0", + "resolved": "https://registry.npmjs.org/pretty-ms/-/pretty-ms-9.3.0.tgz", + "integrity": "sha512-gjVS5hOP+M3wMm5nmNOucbIrqudzs9v/57bWRHQWLYklXqoXKrVfYW2W9+glfGsqtPgpiz5WwyEEB+ksXIx3gQ==", + "license": "MIT", + "dependencies": { + "parse-ms": "^4.0.0" + }, + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/process-nextick-args": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/process-nextick-args/-/process-nextick-args-2.0.1.tgz", + "integrity": "sha512-3ouUOpQhtgrbOa17J7+uxOTpITYWaGP7/AhoR3+A+/1e9skrzelGi/dXzEYyvbxubEF6Wn2ypscTKiKJFFn1ag==", + "license": "MIT", + "optional": true + }, + "node_modules/readable-stream": { + "version": "2.3.8", + "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.8.tgz", + "integrity": "sha512-8p0AUk4XODgIewSi0l8Epjs+EVnWiK7NoDIEGU0HhE7+ZyY8D1IMY7odu5lRrFXGg71L15KG8QrPmum45RTtdA==", + "license": "MIT", + "optional": true, + "dependencies": { + "core-util-is": "~1.0.0", + "inherits": "~2.0.3", + "isarray": "~1.0.0", + "process-nextick-args": "~2.0.0", + "safe-buffer": "~5.1.1", + "string_decoder": "~1.1.1", + "util-deprecate": "~1.0.1" + } + }, + "node_modules/safe-buffer": { + "version": "5.1.2", + "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.1.2.tgz", + "integrity": "sha512-Gd2UZBJDkXlY7GbJxfsE8/nvKkUEU1G38c1siN6QP6a9PT9MmHB8GnpscSmMJSoF8LOIrt8ud/wPtojys4G6+g==", + "license": "MIT", + "optional": true + }, + "node_modules/setimmediate": { + "version": "1.0.5", + "resolved": "https://registry.npmjs.org/setimmediate/-/setimmediate-1.0.5.tgz", + "integrity": "sha512-MATJdZp8sLqDl/68LfQmbP8zKPLQNV6BIZoIgrscFDQ+RsvK/BxeDQOgyxKKoh0y/8h3BqVFnCqQ/gd+reiIXA==", + "license": "MIT", + "optional": true + }, + "node_modules/signal-exit": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-4.1.0.tgz", + "integrity": "sha512-bzyZ1e88w9O1iNJbKnOlvYTrWPDl46O1bG0D3XInv+9tkPrxrN8jUUTiFlDkkmKWgn1M6CfIA13SuGqOa9Korw==", + "license": "ISC", + "engines": { + "node": ">=14" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, + "node_modules/string_decoder": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.1.1.tgz", + "integrity": "sha512-n/ShnvDi6FHbbVfviro+WojiFzv+s8MPMHBczVePfUpDJLwoLT0ht1l4YwBCbi8pJAveEEdnkHyPyTP/mzRfwg==", + "license": "MIT", + "optional": true, + "dependencies": { + "safe-buffer": "~5.1.0" + } + }, + "node_modules/strip-final-newline": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/strip-final-newline/-/strip-final-newline-4.0.0.tgz", + "integrity": "sha512-aulFJcD6YK8V1G7iRB5tigAP4TsHBZZrOV8pjV++zdUwmeV8uzbY7yn6h9MswN62adStNZFuCIx4haBnRuMDaw==", + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/tar": { + "version": "7.5.20", + "resolved": "https://registry.npmjs.org/tar/-/tar-7.5.20.tgz", + "integrity": "sha512-9FcyK4PA6+WbzlTM9WhQm6vB5W7cP7dUiPsv1g7YDwEQnQ1CGpK3MGlKk/ITVWMk05kHZuBhmVhiv8LZoy/PFQ==", + "license": "BlueOak-1.0.0", + "optional": true, + "dependencies": { + "@isaacs/fs-minipass": "^4.0.0", + "chownr": "^3.0.0", + "minipass": "^7.1.2", + "minizlib": "^3.1.0", + "yallist": "^5.0.0" + }, + "engines": { + "node": ">=18" + } + }, + "node_modules/unicorn-magic": { + "version": "0.3.0", + "resolved": "https://registry.npmjs.org/unicorn-magic/-/unicorn-magic-0.3.0.tgz", + "integrity": "sha512-+QBBXBCvifc56fsbuxZQ6Sic3wqqc3WWaqxs58gvJrcOuN83HGTCwz3oS5phzU9LthRNE9VrJCFCLUgHeeFnfA==", + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/util-deprecate": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/util-deprecate/-/util-deprecate-1.0.2.tgz", + "integrity": "sha512-EPD5q1uXyFxJpCrLnCc1nHnq3gOa6DZBocAIiI2TaSCA7VCJ1UJDMagCzIkXNsUYfD1daK//LTEQ8xiIbrHtcw==", + "license": "MIT", + "optional": true + }, + "node_modules/which-command": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/which-command/-/which-command-0.1.0.tgz", + "integrity": "sha512-XZyoF5/5hZtXitIwzrU4NKK+Wtbb9aB9CezUEw2Q0wlYK8NUYQxC1rRXgNueYLtBAJwXIb+/tFVk4dozciNJMA==", + "license": "MIT", + "bin": { + "which-command": "cli.js" + }, + "engines": { + "node": ">=22" + }, + "funding": { + "url": "https://github.com/sindresorhus/which-command?sponsor=1" + } + }, + "node_modules/yallist": { + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/yallist/-/yallist-5.0.0.tgz", + "integrity": "sha512-YgvUTfwqyc7UXVMrB+SImsVYSmTS8X/tSrtdNZMImM+n7+QTriRXyXim0mBrTXNeqzVF0KWGgHPeiyViFFrNDw==", + "license": "BlueOak-1.0.0", + "optional": true, + "engines": { + "node": ">=18" + } + }, + "node_modules/yoctocolors": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/yoctocolors/-/yoctocolors-2.1.2.tgz", + "integrity": "sha512-CzhO+pFNo8ajLM2d2IW/R93ipy99LWjtwblvC1RsoSUMZgyLbYFr221TnSNT7GjGdYui6P459mw9JH/g/zW2ug==", + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } } } } diff --git a/extensions/onepassword/onepassword-op-path.d.ts b/extensions/onepassword/onepassword-op-path.d.ts index b9108789c3e4..9c45a98d7d44 100644 --- a/extensions/onepassword/onepassword-op-path.d.ts +++ b/extensions/onepassword/onepassword-op-path.d.ts @@ -1,4 +1,23 @@ +export function resolveTrustedExecutablePath(targetPath: string): Promise; + export function resolveTrustedOnePasswordCli(options?: { configuredPath?: string; pathEnv?: string; }): Promise; + +export const testing: { + isSafeWindowsDirectoryAclEntries( + entries: Array<{ rawRights: string }>, + allowChildCreation?: boolean, + ): boolean; + isSafeWindowsDirectoryAclSummary( + summary: string | undefined, + allowChildCreation?: boolean, + ): boolean; + isTrustedOwner( + stat: { uid?: number | null }, + permissions: { ownerTrusted?: boolean; ownerSid?: string }, + platform?: NodeJS.Platform, + allowWindowsTrustedInstaller?: boolean, + ): boolean; +}; diff --git a/extensions/onepassword/onepassword-op-path.js b/extensions/onepassword/onepassword-op-path.js index 744b58dbbebe..6a471da596fb 100644 --- a/extensions/onepassword/onepassword-op-path.js +++ b/extensions/onepassword/onepassword-op-path.js @@ -3,19 +3,128 @@ import fs from "node:fs/promises"; import path from "node:path"; import { inspectPathPermissions, safeStat } from "@openclaw/fs-safe/permissions"; +const WINDOWS_TRUSTED_INSTALLER_SID = + "s-1-5-80-956008885-3418522649-1831038044-1853292631-2271478464"; +const WINDOWS_SAFE_DIRECTORY_ACL_TOKENS = new Set([ + "AD", + "CI", + "GE", + "GR", + "I", + "IO", + "NP", + "OI", + "R", + "RA", + "RC", + "RD", + "REA", + "RX", + "S", + "WD", + "X", +]); +const WINDOWS_SAFE_EXECUTABLE_PARENT_ACL_TOKENS = new Set( + [...WINDOWS_SAFE_DIRECTORY_ACL_TOKENS].filter((token) => token !== "AD" && token !== "WD"), +); + function errorCode(error) { return error && typeof error === "object" && "code" in error ? error.code : undefined; } -function isTrustedOwner(stat) { - if (process.platform === "win32" || typeof process.getuid !== "function" || stat.uid == null) { - return true; +function isTrustedOwner( + stat, + permissions, + platform = process.platform, + allowWindowsTrustedInstaller = false, +) { + if (platform === "win32") { + return ( + permissions.ownerTrusted === true || + (allowWindowsTrustedInstaller && + permissions.ownerSid?.toLowerCase() === WINDOWS_TRUSTED_INSTALLER_SID) + ); + } + if (typeof process.getuid !== "function" || stat.uid == null) { + return false; } const uid = process.getuid(); return stat.uid === uid || stat.uid === 0; } -async function assertTrustedPath(targetPath) { +function isSafeWindowsDirectoryAclEntry(entry, allowChildCreation = true) { + if (!entry || typeof entry.rawRights !== "string") { + return false; + } + const tokens = [...entry.rawRights.matchAll(/\(([^)]+)\)/gu)].flatMap((match) => + (match[1] ?? "") + .split(",") + .map((token) => token.trim().toUpperCase()) + .filter(Boolean), + ); + // Inherit-only entries do not grant rights on the directory itself. Descendants are + // still inspected independently with their effective ACL and owner. + const safeTokens = allowChildCreation + ? WINDOWS_SAFE_DIRECTORY_ACL_TOKENS + : WINDOWS_SAFE_EXECUTABLE_PARENT_ACL_TOKENS; + return ( + tokens.includes("IO") || (tokens.length > 0 && tokens.every((token) => safeTokens.has(token))) + ); +} + +function isSafeWindowsDirectoryAclEntries(entries, allowChildCreation = true) { + return entries.every((entry) => isSafeWindowsDirectoryAclEntry(entry, allowChildCreation)); +} + +function isSafeWindowsDirectoryAclSummary(summary, allowChildCreation = true) { + if (summary === "trusted-only") { + return true; + } + if (typeof summary !== "string" || summary.length === 0) { + return false; + } + const entries = summary.split(", ").map((value) => { + const separatorIndex = value.lastIndexOf(":"); + const rawRights = separatorIndex > 0 ? value.slice(separatorIndex + 1) : ""; + return /^(?:\([^)]+\))+$/u.test(rawRights) ? { rawRights } : null; + }); + return ( + entries.length > 0 && + entries.every((entry) => entry !== null) && + isSafeWindowsDirectoryAclEntries(entries, allowChildCreation) + ); +} + +export const testing = { + isSafeWindowsDirectoryAclEntries, + isSafeWindowsDirectoryAclSummary, + isTrustedOwner, +}; + +async function readShebangInterpreter(targetPath) { + const handle = await fs.open(targetPath, "r"); + try { + const buffer = Buffer.alloc(4096); + const { bytesRead } = await handle.read(buffer, 0, buffer.length, 0); + if (bytesRead < 2 || buffer[0] !== 0x23 || buffer[1] !== 0x21) { + return undefined; + } + const newline = buffer.indexOf(0x0a, 2); + if (newline < 0) { + throw new Error(`script interpreter line is too long: ${targetPath}`); + } + const line = buffer.subarray(2, newline).toString("utf8").trim(); + const interpreter = line.split(/\s+/u, 1)[0]; + if (!interpreter || !path.isAbsolute(interpreter)) { + throw new Error(`script interpreter must be an absolute path: ${targetPath}`); + } + return interpreter; + } finally { + await handle.close(); + } +} + +async function assertTrustedPath(targetPath, validatedScripts = new Set()) { const resolvedPath = await fs.realpath(targetPath); const targetStat = await fs.stat(resolvedPath); if (!targetStat.isFile()) { @@ -25,27 +134,56 @@ async function assertTrustedPath(targetPath) { // 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. + const validatedEntries = []; let currentPath = resolvedPath; let first = true; for (;;) { + const before = await fs.lstat(currentPath); const [stat, permissions] = await Promise.all([ safeStat(currentPath), inspectPathPermissions(currentPath), ]); + const after = await fs.lstat(currentPath); if (!stat.ok || !permissions.ok || permissions.source === "unknown") { throw new Error(`permissions could not be verified: ${currentPath}`); } + if ( + before.isSymbolicLink() || + after.isSymbolicLink() || + stat.isSymlink || + permissions.isSymlink || + before.dev !== after.dev || + before.ino !== after.ino + ) { + throw new Error(`path changed during permission verification: ${currentPath}`); + } if ((first && stat.isDir) || (!first && !stat.isDir)) { throw new Error(`unexpected path type: ${currentPath}`); } - if (!isTrustedOwner(stat)) { + // TrustedInstaller legitimately owns Windows system directories. The requested executable + // itself still needs fs-safe's local-owner verdict. + const allowWindowsTrustedInstaller = !first; + if (!isTrustedOwner(stat, permissions, process.platform, allowWindowsTrustedInstaller)) { throw new Error(`path is not owned by the current user or root: ${currentPath}`); } const stickyDirectory = stat.isDir && permissions.mode != null && (permissions.mode & 0o1000) !== 0; if ((permissions.groupWritable || permissions.worldWritable) && !stickyDirectory) { - throw new Error(`path is writable by another user: ${currentPath}`); + const safeWindowsDirectory = + process.platform === "win32" && + stat.isDir && + isSafeWindowsDirectoryAclSummary( + permissions.aclSummary, + 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 + // next component; the executable parent is stricter to prevent side-loaded siblings. + if (!safeWindowsDirectory) { + throw new Error(`path is writable by another user: ${currentPath}`); + } } + validatedEntries.push({ path: currentPath, dev: after.dev, ino: after.ino }); const parentPath = path.dirname(currentPath); if (parentPath === currentPath) { @@ -54,9 +192,42 @@ async function assertTrustedPath(targetPath) { currentPath = parentPath; first = false; } + // Recheck from the trusted root toward the executable. The canonical path contains no links, + // and each verified directory prevents another account replacing the next component. + for (const entry of validatedEntries.toReversed()) { + const current = await fs.lstat(entry.path); + if (current.isSymbolicLink() || current.dev !== entry.dev || current.ino !== entry.ino) { + throw new Error(`path changed after permission verification: ${entry.path}`); + } + } + if (process.platform === "win32" && path.extname(resolvedPath).toLowerCase() !== ".exe") { + throw new Error(`Windows executable must be an .exe file: ${resolvedPath}`); + } + const interpreter = await readShebangInterpreter(resolvedPath); + if (interpreter) { + if (validatedScripts.has(resolvedPath)) { + throw new Error(`script interpreter cycle detected: ${resolvedPath}`); + } + validatedScripts.add(resolvedPath); + if (path.basename(interpreter).toLowerCase() === "env") { + throw new Error(`script interpreter may not use env indirection: ${resolvedPath}`); + } + const resolvedInterpreter = await assertTrustedPath(interpreter, validatedScripts); + // The kernel launches the literal shebang path, so aliases cannot rely on an unverified link. + if (resolvedInterpreter !== interpreter) { + throw new Error(`script interpreter path must be canonical: ${interpreter}`); + } + } return resolvedPath; } +export async function resolveTrustedExecutablePath(targetPath) { + if (!path.isAbsolute(targetPath)) { + throw new Error(`Executable path must be absolute: ${targetPath}`); + } + return await assertTrustedPath(targetPath); +} + export async function resolveTrustedOnePasswordCli(options = {}) { const configuredPath = options.configuredPath?.trim(); if (configuredPath && !path.isAbsolute(configuredPath)) { @@ -72,7 +243,7 @@ export async function resolveTrustedOnePasswordCli(options = {}) { let unsafeError; for (const candidate of candidates) { try { - return await assertTrustedPath(candidate); + return await resolveTrustedExecutablePath(candidate); } catch (error) { if (errorCode(error) === "ENOENT" || errorCode(error) === "ENOTDIR") { continue; diff --git a/extensions/onepassword/package.json b/extensions/onepassword/package.json index 9da997b9479d..4af9287ad8a7 100644 --- a/extensions/onepassword/package.json +++ b/extensions/onepassword/package.json @@ -5,7 +5,8 @@ "description": "1Password SecretRef resolver and audited agent secrets broker for OpenClaw", "type": "module", "dependencies": { - "@openclaw/fs-safe": "0.4.4" + "@openclaw/fs-safe": "0.4.7", + "execa": "10.0.0" }, "devDependencies": { "@openclaw/plugin-sdk": "workspace:*" diff --git a/extensions/onepassword/src/op-client.test.ts b/extensions/onepassword/src/op-client.test.ts index 229db9fa6346..d1f5b2dc71a4 100644 --- a/extensions/onepassword/src/op-client.test.ts +++ b/extensions/onepassword/src/op-client.test.ts @@ -4,6 +4,7 @@ import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { OnePasswordError } from "./errors.js"; import { OpClient } from "./op-client.js"; +import { createTrustedNodeFixture } from "./trusted-node.test-support.js"; type OpProcessRunner = NonNullable[0]["runner"]>; @@ -12,6 +13,7 @@ const tempDirs: string[] = []; describe("OpClient", () => { let root = ""; let opBin = ""; + let interpreter = ""; let tokenFile = ""; const fixtureAuth = ["fixture", "auth"].join("-"); const rightFixture = ["right", "fixture"].join("-"); @@ -20,9 +22,10 @@ describe("OpClient", () => { // openclaw-temp-dir: allow plugin tests cannot import the core-only tracker. root = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-onepassword-")); tempDirs.push(root); - opBin = path.join(root, "op"); + opBin = path.join(root, process.platform === "win32" ? "op.exe" : "op"); tokenFile = path.join(root, "service-account-token"); - await fs.writeFile(opBin, "#!/bin/sh\nexit 0\n", { mode: 0o700 }); + interpreter = createTrustedNodeFixture(root); + await fs.writeFile(opBin, `#!${interpreter}\nprocess.exit(0);\n`, { mode: 0o700 }); await fs.writeFile(tokenFile, ` ${fixtureAuth}\n`, { mode: 0o600 }); }); @@ -218,6 +221,17 @@ describe("OpClient", () => { expect(runner).not.toHaveBeenCalled(); }); + it("fails closed if the resolved binary disappears before a request", async () => { + const runner = vi.fn(); + const client = new OpClient({ opBin, tokenFile, timeoutMs: 1000, runner }); + await fs.rm(opBin); + + await expect( + client.getItem({ item: "Token", vault: "Automation", field: "credential" }), + ).rejects.toMatchObject({ code: "OP_NOT_FOUND" }); + expect(runner).not.toHaveBeenCalled(); + }); + it.runIf(process.platform !== "win32")( "does not read or pass the token to an executable in an unsafe directory", async () => { diff --git a/extensions/onepassword/src/op-client.ts b/extensions/onepassword/src/op-client.ts index d79780cba401..e307f66b3905 100644 --- a/extensions/onepassword/src/op-client.ts +++ b/extensions/onepassword/src/op-client.ts @@ -226,9 +226,17 @@ export class OpClient { } let trustedOpBin: string; try { - trustedOpBin = - (await resolveTrustedOnePasswordCli({ configuredPath: this.opBin })) ?? this.opBin; + // Config rejects relative opBin overrides, and resolveOpBinary expands PATH discovery to + // an absolute candidate, so every value reaching this boundary is an explicit path. + const resolved = await resolveTrustedOnePasswordCli({ configuredPath: this.opBin }); + if (!resolved) { + throw new OnePasswordError("OP_NOT_FOUND", "1Password CLI executable was not found"); + } + trustedOpBin = resolved; } catch (error) { + if (error instanceof OnePasswordError) { + throw error; + } throw new OnePasswordError("OP_NOT_FOUND", "1Password CLI executable is not trusted", { cause: error, }); diff --git a/extensions/onepassword/src/op-path.test.ts b/extensions/onepassword/src/op-path.test.ts new file mode 100644 index 000000000000..b01e3c24b345 --- /dev/null +++ b/extensions/onepassword/src/op-path.test.ts @@ -0,0 +1,164 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; +import { resolveTrustedExecutablePath, testing } from "../onepassword-op-path.js"; +import { createTrustedNodeFixture } from "./trusted-node.test-support.js"; + +describe("1Password CLI owner trust", () => { + it("copies the Node fixture without mutating the installed runtime", async () => { + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-op-path-")); + const before = await fs.stat(process.execPath); + try { + const fixture = createTrustedNodeFixture(tempDir); + const [sourceAfter, fixtureStat] = await Promise.all([ + fs.stat(process.execPath), + fs.stat(fixture), + ]); + expect(sourceAfter.mode).toBe(before.mode); + expect([fixtureStat.dev, fixtureStat.ino]).not.toEqual([sourceAfter.dev, sourceAfter.ino]); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } + }); + + it("requires generic executable paths to be absolute", async () => { + await expect(resolveTrustedExecutablePath("taskkill.exe")).rejects.toThrow( + "Executable path must be absolute", + ); + }); + + it.runIf(process.platform !== "win32")( + "canonicalizes an intentional executable symlink before trust validation", + async () => { + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-op-path-")); + const executable = path.join(tempDir, "op-real"); + const symlink = path.join(tempDir, "op"); + try { + const interpreter = createTrustedNodeFixture(tempDir); + await fs.writeFile(executable, `#!${interpreter}\nprocess.exit(0);\n`, { + mode: 0o700, + }); + await fs.symlink(executable, symlink); + await expect(resolveTrustedExecutablePath(symlink)).resolves.toBe( + await fs.realpath(executable), + ); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } + }, + ); + + it.runIf(process.platform !== "win32")("rejects env-indirected script interpreters", async () => { + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-op-path-")); + const executable = path.join(tempDir, "op"); + try { + await fs.writeFile(executable, "#!/usr/bin/env node\nprocess.exit(0);\n", { mode: 0o700 }); + await expect(resolveTrustedExecutablePath(executable)).rejects.toThrow(/env indirection/); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } + }); + + it.runIf(process.platform !== "win32")( + "rejects non-canonical script interpreter aliases", + async () => { + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-op-path-")); + const executable = path.join(tempDir, "op"); + try { + const canonicalInterpreter = createTrustedNodeFixture(tempDir); + const alias = path.join(tempDir, "node-alias"); + await fs.symlink(canonicalInterpreter, alias); + await fs.writeFile(executable, `#!${alias}\nprocess.exit(0);\n`, { mode: 0o700 }); + await expect(resolveTrustedExecutablePath(executable)).rejects.toThrow(/must be canonical/); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } + }, + ); + + it("requires verified Windows ownership", () => { + expect(testing.isTrustedOwner({}, { ownerTrusted: true }, "win32")).toBe(true); + expect(testing.isTrustedOwner({}, { ownerTrusted: false }, "win32")).toBe(false); + expect(testing.isTrustedOwner({}, {}, "win32")).toBe(false); + }); + + it("accepts the well-known Windows TrustedInstaller owner", () => { + expect( + testing.isTrustedOwner( + {}, + { + ownerTrusted: false, + ownerSid: "S-1-5-80-956008885-3418522649-1831038044-1853292631-2271478464", + }, + "win32", + true, + ), + ).toBe(true); + expect( + testing.isTrustedOwner( + {}, + { + ownerTrusted: false, + ownerSid: "S-1-5-80-956008885-3418522649-1831038044-1853292631-2271478464", + }, + "win32", + ), + ).toBe(false); + }); + + it("accepts only additive, read-only, or inherit-only untrusted directory rights", () => { + expect( + testing.isSafeWindowsDirectoryAclEntries([ + { rawRights: "(OI)(CI)(RX)" }, + { rawRights: "(CI)(AD)" }, + { rawRights: "(CI)(IO)(WD)" }, + { rawRights: "(OI)(CI)(IO)(F)" }, + ]), + ).toBe(true); + expect( + testing.isSafeWindowsDirectoryAclEntries([ + { rawRights: "(I)(CI)(WD,AD)" }, + { rawRights: "(I)(GR,GE)" }, + ]), + ).toBe(true); + expect(testing.isSafeWindowsDirectoryAclEntries([])).toBe(true); + expect(testing.isSafeWindowsDirectoryAclEntries([{ rawRights: "(M)" }])).toBe(false); + expect(testing.isSafeWindowsDirectoryAclEntries([{ rawRights: "unknown" }])).toBe(false); + }); + + it("rejects child creation rights beside the executable", () => { + expect(testing.isSafeWindowsDirectoryAclEntries([{ rawRights: "(RX)" }], false)).toBe(true); + expect(testing.isSafeWindowsDirectoryAclEntries([{ rawRights: "(CI)(AD)" }], false)).toBe( + false, + ); + expect(testing.isSafeWindowsDirectoryAclEntries([{ rawRights: "(CI)(WD)" }], false)).toBe( + false, + ); + expect(testing.isSafeWindowsDirectoryAclEntries([{ rawRights: "(CI)(IO)(WD)" }], false)).toBe( + true, + ); + }); + + it("uses the verified fs-safe ACL summary for Windows directory rights", () => { + expect(testing.isSafeWindowsDirectoryAclSummary("trusted-only")).toBe(true); + expect( + testing.isSafeWindowsDirectoryAclSummary( + "BUILTIN\\Users:(RX), CREATOR OWNER:(OI)(CI)(IO)(F)", + ), + ).toBe(true); + expect(testing.isSafeWindowsDirectoryAclSummary("BUILTIN\\Users:(M)")).toBe(false); + expect(testing.isSafeWindowsDirectoryAclSummary("BUILTIN\\Users:unknown")).toBe(false); + expect(testing.isSafeWindowsDirectoryAclSummary(undefined)).toBe(false); + }); + + it.runIf(typeof process.getuid === "function")( + "accepts only the current POSIX uid or root", + () => { + const uid = process.getuid?.() ?? 1; + expect(testing.isTrustedOwner({ uid }, {}, "linux")).toBe(true); + expect(testing.isTrustedOwner({ uid: 0 }, {}, "linux")).toBe(true); + expect(testing.isTrustedOwner({ uid: uid + 1 }, {}, "linux")).toBe(false); + }, + ); +}); diff --git a/extensions/onepassword/src/trusted-node.test-support.ts b/extensions/onepassword/src/trusted-node.test-support.ts new file mode 100644 index 000000000000..d7ae1d193e7a --- /dev/null +++ b/extensions/onepassword/src/trusted-node.test-support.ts @@ -0,0 +1,29 @@ +import fs from "node:fs"; +import path from "node:path"; + +function copyExecutable(source: string, target: string): void { + fs.copyFileSync(source, target); + fs.chmodSync(target, 0o700); +} + +/** Builds a user-owned Node layout whose executable and relative libnode path are both trusted. */ +export function createTrustedNodeFixture(directory: string): string { + const runtimeRoot = path.join(directory, "node-runtime"); + const binDir = path.join(runtimeRoot, "bin"); + const target = path.join(binDir, process.platform === "win32" ? "node.exe" : "node"); + if (fs.existsSync(target)) { + return fs.realpathSync(target); + } + fs.mkdirSync(binDir, { recursive: true, mode: 0o700 }); + copyExecutable(process.execPath, target); + + const sourceLibDir = path.resolve(path.dirname(process.execPath), "..", "lib"); + if (fs.existsSync(sourceLibDir)) { + const libDir = path.join(runtimeRoot, "lib"); + for (const name of fs.readdirSync(sourceLibDir).filter((entry) => /^libnode[.]/u.test(entry))) { + fs.mkdirSync(libDir, { recursive: true, mode: 0o700 }); + copyExecutable(fs.realpathSync(path.join(sourceLibDir, name)), path.join(libDir, name)); + } + } + return fs.realpathSync(target); +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 46b34e41402b..1615fca18a61 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1425,8 +1425,11 @@ importers: extensions/onepassword: dependencies: '@openclaw/fs-safe': - specifier: 0.4.4 - version: 0.4.4 + specifier: 0.4.7 + version: 0.4.7 + execa: + specifier: 10.0.0 + version: 10.0.0 devDependencies: '@openclaw/plugin-sdk': specifier: workspace:*