T09 · Insecure Skill Coding Practices
Error
- Location
- handler.js:32
- Finding
- Message-Masking Hook Fails Open Because stderr Is Configured as Ignored<![CDATA[ ## Vulnerability Details **File Location**: `handler.js:32-42` and identical code in `handler.en.js:32-42` **Vulnerability Type**: Fail-open error handling resulting in disclosure of sensitive message content **Risk Level**: High ### Vulnerable Code ```javascript const masker = spawn('python3', [MASKER_SCRIPT, 'mask', content], { stdio: ['pipe', 'pipe', 'ignore'] }); let output = ''; let error = ''; masker.stdout.on('data', (data) => { output += data.toString(); }); masker.stderr.on('data', (data) => { error += data.toString(); }); ``` The surrounding exception handler explicitly permits processing to continue with the original message: ```javascript } catch (error) { console.error('[sensitive-masker] Handler error:', error.message); // Error doesn't affect message processing, continue with original message } ``` ### Technical Analysis Node.js returns `null` for `masker.stderr` when the child process's stderr stream is configured as `"ignore"`. The subsequent call to `masker.stderr.on(...)` therefore throws a `TypeError`. The exception is caught by the outer handler, which neither blocks the message nor substitutes a safe fallback. Consequently, the handler exits before waiting for the Python process and before assigning the masked result to `event.context.content`. This is a deterministic fail-open condition in the security control. It undermines the Skill's primary declared purpose: preventing credentials and personally identifiable information from reaching the downstream LLM API. Passing the message as an element of an argument array does avoid shell interpolation, so this is not a command-injection finding. The issue is the inconsistent child-process stream configuration and unsafe failure policy. ### Attack Path 1. A user or attacker submits a message containing a password, API key, database URL, email address, or other detected sensitive value. 2. The `message:received` hook invokes the Python wrapper. 3. The c ...[truncated 1251 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Configure stderr as an actual pipe when registering a listener: ```javascript const masker = spawn('python3', [MASKER_SCRIPT, 'mask', content], { stdio: ['ignore', 'pipe', 'pipe'] }); ``` Alternatively, remove the stderr listener if stderr must remain ignored. Piping stderr is preferable because it preserves diagnostic information. 2. Handle process startup failures explicitly: ```javascript const exitCode = await new Promise((resolve, reject) => { masker.once('error', reject); masker.once('close', resolve); }); ``` 3. Apply a fail-closed policy for this security boundary. If masking cannot be completed, do not forward the original message. Return an explicit processing error or replace the content with a safe placeholder. 4. Validate the wrapper output before modifying the event: ```javascript const result = JSON.parse(output); if (typeof result.masked !== 'string') { throw new Error('Invalid masker output'); } ``` 5. Add integration tests that: - Submit a message containing a known test credential. - Assert that `event.context.content` contains a mask marker rather than the credential. - Simulate Python startup failure, malformed JSON, and nonzero exit status. - Assert that every failure mode blocks or safely redacts the original content. 6. Apply the same correction to `handler.en.js`, or remove duplicated executable variants to prevent future security fixes from diverging. ]]>