T09 · Insecure Skill Coding Practices
Warning
- Location
- extension/index.ts:74
- Finding
- Custom Unity Tools Can Bypass the Destructive-Operation Gate Through Read-Like Names## Vulnerability Details **File Location**: `extension/index.ts:74-75, 134-138` **Vulnerability Type**: Name-based authorization bypass **Risk Level**: Medium ### Vulnerable Code ```ts /** Verbs that only read Editor/project state. */ const READ_ONLY_VERB = /^(get|list|find|search|read|inspect|describe|query|exists|has|count|status|state|info|tree|hierarchy|screenshot|capture)/i; ``` ```ts if (NON_MUTATING_TOOLS.has(key)) return "read-only"; if (HIGH_RISK_NAME.test(key)) return "project-changing"; const verb = key.includes(".") ? key.slice(key.lastIndexOf(".") + 1) : key; return READ_ONLY_VERB.test(verb) ? "read-only" : "project-changing"; ``` The intended behavior is also explicitly accepted by the test at `tests/gate.test.mjs:263-264`: ```js // A custom read-only tool still passes: the allowlist is on the verb. assert.equal(classifyTool("mygame.getScore"), "read-only"); ``` ### Technical Analysis The gateway determines whether a tool is read-only from its name rather than from an authoritative capability declaration. Any unknown or project-registered custom tool whose final name component begins with a recognized read-like prefix—such as `get`, `list`, `read`, or `inspect`—is classified as read-only. This creates an authorization bypass because custom Unity tool names and implementations are controlled by project code. A name such as `mygame.getScore` does not prove that its callback only reads data. Its implementation could modify assets, delete project data, install packages, invoke Editor APIs, or execute other code inside the Unity process. Once the tool is classified as read-only, `evaluateDestructiveGate()` permits it without either of the protections required for project-changing operations: 1. The gateway operator's `OPENCLAW_EDITOR_ALLOW_DESTRUCTIVE=1` or `OPENCLAW_UNITY_ALLOW_DESTRUCTIVE=1` opt-in. 2. The individual call's `confirm: true` flag. This contradicts the documented claim that unknown and project-registered custom tools fail ...[truncated 1723 chars]
- Remediation
- ## Remediation Suggestions 1. Replace prefix-based classification with an exact allowlist of audited built-in read-only tools: ```ts const READ_ONLY_TOOLS = new Set([ "app.getstate", "asset.find", "asset.getpath", "component.get", "component.list", "console.geterrors", "console.getlogs", "debug.hierarchy", "debug.screenshot", "editor.getstate", "editor.listwindows", "gameobject.find", "gameobject.getall", "gameobject.getdata", "scene.getactive", "scene.getdata", "scene.list", "script.list", "script.read", // Other individually reviewed built-in tools. ]); if (READ_ONLY_TOOLS.has(key)) return "read-only"; return "project-changing"; ``` 2. Treat every custom, unknown, unnamed, or malformed tool as project-changing by default, regardless of its verb. 3. If custom read-only tools must be supported, require each exact tool name to be explicitly approved in trusted gateway configuration. Do not allow Unity project code to self-declare an ungated capability without operator approval. 4. Bind custom-tool authorization to authenticated registration metadata and reject conflicting, duplicate, or unrecognized declarations. 5. Add regression tests proving that unknown read-like names fail closed: ```js assert.equal(classifyTool("mygame.getScore"), "project-changing"); assert.equal(classifyTool("project.inspectAssets"), "project-changing"); assert.equal(classifyTool("custom.readData"), "project-changing"); ``` 6. Preserve batch recursion checks so that custom tools nested inside `batch.execute` receive the same fail-closed treatment. 7. Update the documentation to avoid claiming that all unknown tools fail closed until the implementation enforces that property.