From 459db523e23edd49c9ea5131f2243c43e0a0f779 Mon Sep 17 00:00:00 2001 From: galz10 Date: Mon, 16 Mar 2026 15:07:51 -0700 Subject: [PATCH] fix(core): secure shell execution with AST validation Replaces simplistic prefix-matching for shell command policies with robust Abstract Syntax Tree (AST) parsing using `bash-parser`. Previously, policies for shell tools only checked if the command string started with an allowed prefix (e.g., `echo`), allowing trivial bypasses via shell operators like `&&` or `;` (e.g., `echo "ok" && rm -rf /`). This update secures the execution pipeline by parsing the shell string and validating *every* extracted sub-command against the allowed policies. Key changes: - Integrated `bash-parser` to synchronously extract executable commands from pipelines, lists, and subshells. - Updated `doesToolInvocationMatch` to enforce policy on all extracted sub-commands instead of just the string prefix. - Enforced `coreTools` validation at execution time within `ShellTool` to prevent bypasses when tools are configured via `settings.json`. - Updated the CLI `useShellCommandProcessor` to run human-input commands through the AST `PolicyEngine` check before spawning the process. - Fixed asynchronous test flakiness in the CLI package caused by the new policy enforcement. --- package-lock.json | 263 +++++++++++++++++- .../ui/hooks/shellCommandProcessor.test.tsx | 54 ++-- .../cli/src/ui/hooks/shellCommandProcessor.ts | 40 +++ packages/core/package.json | 2 + packages/core/src/tools/shell.test.ts | 17 +- packages/core/src/tools/shell.ts | 29 ++ .../core/src/utils/shell-ast-parser.test.ts | 44 +++ packages/core/src/utils/shell-ast-parser.ts | 84 ++++++ packages/core/src/utils/tool-utils.ts | 85 ++++-- 9 files changed, 557 insertions(+), 61 deletions(-) create mode 100644 packages/core/src/utils/shell-ast-parser.test.ts create mode 100644 packages/core/src/utils/shell-ast-parser.ts diff --git a/package-lock.json b/package-lock.json index 3757403f78..b16bf87787 100644 --- a/package-lock.json +++ b/package-lock.json @@ -486,7 +486,8 @@ "version": "2.11.0", "resolved": "https://registry.npmjs.org/@bufbuild/protobuf/-/protobuf-2.11.0.tgz", "integrity": "sha512-sBXGT13cpmPR5BMgHE6UEEfEaShh5Ror6rfN3yEK5si7QVrtZg8LEPQb0VVhiLRUslD2yLnXtnRzG035J/mZXQ==", - "license": "(Apache-2.0 AND BSD-3-Clause)" + "license": "(Apache-2.0 AND BSD-3-Clause)", + "peer": true }, "node_modules/@bundled-es-modules/cookie": { "version": "2.0.1", @@ -1489,6 +1490,7 @@ "resolved": "https://registry.npmjs.org/@grpc/grpc-js/-/grpc-js-1.13.4.tgz", "integrity": "sha512-GsFaMXCkMqkKIvwCQjCrwH+GHbPKBjhwo/8ZuUkWHqbI73Kky9I+pQltrlT0+MWpedCoosda53lgjYfyEPgxBg==", "license": "Apache-2.0", + "peer": true, "dependencies": { "@grpc/proto-loader": "^0.7.13", "@js-sdsl/ordered-map": "^4.4.2" @@ -5416,6 +5418,12 @@ "integrity": "sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q==", "license": "Python-2.0" }, + "node_modules/arity-n": { + "version": "1.0.4", + "resolved": "https://registry.npmjs.org/arity-n/-/arity-n-1.0.4.tgz", + "integrity": "sha512-fExL2kFDC1Q2DUOx3whE/9KoN66IzkY4b4zUHUBFM1ojEYjZZYDcUW3bek/ufGionX9giIKDC5redH2IlGqcQQ==", + "license": "MIT" + }, "node_modules/array-buffer-byte-length": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/array-buffer-byte-length/-/array-buffer-byte-length-1.0.2.tgz", @@ -5466,6 +5474,27 @@ "url": "https://github.com/sponsors/ljharb" } }, + "node_modules/array-last": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/array-last/-/array-last-1.3.0.tgz", + "integrity": "sha512-eOCut5rXlI6aCOS7Z7kCplKRKyiFQ6dHFBem4PwlwKeNFk2/XxTrhRh5T9PyaEWGy/NHTZWbY+nsZlNFJu9rYg==", + "license": "MIT", + "dependencies": { + "is-number": "^4.0.0" + }, + "engines": { + "node": ">=0.10.0" + } + }, + "node_modules/array-last/node_modules/is-number": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/is-number/-/is-number-4.0.0.tgz", + "integrity": "sha512-rSklcAIlf1OmFdyAqbnWTLVelsQ58uvZ66S/ZyawjWqIviTWCjg2PzVGw8WUA+nNuPTqb4wgA+NszrJ+08LlgQ==", + "license": "MIT", + "engines": { + "node": ">=0.10.0" + } + }, "node_modules/array-timsort": { "version": "1.0.3", "resolved": "https://registry.npmjs.org/array-timsort/-/array-timsort-1.0.3.tgz", @@ -5745,6 +5774,15 @@ } } }, + "node_modules/babylon": { + "version": "6.18.0", + "resolved": "https://registry.npmjs.org/babylon/-/babylon-6.18.0.tgz", + "integrity": "sha512-q/UEjfGJ2Cm3oKV71DJz9d25TPnq5rhBVL2Q4fA5wcC3jcrdn7+SssEybFIxwAvvP+YCsCYNKughoF33GxgycQ==", + "license": "MIT", + "bin": { + "babylon": "bin/babylon.js" + } + }, "node_modules/balanced-match": { "version": "4.0.4", "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-4.0.4.tgz", @@ -5861,6 +5899,47 @@ ], "license": "MIT" }, + "node_modules/bash-parser": { + "version": "0.5.0", + "resolved": "https://registry.npmjs.org/bash-parser/-/bash-parser-0.5.0.tgz", + "integrity": "sha512-AQR43o4W4sj4Jf+oy4cFtGgyBps4B+MYnJg6Xds8VVC7yomFtQekhOORQNHfQ8D6YJ0XENykr3TpxMn3rUtgeg==", + "license": "MIT", + "dependencies": { + "array-last": "^1.1.1", + "babylon": "^6.9.1", + "compose-function": "^3.0.3", + "curry": "^1.2.0", + "deep-freeze": "0.0.1", + "filter-iterator": "0.0.1", + "filter-obj": "^1.1.0", + "has-own-property": "^0.1.0", + "identity-function": "^1.0.0", + "iterable-lookahead": "^1.0.0", + "iterable-transform-replace": "^1.1.1", + "magic-string": "^0.16.0", + "map-iterable": "^1.0.1", + "map-obj": "^2.0.0", + "object-pairs": "^0.1.0", + "object-values": "^1.0.0", + "reverse-arguments": "^1.0.0", + "shell-quote-word": "^1.0.1", + "to-pascal-case": "^1.0.0", + "transform-spread-iterable": "^1.1.0", + "unescape-js": "^1.0.5" + }, + "engines": { + "node": ">=4" + } + }, + "node_modules/bash-parser/node_modules/magic-string": { + "version": "0.16.0", + "resolved": "https://registry.npmjs.org/magic-string/-/magic-string-0.16.0.tgz", + "integrity": "sha512-c4BEos3y6G2qO0B9X7K0FVLOPT9uGrjYwYRLFmDqyl5YMboUviyecnXWp94fJTSMwPw2/sf+CEYt5AGpmklkkQ==", + "license": "MIT", + "dependencies": { + "vlq": "^0.2.1" + } + }, "node_modules/basic-ftp": { "version": "5.2.0", "resolved": "https://registry.npmjs.org/basic-ftp/-/basic-ftp-5.2.0.tgz", @@ -6664,6 +6743,15 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/compose-function": { + "version": "3.0.3", + "resolved": "https://registry.npmjs.org/compose-function/-/compose-function-3.0.3.tgz", + "integrity": "sha512-xzhzTJ5eC+gmIzvZq+C3kCJHsp9os6tJkrigDRZclyGtOKINbZtE8n1Tzmeh32jW+BUDPbvZpibwvJHBLGMVwg==", + "license": "MIT", + "dependencies": { + "arity-n": "^1.0.4" + } + }, "node_modules/config-chain": { "version": "1.1.13", "resolved": "https://registry.npmjs.org/config-chain/-/config-chain-1.1.13.tgz", @@ -6909,6 +6997,11 @@ "devOptional": true, "license": "MIT" }, + "node_modules/curry": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/curry/-/curry-1.2.0.tgz", + "integrity": "sha512-PAdmqPH2DUYTCc/aknv6RxRxmqdRHclvbz+wP8t1Xpg2Nu13qg+oLb6/5iFoDmf4dbmC9loYoy9PwwGbFt/AqA==" + }, "node_modules/data-uri-to-buffer": { "version": "4.0.1", "resolved": "https://registry.npmjs.org/data-uri-to-buffer/-/data-uri-to-buffer-4.0.1.tgz", @@ -7034,6 +7127,12 @@ "node": ">=4.0.0" } }, + "node_modules/deep-freeze": { + "version": "0.0.1", + "resolved": "https://registry.npmjs.org/deep-freeze/-/deep-freeze-0.0.1.tgz", + "integrity": "sha512-Z+z8HiAvsGwmjqlphnHW5oz6yWlOwu6EQfFTjmeTWlDeda3FS2yv3jhq35TX/ewmsnqB+RX2IdsIOyjJCQN5tg==", + "license": "public domain" + }, "node_modules/deep-is": { "version": "0.1.4", "resolved": "https://registry.npmjs.org/deep-is/-/deep-is-0.1.4.tgz", @@ -7411,7 +7510,8 @@ "version": "0.0.1581282", "resolved": "https://registry.npmjs.org/devtools-protocol/-/devtools-protocol-0.0.1581282.tgz", "integrity": "sha512-nv7iKtNZQshSW2hKzYNr46nM/Cfh5SEvE2oV0/SEGgc9XupIY5ggf84Cz8eJIkBce7S3bmTAauFD6aysMpnqsQ==", - "license": "BSD-3-Clause" + "license": "BSD-3-Clause", + "peer": true }, "node_modules/dezalgo": { "version": "1.0.4", @@ -8830,6 +8930,20 @@ "node": ">=8" } }, + "node_modules/filter-iterator": { + "version": "0.0.1", + "resolved": "https://registry.npmjs.org/filter-iterator/-/filter-iterator-0.0.1.tgz", + "integrity": "sha512-v4lhL7Qa8XpbW3LN46CEnmhGk3eHZwxfNl5at20aEkreesht4YKb/Ba3BUIbnPhAC/r3dmu7ABaGk6MAvh2alA==" + }, + "node_modules/filter-obj": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/filter-obj/-/filter-obj-1.1.0.tgz", + "integrity": "sha512-8rXg1ZnX7xzy2NGDVkBVaAy+lSlPNwad13BtgSlLuxfIslyt5Vg64U7tFcCt4WS1R0hvtnQybT/IyCkGZ3DpXQ==", + "license": "MIT", + "engines": { + "node": ">=0.10.0" + } + }, "node_modules/finalhandler": { "version": "2.1.1", "resolved": "https://registry.npmjs.org/finalhandler/-/finalhandler-2.1.1.tgz", @@ -9725,6 +9839,12 @@ "node": ">=8" } }, + "node_modules/has-own-property": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/has-own-property/-/has-own-property-0.1.0.tgz", + "integrity": "sha512-14qdBKoonU99XDhWcFKZTShK+QV47qU97u8zzoVo9cL5TZ3BmBHXogItSt9qJjR0KUMFRhcCW8uGIGl8nkl7Aw==", + "license": "MIT" + }, "node_modules/has-property-descriptors": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/has-property-descriptors/-/has-property-descriptors-1.0.2.tgz", @@ -10014,6 +10134,12 @@ "node": ">=0.10.0" } }, + "node_modules/identity-function": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/identity-function/-/identity-function-1.0.0.tgz", + "integrity": "sha512-kNrgUK0qI+9qLTBidsH85HjDLpZfrrS0ElquKKe/fJFdB3D7VeKdXXEvOPDUHSHOzdZKCAAaQIWWyp0l2yq6pw==", + "license": "public domain" + }, "node_modules/ignore": { "version": "5.3.2", "resolved": "https://registry.npmjs.org/ignore/-/ignore-5.3.2.tgz", @@ -10533,6 +10659,15 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/is-iterable": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/is-iterable/-/is-iterable-1.1.1.tgz", + "integrity": "sha512-EdOZCr0NsGE00Pot+x1ZFx9MJK3C6wy91geZpXwvwexDLJvA4nzYyZf7r+EIwSeVsOLDdBz7ATg9NqKTzuNYuQ==", + "license": "MIT", + "engines": { + "node": ">= 4" + } + }, "node_modules/is-map": { "version": "2.0.3", "resolved": "https://registry.npmjs.org/is-map/-/is-map-2.0.3.tgz", @@ -10938,6 +11073,24 @@ "url": "https://bevry.me/fund" } }, + "node_modules/iterable-lookahead": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/iterable-lookahead/-/iterable-lookahead-1.0.0.tgz", + "integrity": "sha512-hJnEP2Xk4+44DDwJqUQGdXal5VbyeWLaPyDl2AQc242Zr7iqz4DgpQOrEzglWVMGHMDCkguLHEKxd1+rOsmgSQ==", + "license": "MIT", + "engines": { + "node": ">=4" + } + }, + "node_modules/iterable-transform-replace": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/iterable-transform-replace/-/iterable-transform-replace-1.2.0.tgz", + "integrity": "sha512-AVCCj7CTUifWQ0ubraDgx5/e6tOWaL5qh/C8BDTjH0GuhNyFMCSsSmDtYpa4Y3ReAAQNSjUWfQ+ojhmjX10pdQ==", + "license": "MIT", + "dependencies": { + "curry": "^1.2.0" + } + }, "node_modules/iterator.prototype": { "version": "1.1.5", "resolved": "https://registry.npmjs.org/iterator.prototype/-/iterator.prototype-1.1.5.tgz", @@ -11800,6 +11953,28 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/map-iterable": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/map-iterable/-/map-iterable-1.0.1.tgz", + "integrity": "sha512-siKFftph+ka2jWt8faiOWFzKP+eEuXrHuhYBitssJ5zJm209FCw5JBnaNLDiaCCb/CYZmxprdM6P7p16nA6YRA==", + "license": "MIT", + "dependencies": { + "curry": "^1.2.0", + "is-iterable": "^1.1.0" + }, + "engines": { + "node": ">=4" + } + }, + "node_modules/map-obj": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/map-obj/-/map-obj-2.0.0.tgz", + "integrity": "sha512-TzQSV2DiMYgoF5RycneKVUzIa9bQsj/B3tTgsE3dOGqlzHnGIDaC7XBE7grnA+8kZPnfqSGFe95VHc2oc0VFUQ==", + "license": "MIT", + "engines": { + "node": ">=4" + } + }, "node_modules/markdown-it": { "version": "14.1.1", "resolved": "https://registry.npmjs.org/markdown-it/-/markdown-it-14.1.1.tgz", @@ -12716,6 +12891,21 @@ "node": ">= 0.4" } }, + "node_modules/object-pairs": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/object-pairs/-/object-pairs-0.1.0.tgz", + "integrity": "sha512-3ECr6K831I4xX/Mduxr9UC+HPOz/d6WKKYj9p4cmC8Lg8p7g8gitzsxNX5IWlSIgFWN/a4JgrJaoAMKn20oKwA==", + "license": "MIT" + }, + "node_modules/object-values": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/object-values/-/object-values-1.0.0.tgz", + "integrity": "sha512-+8hwcz/JnQ9EpLIXzN0Rs7DLsBpJNT/xYehtB/jU93tHYr5BFEO8E+JGQNOSqE7opVzz5cGksKFHt7uUJVLSjQ==", + "license": "MIT", + "engines": { + "node": ">=0.10.0" + } + }, "node_modules/object.assign": { "version": "4.1.7", "resolved": "https://registry.npmjs.org/object.assign/-/object.assign-4.1.7.tgz", @@ -14321,6 +14511,12 @@ "node": ">=0.10.0" } }, + "node_modules/reverse-arguments": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/reverse-arguments/-/reverse-arguments-1.0.0.tgz", + "integrity": "sha512-/x8uIPdTafBqakK0TmPNJzgkLP+3H+yxpUJhCQHsLBg1rYEVNR2D8BRYNWQhVBjyOd7oo1dZRVzIkwMY2oqfYQ==", + "license": "MIT" + }, "node_modules/rfdc": { "version": "1.4.1", "resolved": "https://registry.npmjs.org/rfdc/-/rfdc-1.4.1.tgz", @@ -14825,6 +15021,12 @@ "url": "https://github.com/sponsors/ljharb" } }, + "node_modules/shell-quote-word": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/shell-quote-word/-/shell-quote-word-1.0.1.tgz", + "integrity": "sha512-lT297f1WLAdq0A4O+AknIFRP6kkiI3s8C913eJ0XqBxJbZPGWUNkRQk2u8zk4bEAjUJ5i+fSLwB6z1HzeT+DEg==", + "license": "MIT" + }, "node_modules/side-channel": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/side-channel/-/side-channel-1.1.0.tgz", @@ -15279,6 +15481,11 @@ "node": ">=8" } }, + "node_modules/string.fromcodepoint": { + "version": "0.2.1", + "resolved": "https://registry.npmjs.org/string.fromcodepoint/-/string.fromcodepoint-0.2.1.tgz", + "integrity": "sha512-n69H31OnxSGSZyZbgBlvYIXlrMhJQ0dQAX1js1QDhpaUH6zmU3QYlj07bCwCNlPOu3oRXIubGPl2gDGnHsiCqg==" + }, "node_modules/string.prototype.matchall": { "version": "4.0.12", "resolved": "https://registry.npmjs.org/string.prototype.matchall/-/string.prototype.matchall-4.0.12.tgz", @@ -16079,6 +16286,21 @@ "node": ">=14.14" } }, + "node_modules/to-no-case": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/to-no-case/-/to-no-case-1.0.2.tgz", + "integrity": "sha512-Z3g735FxuZY8rodxV4gH7LxClE4H0hTIyHNIHdk+vpQxjLm0cwnKXq/OFVZ76SOQmto7txVcwSCwkU5kqp+FKg==", + "license": "MIT" + }, + "node_modules/to-pascal-case": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/to-pascal-case/-/to-pascal-case-1.0.0.tgz", + "integrity": "sha512-QGMWHqM6xPrcQW57S23c5/3BbYb0Tbe9p+ur98ckRnGDwD4wbbtDiYI38CfmMKNB5Iv0REjs5SNDntTwvDxzZA==", + "license": "MIT", + "dependencies": { + "to-space-case": "^1.0.0" + } + }, "node_modules/to-regex-range": { "version": "5.0.1", "resolved": "https://registry.npmjs.org/to-regex-range/-/to-regex-range-5.0.1.tgz", @@ -16092,6 +16314,15 @@ "node": ">=8.0" } }, + "node_modules/to-space-case": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/to-space-case/-/to-space-case-1.0.0.tgz", + "integrity": "sha512-rLdvwXZ39VOn1IxGL3V6ZstoTbwLRckQmn/U8ZDLuWwIXNpuZDhQ3AiRUlhTbOXFVE9C+dR51wM0CBDhk31VcA==", + "license": "MIT", + "dependencies": { + "to-no-case": "^1.0.0" + } + }, "node_modules/toidentifier": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/toidentifier/-/toidentifier-1.0.1.tgz", @@ -16101,6 +16332,15 @@ "node": ">=0.6" } }, + "node_modules/transform-spread-iterable": { + "version": "1.4.1", + "resolved": "https://registry.npmjs.org/transform-spread-iterable/-/transform-spread-iterable-1.4.1.tgz", + "integrity": "sha512-/GnF26X3zC8wfWyRzvuXX/Vb31TrU3Rwipmr4MC5hTi6X/yOXxXUSw4+pcHmKJ2+0KRrcS21YWZw77ukhVJBdQ==", + "license": "MIT", + "dependencies": { + "curry": "^1.2.0" + } + }, "node_modules/tree-dump": { "version": "1.0.3", "resolved": "https://registry.npmjs.org/tree-dump/-/tree-dump-1.0.3.tgz", @@ -16247,7 +16487,6 @@ "version": "2.8.1", "resolved": "https://registry.npmjs.org/tslib/-/tslib-2.8.1.tgz", "integrity": "sha512-oJFu94HQb+KVduSUQL7wnpmqnfmLsOA/nAh6b6EH0wCEoK0/mPeXU6c3wKDV83MkOuHPRHtSXKKU99IBazS/2w==", - "dev": true, "license": "0BSD", "peer": true }, @@ -16503,6 +16742,15 @@ "integrity": "sha512-iwDZqg0QAGrg9Rav5H4n0M64c3mkR59cJ6wQp+7C4nI0gsmExaedaYLNO44eT4AtBBwjbTiGPMlt2Md0T9H9JQ==", "license": "MIT" }, + "node_modules/unescape-js": { + "version": "1.1.4", + "resolved": "https://registry.npmjs.org/unescape-js/-/unescape-js-1.1.4.tgz", + "integrity": "sha512-42SD8NOQEhdYntEiUQdYq/1V/YHwr1HLwlHuTJB5InVVdOSbgI6xu8jK5q65yIzuFCfczzyDF/7hbGzVbyCw0g==", + "license": "MIT", + "dependencies": { + "string.fromcodepoint": "^0.2.1" + } + }, "node_modules/unicorn-magic": { "version": "0.1.0", "resolved": "https://registry.npmjs.org/unicorn-magic/-/unicorn-magic-0.1.0.tgz", @@ -16853,6 +17101,12 @@ "url": "https://github.com/sponsors/jonschlinkert" } }, + "node_modules/vlq": { + "version": "0.2.3", + "resolved": "https://registry.npmjs.org/vlq/-/vlq-0.2.3.tgz", + "integrity": "sha512-DRibZL6DsNhIgYQ+wNdWDL2SL3bKPlVrRiBqV5yuMm++op8W4kGFtaQfCs4KEJn0wBZcHVHJ3eoywX8983k1ow==", + "license": "MIT" + }, "node_modules/web-streams-polyfill": { "version": "3.3.3", "resolved": "https://registry.npmjs.org/web-streams-polyfill/-/web-streams-polyfill-3.3.3.tgz", @@ -17759,6 +18013,7 @@ "@xterm/headless": "5.5.0", "ajv": "^8.17.1", "ajv-formats": "^3.0.0", + "bash-parser": "^0.5.0", "chardet": "^2.1.0", "diff": "^8.0.3", "dotenv": "^17.2.4", @@ -17800,6 +18055,7 @@ "@types/js-yaml": "^4.0.9", "@types/json-stable-stringify": "^1.1.0", "@types/picomatch": "^4.0.1", + "@vitest/coverage-v8": "^3.2.4", "chrome-devtools-mcp": "^0.19.0", "msw": "^2.3.4", "typescript": "^5.3.3", @@ -17865,6 +18121,7 @@ "resolved": "https://registry.npmjs.org/@grpc/grpc-js/-/grpc-js-1.14.3.tgz", "integrity": "sha512-Iq8QQQ/7X3Sac15oB6p0FmUg/klxQvXLeileoqrTRGJYLV+/9tubbr9ipz0GKHjmXVsgFPo/+W+2cA8eNcR+XA==", "license": "Apache-2.0", + "peer": true, "dependencies": { "@grpc/proto-loader": "^0.8.0", "@js-sdsl/ordered-map": "^4.4.2" diff --git a/packages/cli/src/ui/hooks/shellCommandProcessor.test.tsx b/packages/cli/src/ui/hooks/shellCommandProcessor.test.tsx index f5e3b61e2b..cec7310e5e 100644 --- a/packages/cli/src/ui/hooks/shellCommandProcessor.test.tsx +++ b/packages/cli/src/ui/hooks/shellCommandProcessor.test.tsx @@ -77,6 +77,7 @@ import { type ShellExecutionResult, type ShellOutputEvent, CoreToolCallStatus, + PolicyDecision, } from '@google/gemini-cli-core'; import * as fs from 'node:fs'; import * as os from 'node:os'; @@ -107,6 +108,7 @@ describe('useShellCommandProcessor', () => { mockConfig = { getTargetDir: () => '/test/dir', getEnableInteractiveShell: () => false, + getPolicyEngine: () => ({ check: vi.fn().mockResolvedValue({ decision: PolicyDecision.ALLOW }) }), getShellExecutionConfig: () => ({ terminalHeight: 20, terminalWidth: 80, @@ -228,8 +230,8 @@ describe('useShellCommandProcessor', () => { it('should handle successful execution and update history correctly', async () => { const { result } = renderProcessorHook(); - act(() => { - result.current.handleShellCommand( + await act(async () => { +result.current.handleShellCommand( 'echo "ok"', new AbortController().signal, ); @@ -260,8 +262,8 @@ describe('useShellCommandProcessor', () => { it('should handle command failure and display error status', async () => { const { result } = renderProcessorHook(); - act(() => { - result.current.handleShellCommand( + await act(async () => { +result.current.handleShellCommand( 'bad-cmd', new AbortController().signal, ); @@ -357,8 +359,8 @@ describe('useShellCommandProcessor', () => { it('should show binary progress messages correctly', async () => { const { result } = renderProcessorHook(); - act(() => { - result.current.handleShellCommand( + await act(async () => { +result.current.handleShellCommand( 'cat img', new AbortController().signal, ); @@ -449,8 +451,8 @@ describe('useShellCommandProcessor', () => { const { result } = renderProcessorHook(); const abortController = new AbortController(); - act(() => { - result.current.handleShellCommand('sleep 5', abortController.signal); + await act(async () => { +result.current.handleShellCommand('sleep 5', abortController.signal); }); const execPromise = onExecMock.mock.calls[0][0]; @@ -474,8 +476,8 @@ describe('useShellCommandProcessor', () => { const binaryBuffer = Buffer.from([0x89, 0x50, 0x4e, 0x47]); mockIsBinary.mockReturnValue(true); - act(() => { - result.current.handleShellCommand( + await act(async () => { +result.current.handleShellCommand( 'cat image.png', new AbortController().signal, ); @@ -504,8 +506,8 @@ describe('useShellCommandProcessor', () => { result: Promise.reject(testError), })); - act(() => { - result.current.handleShellCommand( + await act(async () => { +result.current.handleShellCommand( 'a-command', new AbortController().signal, ); @@ -533,8 +535,8 @@ describe('useShellCommandProcessor', () => { const { result } = renderProcessorHook(); - act(() => { - result.current.handleShellCommand( + await act(async () => { +result.current.handleShellCommand( 'a-command', new AbortController().signal, ); @@ -562,8 +564,8 @@ describe('useShellCommandProcessor', () => { vi.mocked(fs.readFileSync).mockReturnValue('/test/dir/new'); // A different directory const { result } = renderProcessorHook(); - act(() => { - result.current.handleShellCommand( + await act(async () => { +result.current.handleShellCommand( 'cd new', new AbortController().signal, ); @@ -587,8 +589,8 @@ describe('useShellCommandProcessor', () => { vi.mocked(fs.readFileSync).mockReturnValue('/test/dir'); // The same directory const { result } = renderProcessorHook(); - act(() => { - result.current.handleShellCommand('ls', new AbortController().signal); + await act(async () => { +result.current.handleShellCommand('ls', new AbortController().signal); }); const execPromise = onExecMock.mock.calls[0][0]; @@ -729,8 +731,8 @@ describe('useShellCommandProcessor', () => { expect(result.current.activeShellPtyId).toBeNull(); // Pre-condition - act(() => { - result.current.handleShellCommand('cmd', new AbortController().signal); + await act(async () => { +result.current.handleShellCommand('cmd', new AbortController().signal); }); const execPromise = onExecMock.mock.calls[0][0]; @@ -756,8 +758,8 @@ describe('useShellCommandProcessor', () => { const { result } = renderProcessorHook(); - act(() => { - result.current.handleShellCommand('ls', new AbortController().signal); + await act(async () => { +result.current.handleShellCommand('ls', new AbortController().signal); }); // Let microtasks run @@ -1104,8 +1106,8 @@ describe('useShellCommandProcessor', () => { expect(result.current.isBackgroundShellVisible).toBe(true); // 2. Start foreground shell - act(() => { - result.current.handleShellCommand('ls', new AbortController().signal); + await act(async () => { +result.current.handleShellCommand('ls', new AbortController().signal); }); // Wait for PID to be set @@ -1140,8 +1142,8 @@ describe('useShellCommandProcessor', () => { expect(result.current.isBackgroundShellVisible).toBe(true); // 2. Start foreground shell - act(() => { - result.current.handleShellCommand('ls', new AbortController().signal); + await act(async () => { +result.current.handleShellCommand('ls', new AbortController().signal); }); await waitFor(() => expect(result.current.activeShellPtyId).toBe(12345)); expect(result.current.isBackgroundShellVisible).toBe(false); diff --git a/packages/cli/src/ui/hooks/shellCommandProcessor.ts b/packages/cli/src/ui/hooks/shellCommandProcessor.ts index 7e33d37d1f..74cf6793bf 100644 --- a/packages/cli/src/ui/hooks/shellCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/shellCommandProcessor.ts @@ -14,6 +14,7 @@ import { isBinary, ShellExecutionService, CoreToolCallStatus, + PolicyDecision, } from '@google/gemini-cli-core'; import { type PartListUnion } from '@google/genai'; import type { UseHistoryManagerReturn } from './useHistoryManager.js'; @@ -298,6 +299,45 @@ export const useShellCommandProcessor = ( } const executeCommand = async () => { + try { + const policyEngine = config.getPolicyEngine(); + const { decision } = await policyEngine.check( + { name: 'run_shell_command', args: { command: rawQuery } }, + undefined, + ); + + if (decision === PolicyDecision.DENY) { + addItemToHistory( + { + type: 'error', + text: `Command cannot be run. Blocked command: "${rawQuery}". Reason: Blocked by policy.`, + }, + userMessageTimestamp, + ); + if (pwdFilePath && fs.existsSync(pwdFilePath)) { + fs.unlinkSync(pwdFilePath); + } + dispatch({ type: 'SET_ACTIVE_PTY', pid: null }); + setShellInputFocused(false); + return; + } + } catch (err) { + const errorMessage = err instanceof Error ? err.message : String(err); + addItemToHistory( + { + type: 'error', + text: `Policy validation error: ${errorMessage}`, + }, + userMessageTimestamp, + ); + if (pwdFilePath && fs.existsSync(pwdFilePath)) { + fs.unlinkSync(pwdFilePath); + } + dispatch({ type: 'SET_ACTIVE_PTY', pid: null }); + setShellInputFocused(false); + return; + } + let cumulativeStdout: string | AnsiOutput = ''; let isBinaryStream = false; let binaryBytesReceived = 0; diff --git a/packages/core/package.json b/packages/core/package.json index 090b11dfca..e3fb4b153b 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -54,6 +54,7 @@ "@xterm/headless": "5.5.0", "ajv": "^8.17.1", "ajv-formats": "^3.0.0", + "bash-parser": "^0.5.0", "chardet": "^2.1.0", "diff": "^8.0.3", "dotenv": "^17.2.4", @@ -105,6 +106,7 @@ "@types/js-yaml": "^4.0.9", "@types/json-stable-stringify": "^1.1.0", "@types/picomatch": "^4.0.1", + "@vitest/coverage-v8": "^3.2.4", "chrome-devtools-mcp": "^0.19.0", "msw": "^2.3.4", "typescript": "^5.3.3", diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index ace59cd7cf..fbe1a6168e 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -102,9 +102,9 @@ describe('ShellTool', () => { stripThoughtsFromHistory: vi.fn(), }, - getAllowedTools: vi.fn().mockReturnValue([]), + getAllowedTools: vi.fn().mockReturnValue(undefined), getApprovalMode: vi.fn().mockReturnValue('strict'), - getCoreTools: vi.fn().mockReturnValue([]), + getCoreTools: vi.fn().mockReturnValue(undefined), getExcludeTools: vi.fn().mockReturnValue(new Set([])), getDebugMode: vi.fn().mockReturnValue(false), getTargetDir: vi.fn().mockReturnValue(tempRootDir), @@ -439,6 +439,19 @@ describe('ShellTool', () => { ); }); + it('should return a policy violation error if the command is disallowed', async () => { + (mockConfig.getAllowedTools as Mock).mockReturnValueOnce(['run_shell_command(ls)']); + const invocation = shellTool.build({ command: 'ls && cat /etc/passwd' }); + const promise = invocation.execute(mockAbortSignal); + + const result = await promise; + + expect(result.error).toBeDefined(); + expect(result.error?.type).toBe(ToolErrorType.SHELL_EXECUTE_ERROR); + expect(result.error?.message).toBe('Command rejected by policy.'); + expect(result.llmContent).toBe('Command rejected by policy.'); + }); + it('should summarize output when configured', async () => { (mockConfig.getSummarizeToolOutputConfig as Mock).mockReturnValue({ [SHELL_TOOL_NAME]: { tokenBudget: 1000 }, diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 069bcd5981..872bb95254 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -44,6 +44,7 @@ import { SHELL_TOOL_NAME } from './tool-names.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import { getShellDefinition } from './definitions/coreTools.js'; import { resolveToolDeclaration } from './definitions/resolver.js'; +import { doesToolInvocationMatch } from '../utils/tool-utils.js'; import type { AgentLoopContext } from '../config/agent-loop-context.js'; export const OUTPUT_UPDATE_INTERVAL_MS = 1000; @@ -153,6 +154,34 @@ export class ShellToolInvocation extends BaseToolInvocation< shellExecutionConfig?: ShellExecutionConfig, setExecutionIdCallback?: (executionId: number) => void, ): Promise { + const allowedTools = this.context.config.getAllowedTools?.(); + if (allowedTools !== undefined) { + if (!doesToolInvocationMatch('ShellTool', this.params.command, allowedTools)) { + return { + llmContent: 'Command rejected by policy.', + returnDisplay: 'Command rejected by policy.', + error: { + message: 'Command rejected by policy.', + type: ToolErrorType.SHELL_EXECUTE_ERROR, + }, + }; + } + } + + const coreTools = this.context.config.getCoreTools?.(); + if (coreTools !== undefined) { + if (!doesToolInvocationMatch('ShellTool', this.params.command, coreTools)) { + return { + llmContent: 'Command rejected by policy.', + returnDisplay: 'Command rejected by policy.', + error: { + message: 'Command rejected by policy.', + type: ToolErrorType.SHELL_EXECUTE_ERROR, + }, + }; + } + } + const strippedCommand = stripShellWrapper(this.params.command); if (signal.aborted) { diff --git a/packages/core/src/utils/shell-ast-parser.test.ts b/packages/core/src/utils/shell-ast-parser.test.ts new file mode 100644 index 0000000000..7858f6b462 --- /dev/null +++ b/packages/core/src/utils/shell-ast-parser.test.ts @@ -0,0 +1,44 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { extractCommandsFromAst } from './shell-ast-parser.js'; + +describe('shell-ast-parser', () => { + it('extracts a simple command', () => { + const cmds = extractCommandsFromAst('echo "hello"'); + expect(cmds).toEqual(['echo hello']); + }); + + it('extracts commands from a pipeline', () => { + const cmds = extractCommandsFromAst('echo "hello" | grep h'); + expect(cmds).toEqual(['echo hello', 'grep h']); + }); + + it('extracts commands from lists', () => { + const cmds = extractCommandsFromAst('mkdir foo && cd foo || echo "failed" ; ls'); + expect(cmds).toEqual(['mkdir foo', 'cd foo', 'echo failed', 'ls']); + }); + + it('extracts commands from subshells', () => { + const cmds = extractCommandsFromAst('echo $(ls -la) && (cd /tmp && pwd)'); + // Depending on reconstruction, we should at least see the commands + expect(cmds).toContain('ls -la'); + expect(cmds).toContain('cd /tmp'); + expect(cmds).toContain('pwd'); + expect(cmds).toContain('echo $(ls -la)'); + }); + + it('returns empty array on syntax error', () => { + const cmds = extractCommandsFromAst('echo "unterminated'); + expect(cmds).toEqual([]); + }); + + it('handles empty strings gracefully', () => { + expect(extractCommandsFromAst('')).toEqual([]); + expect(extractCommandsFromAst(' ')).toEqual([]); + }); +}); diff --git a/packages/core/src/utils/shell-ast-parser.ts b/packages/core/src/utils/shell-ast-parser.ts new file mode 100644 index 0000000000..56b26c2671 --- /dev/null +++ b/packages/core/src/utils/shell-ast-parser.ts @@ -0,0 +1,84 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +// @ts-ignore +import parse from 'bash-parser'; + +interface BashNode { + type?: string; + name?: { text?: string }; + suffix?: Array<{ text?: string }>; +} + +/** + * Parses a raw shell string and extracts all individual executable commands. + * Handles simple commands, pipelines, lists, and subshells. + * + * @param shellString The raw shell command string + * @returns An array of string representing the commands + */ +export function extractCommandsFromAst(shellString: string): string[] { + if (!shellString || !shellString.trim()) { + return []; + } + + const commands: string[] = []; + + try { + // We use bash-parser to construct an AST synchronously. + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-call + const ast = parse(shellString, { insertResolutionScope: true }); + + // A simple recursive traversal to find all 'Command' nodes + const traverse = (node: unknown) => { + if (!node || typeof node !== 'object') return; + + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + const bashNode = node as BashNode; + + if (bashNode.type === 'Command') { + const parts: string[] = []; + if (bashNode.name && bashNode.name.text) { + parts.push(bashNode.name.text); + } + + if (Array.isArray(bashNode.suffix)) { + bashNode.suffix.forEach((s: unknown) => { + if (s && typeof s === 'object' && 'text' in s && typeof (s as Record)['text'] === 'string') { + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + parts.push((s as {text: string}).text); + } + }); + } + + if (parts.length > 0) { + commands.push(parts.join(' ')); + } + } + + // Recursively traverse all object properties to find nested commands + // (like those in pipelines, lists, or subshells) + for (const key of Object.keys(node)) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + const child = (node as Record)[key]; + if (typeof child === 'object' && child !== null) { + if (Array.isArray(child)) { + child.forEach(traverse); + } else { + traverse(child); + } + } + } + }; + + traverse(ast); + } catch (_error) { + // Graceful failure on syntax errors; return whatever we successfully parsed so far, or empty. + } + + return commands; +} diff --git a/packages/core/src/utils/tool-utils.ts b/packages/core/src/utils/tool-utils.ts index 591df6a87b..f6985de909 100644 --- a/packages/core/src/utils/tool-utils.ts +++ b/packages/core/src/utils/tool-utils.ts @@ -10,6 +10,7 @@ import { type AnyToolInvocation, } from '../index.js'; import { SHELL_TOOL_NAMES } from './shell-utils.js'; +import { extractCommandsFromAst } from './shell-ast-parser.js'; import levenshtein from 'fast-levenshtein'; import { ApprovalMode } from '../policy/types.js'; import { @@ -153,48 +154,72 @@ export function doesToolInvocationMatch( toolNames = [toolOrToolName]; } - if (toolNames.some((name) => SHELL_TOOL_NAMES.includes(name))) { + const isShellTool = toolNames.some((name) => SHELL_TOOL_NAMES.includes(name)); + + if (isShellTool) { toolNames = [...new Set([...toolNames, ...SHELL_TOOL_NAMES])]; - } - for (const pattern of patterns) { - const openParen = pattern.indexOf('('); - - if (openParen === -1) { - // No arguments, just a tool name - if (toolNames.includes(pattern)) { - return true; - } - continue; - } - - const patternToolName = pattern.substring(0, openParen); - if (!toolNames.includes(patternToolName)) { - continue; - } - - if (!pattern.endsWith(')')) { - continue; - } - - const argPattern = pattern.substring(openParen + 1, pattern.length - 1); - - let command: string; + let command: string | undefined; if (typeof invocation === 'string') { command = invocation; } else { if (!('command' in invocation.params)) { - // This invocation has no command - nothing to check. - continue; + return false; } // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion command = String((invocation.params as { command: string }).command); } - if (toolNames.some((name) => SHELL_TOOL_NAMES.includes(name))) { - if (command === argPattern || command.startsWith(argPattern + ' ')) { - return true; + if (!command) { + return false; + } + + const subCommands = extractCommandsFromAst(command); + if (subCommands.length === 0) { + return false; // Fail-closed for empty or unparseable commands + } + + // Every extracted sub-command must match at least one pattern. + for (const subCommand of subCommands) { + let subCommandMatched = false; + + for (const pattern of patterns) { + const openParen = pattern.indexOf('('); + + if (openParen === -1) { + // No arguments, just a tool name + if (toolNames.includes(pattern)) { + subCommandMatched = true; + break; + } + continue; + } + + const patternToolName = pattern.substring(0, openParen); + if (!toolNames.includes(patternToolName) || !pattern.endsWith(')')) { + continue; + } + + const argPattern = pattern.substring(openParen + 1, pattern.length - 1); + if (subCommand === argPattern || subCommand.startsWith(argPattern + ' ')) { + subCommandMatched = true; + break; + } } + + if (!subCommandMatched) { + return false; // This sub-command failed all patterns, so the whole invocation fails + } + } + + return true; // All sub-commands matched at least one pattern + } + + // Non-shell tool validation + for (const pattern of patterns) { + const openParen = pattern.indexOf('('); + if (openParen === -1 && toolNames.includes(pattern)) { + return true; } }