Skip to content

feat: add OpenBao-backed MCP guard plugin - #1

Open
papastanb wants to merge 6 commits into
mainfrom
feat/openbao-mcp-guard
Open

papastanb wants to merge 6 commits into
mainfrom
feat/openbao-mcp-guard

Conversation

@papastanb

@papastanb papastanb commented Apr 20, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • add an OpenCode plugin that prevents hardcoded MCP secrets in opencode config
  • add a TUI integration and guided /add-secure-mcp command for secure setup
  • scaffold the plugin repo from the OpenCode plugin template conventions with build, lint, and typecheck support

Validation

  • bun run check
  • bun run lint
  • bun run build

Open in Devin Review

Summary by CodeRabbit

  • New Features

    • Secure MCP plugin that detects and blocks hardcoded API keys and enforces secure key storage
    • Guided "Add Secure MCP" command and TUI onboarding for interactive secure setup
    • Chat reminders promoting secure MCP configuration practices
  • Documentation

    • Full docs and guides for plugin usage, OpenBao/LLM setup, command reference, changelog, release process, and license
  • Chores

    • Project tooling and build configuration, linting/formatting, and package manifest initialized

@coderabbitai

coderabbitai Bot commented Apr 20, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Introduces the opencode-openbao-mcp-guard OpenCode plugin plus project scaffolding: command templates, TUI integration, chat/tool hooks to detect and block hardcoded MCP secrets, and documentation for storing MCP credentials in OpenBao and wiring via openbao-mcp-exec.

Changes

Cohort / File(s) Summary
Build & Project Configuration
\.gitignore, tsconfig.json, eslint.config.js, package.json
New project configuration: ignore build artifacts, TypeScript settings, ESLint/Prettier rules, package exports, Bun-based scripts, dependencies and peer metadata.
Plugin Core
src/index.ts
New plugin implementation: loads markdown command templates with frontmatter at init, registers commands, injects secure-MCP reminder into chat messages, and implements tool.execute.before hook to scan writes/patches for hardcoded secrets and reject them with guidance.
TUI Integration
src/tui.ts
New TUI module registering a "Secure MCP setup" command route (add-secure-mcp) and showing an onboarding toast on first-run.
Commands & Docs (user-facing)
src/commands/add-secure-mcp.md, README.md, AGENTS.md, docs/OPENBAO_SETUP.md, docs/LLM_SETUP.md
Added command documentation and comprehensive user/developer docs describing OpenBao secret workflow, openbao-mcp-exec usage, examples, install and local dev instructions.
Release & Meta
CHANGELOG.md, RELEASE.md, LICENSE
Added changelog entries up to v0.3.0, release process docs, and MIT license file.
Formatting / Linting examples
(implicit) src/..., dist/
Configured formatting and linting rules and added packaging targets; added .gitignore entries for dist/ and node_modules/.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant OpenCode
  participant Plugin as OpenBaoMcpGuard
  participant FS as FileSystem
  participant OpenBao
  User->>OpenCode: sends chat message or triggers tool action
  OpenCode->>Plugin: deliver chat.message / tool.execute.before
  alt chat message matches MCP-related prompt
    Plugin->>OpenCode: append secure-MCP reminder to message parts
    OpenCode->>User: show enriched chat output
  else tool write/edit/apply_patch
    Plugin->>FS: inspect target filename and payload
    alt hardcoded secret detected
      Plugin->>OpenCode: throw validation error with OpenBao instructions
      OpenCode->>User: block write and surface error
    else
      OpenCode->>FS: proceed with write
    end
  end
  Note right of Plugin: On init, Plugin discovers `src/commands` and registers markdown commands into config
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰
Hopping through README and files so neat,
Guarding API keys with soft, steady feet.
I whisper to chats and block risky deeds,
Safe MCP setups from clever little seeds. 🥕🔐

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: add OpenBao-backed MCP guard plugin' directly and accurately summarizes the main objective: introducing a new OpenCode plugin that enforces secure MCP installation via OpenBao integration.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/openbao-mcp-guard
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch feat/openbao-mcp-guard

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@kilo-code-bot

kilo-code-bot Bot commented Apr 20, 2026 •

Copy link
Copy Markdown

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (18 files)
  • .gitignore
  • AGENTS.md
  • CHANGELOG.md
  • LICENSE
  • README.md
  • RELEASE.md
  • docs/LLM_SETUP.md
  • docs/OPENBAO_SETUP.md
  • eslint.config.js
  • package.json
  • src/commands/add-secure-mcp.md
  • src/index.ts
  • src/tui.ts
  • tsconfig.json

Reviewed by grok-code-fast-1:optimized:free · 397,745 tokens

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 5 potential issues.

Open in Devin Review

Comment thread src/index.ts Outdated
Comment thread src/index.ts
Comment thread src/index.ts
Comment on lines +19 to +20
const MCP_PROMPT_RE =
/\b(mcp|model context protocol|context7|chrome-devtools|api key|token|secret)\b/i;

@devin-ai-integration devin-ai-integration Bot Apr 20, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: MCP_PROMPT_RE matches very common words like 'token' and 'secret'

The MCP_PROMPT_RE at src/index.ts:19-20 matches on generic terms like token, secret, and api key. Since this triggers a synthetic reminder injection in chat.message (line 136-151), any user prompt mentioning these common words (e.g., 'What is a JWT token?', 'Tell me a secret') will get the OpenBao reminder appended. This could be noisy in practice. It's not a bug per se, but may cause undesirable UX friction.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread src/index.ts Outdated
Comment thread src/index.ts
Comment on lines +140 to +151
output.parts.push({
type: 'text',
synthetic: true,
text: [
'[Secure MCP reminder] For MCPs that need an API key, do not hardcode secrets in opencode.json.',
'Store the key in OpenBao first:',
'bao kv put -address=http://127.0.0.1:8200 -tls-skip-verify -mount=secret <mcp>/api_key key=TA_CLE_API',
'Then wire the MCP through /home/stan/.local/bin/openbao-mcp-exec.',
'Use /add-secure-mcp for the guided secure setup flow.',
].join(' '),
} as unknown as Part);
},

@devin-ai-integration devin-ai-integration Bot Apr 20, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Part type cast via as unknown as Part for synthetic field

At src/index.ts:170, the object pushed to output.parts includes a synthetic: true field that likely isn't declared in the Part type from @opencode-ai/sdk. The double cast as unknown as Part forces it through. This works at runtime but is fragile — if the SDK changes its Part type to a discriminated union with stricter validation, this could break silently. Consider filing an issue upstream to add synthetic to the Part type if it's a supported field.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

coderabbitai[bot]

This comment was marked as resolved.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 3 new potential issues.

Open in Devin Review

Comment thread src/index.ts Outdated
Comment on lines +160 to +161
const payload = getWritablePayload(output.args);
if (!HARDCODED_SECRET_RE.test(payload)) return;

@devin-ai-integration devin-ai-integration Bot Apr 20, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 HARDCODED_SECRET_RE generic key-value pattern is non-functional for JSON content

The HARDCODED_SECRET_RE regex's generic key-value branch (?:api[_-]?key|token|secret)\s*[=:]\s*["'][^"'{][^"']*["'] cannot detect secrets in JSON format — the primary target of this guard. In JSON, keys are quoted (e.g., "api_key": "secret_value"), so after the regex matches api_key, it expects \s*[=:] but encounters the closing " of the JSON key string, which breaks the match. This means any secret written to opencode.json or opencode.jsonc that doesn't match one of the five hardcoded token prefixes (ctx7sk-, ghp_, xox*-, AIza, AKIA) will silently bypass the guard. For example, a hardcoded "sk_live_Xk9j2mNpQrSt4u5v" passed via --api-key in an MCP command array goes completely undetected.

Test demonstrating the bypass

The generic pattern only matches env/YAML-style configs (e.g., api_key = "value"), never JSON:

  • {"api_key":"sk_live_abc"} → NOT detected
  • {"token":"my-secret"} → NOT detected
  • api_key = "sk_live_abc" → detected
  • {"k":"ghp_abc123"} → detected (via specific prefix)
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread src/index.ts
Comment thread src/index.ts
Comment on lines +136 to +138
'chat.message': async (_input, output) => {
const promptText = getPromptText(output.parts);
if (!MCP_PROMPT_RE.test(promptText)) return;

@devin-ai-integration devin-ai-integration Bot Apr 20, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: chat.message hook correctly reads output.parts, not _input

At first glance, ignoring _input and reading from output.parts in the chat.message hook looks like it might be checking the wrong thing. However, per the plugin API types (@opencode-ai/plugin/dist/index.d.ts:183-195), input only carries metadata (sessionID, agent, etc.) while output carries { message: UserMessage; parts: Part[] } — the actual message content. So reading output.parts is the correct approach to inspect the user's message and inject the reminder part. No bug here.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

devin-ai-integration[bot]

This comment was marked as resolved.

- fix HARDCODED_SECRET_RE bypass by scanning raw string values instead of JSON.stringify'd args (JSON escaping broke generic pattern matching)
- fix Bun.file().exists() for directory checks by using node:fs/promises stat instead
- move OPENBAO_EXECUTABLE constant to top of file near other constants
- remove duplicate OPENBAO_EXECUTABLE declaration at bottom of file
- fix TUI command trigger by removing leading slash from 'add-secure-mcp'
@papastanb

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Apr 20, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@papastanb

Copy link
Copy Markdown
Owner Author

/review

1 similar comment
@papastanb

Copy link
Copy Markdown
Owner Author

/review

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 3 new potential issues.

Open in Devin Review

Comment thread src/index.ts
Comment on lines +56 to +74
async function resolveCommandDirectory(): Promise<string | null> {
const candidateDirs = [
path.join(import.meta.dir, 'commands'),
path.join(import.meta.dir, '../src/commands'),
];

for (const candidate of candidateDirs) {
try {
const stat = await fs.stat(candidate);
if (stat.isDirectory()) {
return candidate;
}
} catch {
// directory does not exist
}
}

return null;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Command directory resolution fallback correctly handles published-package layout

The resolveCommandDirectory() function at src/index.ts:56-74 tries two candidate paths: import.meta.dir + '/commands' (works during development when running source) and import.meta.dir + '/../src/commands' (works in published packages where the entry point is dist/index.js and src/commands/ is a sibling). The package.json files field at line 35-38 correctly includes both dist and src/commands, so the .md command files ship alongside the built JS. This was confirmed by npm pack --dry-run showing src/commands/add-secure-mcp.md in the tarball.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread src/index.ts
Comment on lines +110 to +116
function targetsOpencodeConfig(args: Record<string, unknown>): boolean {
const filePath = getFilePath(args);
if (CONFIG_FILE_RE.test(filePath)) return true;

const payload = getWritablePayload(args);
return /opencode\.jsonc?/i.test(payload);
}

@devin-ai-integration devin-ai-integration Bot Apr 20, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: targetsOpencodeConfig payload fallback could produce false positives

At src/index.ts:127-128, when the filePath arg doesn't match CONFIG_FILE_RE, the function falls back to JSON.stringify(args) and tests for /opencode\.jsonc?/i. This could match if any string value in the args (e.g. file content being written) mentions opencode.json even though the target file is unrelated. However, this is a defense-in-depth measure — the guard only blocks if hasSecretInValue also returns true. The combination makes false-positive blocking unlikely in practice, but a tool writing content that discusses opencode.json AND happens to contain a token-like pattern in a different field would be incorrectly blocked.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread src/index.ts Outdated
@papastanb
papastanb force-pushed the feat/openbao-mcp-guard branch from ac53a96 to 0742274 Compare April 20, 2026 19:45
- replace top-level-only string scanning with recursive hasSecretInValue()
- now detects secrets in nested objects and arrays within tool args
- addresses Devin finding about missed nested secrets

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 new potential issues.

Open in Devin Review

Comment thread src/index.ts
Comment on lines +110 to +116

return commands;
}

function getFilePath(args: Record<string, unknown>): string {
const filePath = args.filePath;
return typeof filePath === 'string' ? filePath : '';

@devin-ai-integration devin-ai-integration Bot Apr 20, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 targetsOpencodeConfig JSON payload fallback causes false positives on non-config file writes

When a write/edit targets a file other than opencode.json (e.g., README.md), the filePath check correctly returns false. However, the fallback at line 115 serializes all args to JSON and checks whether the string opencode.json appears anywhere in it — including inside the file content being written. If the content mentions opencode.json (common in docs) and also matches HARDCODED_SECRET_RE (which triggers on generic patterns like token = "any_value"), the guard incorrectly throws and blocks the write.

Concrete false-positive scenario

A write to README.md with content like Configure in opencode.json\ntoken = "my-secret-value" triggers the guard because:

  1. filePath is README.md → first check passes (not opencode config)
  2. JSON.stringify(args) contains opencode.json → fallback returns true
  3. The content string matches HARDCODED_SECRET_RE on token = "my-secret-value"
  4. The guard throws, blocking a legitimate write to a non-config file
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread src/index.ts
Comment on lines +22 to +23
const HARDCODED_SECRET_RE =
/(?:ctx7sk-[A-Za-z0-9-]+|ghp_[A-Za-z0-9]+|xox[baprs]-[A-Za-z0-9-]+|AIza[0-9A-Za-z\-_]{20,}|AKIA[0-9A-Z]{16}|(?:api[_-]?key|token|secret)\s*[=:]\s*["'][^"'{][^"']*["'])/i;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: HARDCODED_SECRET_RE generic keyword branch is very broad — matches placeholder and example values

The last alternative in HARDCODED_SECRET_RE (src/index.ts:23) matches patterns like token = "any_value", secret: "placeholder", and api_key = "test". This is intentionally broad for a security guard, but it means the guard will block writes containing documentation examples, placeholder values, or non-secret config entries that happen to use common keywords with quoted values. Combined with the targetsOpencodeConfig false-positive (reported as a bug), this broadness amplifies the chance of incorrectly blocking legitimate writes. This is a design tradeoff rather than a bug — being overly strict is generally better for a security tool — but may frustrate users when documenting MCP configurations.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@papastanb

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Apr 20, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/index.ts`:
- Around line 20-21: The current MCP_PROMPT_RE matches credential words alone
and triggers MCP reminders too broadly; update MCP_PROMPT_RE to only match when
an MCP/server context term (e.g., "mcp", "model context protocol", "context7",
"chrome-devtools") appears together with a credential/setup term (e.g., "api
key", "token", "secret")—for example by using a regex with positive lookahead(s)
or an AND-style pattern so both categories must be present; apply the same
tightened pattern to any other occurrences of the original regex used around the
MCP reminder logic so the reminder only fires for true MCP-related credential
mentions.
- Around line 22-35: The current hasSecretInValue only tests each string
independently, so CLI arg arrays like ["--api-key","real-value"] bypass
detection; update hasSecretInValue (and any MCP args handling) to detect flag
keys and treat the adjacent element as a secret: when Array.isArray(value)
iterate by index, if an element matches a flag pattern (e.g.
/^(--(?:api[_-]?key|token|secret)|-k)$/i or other vendor flag names) then run
HARDCODED_SECRET_RE.test on the next element and return true if it matches; also
handle nested objects where property names match flag-like keys by testing their
values with HARDCODED_SECRET_RE; keep existing per-string and recursive checks.
Ensure this logic is applied to MCP args arrays before allowing writes
(functions: hasSecretInValue and any code that passes MCP "args").
- Around line 163-188: Update the user-facing OpenBao guidance in the messages
emitted by the hook 'tool.execute.before' (and the earlier text array) to avoid
recommending insecure defaults: replace the hardcoded 'http://127.0.0.1:8200'
and the '-tls-skip-verify' flag with a secure placeholder (e.g.,
'<OPENBAO_ADDRESS>' or 'https://your-openbao-host:8200') and remove the
'-tls-skip-verify' suggestion; modify the strings composed in the text array and
the Error thrown in 'tool.execute.before' (also referenced alongside
OPENBAO_EXECUTABLE) so they instruct users to use a secure HTTPS endpoint and
valid TLS verification instead of disabling it.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ac4c3f46-4d39-4d67-bb04-7ce96dec2849

📥 Commits

Reviewing files that changed from the base of the PR and between 20db852 and 89e759e.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (14)
  • .gitignore
  • AGENTS.md
  • CHANGELOG.md
  • LICENSE
  • README.md
  • RELEASE.md
  • docs/LLM_SETUP.md
  • docs/OPENBAO_SETUP.md
  • eslint.config.js
  • package.json
  • src/commands/add-secure-mcp.md
  • src/index.ts
  • src/tui.ts
  • tsconfig.json
✅ Files skipped from review due to trivial changes (12)
  • .gitignore
  • LICENSE
  • RELEASE.md
  • CHANGELOG.md
  • docs/OPENBAO_SETUP.md
  • docs/LLM_SETUP.md
  • eslint.config.js
  • AGENTS.md
  • tsconfig.json
  • src/commands/add-secure-mcp.md
  • README.md
  • package.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/tui.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: papastanb/Bao_MCP

Timestamp: 2026-04-20T19:56:21.332Z
Learning: Store MCP credentials in OpenBao first before using them
Learnt from: CR
Repo: papastanb/Bao_MCP

Timestamp: 2026-04-20T19:56:21.332Z
Learning: Wire MCP processes through `openbao-mcp-exec` or an equivalent absolute path
Learnt from: CR
Repo: papastanb/Bao_MCP

Timestamp: 2026-04-20T19:56:21.332Z
Learning: Treat the TUI module as a first-class surface for OpenCode plugin UX
Learnt from: CR
Repo: papastanb/Bao_MCP

Timestamp: 2026-04-20T19:56:21.332Z
Learning: Prefer minimal, deterministic plugin hooks over complex magic
🔇 Additional comments (1)
src/index.ts (1)

69-99: No action required—command markdown is already properly configured for packaging.

The package.json explicitly includes "src/commands" in the "files" array, ensuring the markdown assets ship with the published package. When installed as a dependency, the fallback path ../src/commands in resolveCommandDirectory() will correctly resolve to the included directory. The TUI command is fully supported.

			> Likely an incorrect or invalid review comment.

Comment thread src/index.ts
Comment on lines +20 to +21
const MCP_PROMPT_RE =
/\b(mcp|model context protocol|context7|chrome-devtools|api key|token|secret)\b/i;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Narrow the chat reminder trigger to MCP context.

Right now any assistant output mentioning token, secret, or api key gets an MCP-specific reminder. Consider requiring MCP/server context plus a credential/setup term to avoid unrelated reminder spam. Based on learnings: Prefer minimal, deterministic plugin hooks over complex magic.

Suggested refinement
-const MCP_PROMPT_RE =
-  /\b(mcp|model context protocol|context7|chrome-devtools|api key|token|secret)\b/i;
+const MCP_CONTEXT_RE = /\b(mcp|model context protocol|context7|chrome-devtools)\b/i;
+const CREDENTIAL_CONTEXT_RE = /\b(api key|token|secret|credential|auth)\b/i;
@@
     'chat.message': async (_input, output) => {
       const promptText = getPromptText(output.parts);
-      if (!MCP_PROMPT_RE.test(promptText)) return;
+      if (!MCP_CONTEXT_RE.test(promptText) || !CREDENTIAL_CONTEXT_RE.test(promptText)) {
+        return;
+      }

Also applies to: 156-170

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/index.ts` around lines 20 - 21, The current MCP_PROMPT_RE matches
credential words alone and triggers MCP reminders too broadly; update
MCP_PROMPT_RE to only match when an MCP/server context term (e.g., "mcp", "model
context protocol", "context7", "chrome-devtools") appears together with a
credential/setup term (e.g., "api key", "token", "secret")—for example by using
a regex with positive lookahead(s) or an AND-style pattern so both categories
must be present; apply the same tightened pattern to any other occurrences of
the original regex used around the MCP reminder logic so the reminder only fires
for true MCP-related credential mentions.

Comment thread src/index.ts
Comment on lines +22 to +35
const HARDCODED_SECRET_RE =
/(?:ctx7sk-[A-Za-z0-9-]+|ghp_[A-Za-z0-9]+|xox[baprs]-[A-Za-z0-9-]+|AIza[0-9A-Za-z\-_]{20,}|AKIA[0-9A-Z]{16}|(?:api[_-]?key|token|secret)\s*[=:]\s*["'][^"'{][^"']*["'])/i;
const OPENBAO_EXECUTABLE = 'openbao-mcp-exec';

function hasSecretInValue(value: unknown): boolean {
if (typeof value === 'string') {
return HARDCODED_SECRET_RE.test(value);
}
if (Array.isArray(value)) {
return value.some(hasSecretInValue);
}
if (value !== null && typeof value === 'object') {
return Object.values(value).some(hasSecretInValue);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Catch CLI flag/value secrets in MCP args arrays.

hasSecretInValue() tests each string independently, so a config like "args": ["--api-key", "real-value"] bypasses the guard unless the value matches a vendor-specific prefix. Track secret-looking keys/flags with their adjacent or nested value before allowing the write. Based on learnings: Store MCP credentials in OpenBao first before using them.

Suggested direction
 const HARDCODED_SECRET_RE =
   /(?:ctx7sk-[A-Za-z0-9-]+|ghp_[A-Za-z0-9]+|xox[baprs]-[A-Za-z0-9-]+|AIza[0-9A-Za-z\-_]{20,}|AKIA[0-9A-Z]{16}|(?:api[_-]?key|token|secret)\s*[=:]\s*["'][^"'{][^"']*["'])/i;
+const SECRET_KEY_RE = /\b(?:api[_-]?key|token|secret)\b/i;
+const PLACEHOLDER_SECRET_RE =
+  /^(?:<[^>]+>|\$\{[^}]+}|process\.env\.|env:|OPENBAO_|VAULT_)/i;
 const OPENBAO_EXECUTABLE = 'openbao-mcp-exec';
 
+function isConcreteSecretValue(value: unknown): boolean {
+  return (
+    typeof value === 'string' &&
+    value.trim().length > 0 &&
+    !PLACEHOLDER_SECRET_RE.test(value.trim())
+  );
+}
+
 function hasSecretInValue(value: unknown): boolean {
   if (typeof value === 'string') {
     return HARDCODED_SECRET_RE.test(value);
   }
   if (Array.isArray(value)) {
-    return value.some(hasSecretInValue);
+    return value.some((item, index) => {
+      if (
+        typeof item === 'string' &&
+        SECRET_KEY_RE.test(item) &&
+        isConcreteSecretValue(value[index + 1])
+      ) {
+        return true;
+      }
+      return hasSecretInValue(item);
+    });
   }
   if (value !== null && typeof value === 'object') {
-    return Object.values(value).some(hasSecretInValue);
+    return Object.entries(value).some(([key, nestedValue]) => {
+      if (SECRET_KEY_RE.test(key) && isConcreteSecretValue(nestedValue)) {
+        return true;
+      }
+      return hasSecretInValue(nestedValue);
+    });
   }
   return false;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/index.ts` around lines 22 - 35, The current hasSecretInValue only tests
each string independently, so CLI arg arrays like ["--api-key","real-value"]
bypass detection; update hasSecretInValue (and any MCP args handling) to detect
flag keys and treat the adjacent element as a secret: when Array.isArray(value)
iterate by index, if an element matches a flag pattern (e.g.
/^(--(?:api[_-]?key|token|secret)|-k)$/i or other vendor flag names) then run
HARDCODED_SECRET_RE.test on the next element and return true if it matches; also
handle nested objects where property names match flag-like keys by testing their
values with HARDCODED_SECRET_RE; keep existing per-string and recursive checks.
Ensure this logic is applied to MCP args arrays before allowing writes
(functions: hasSecretInValue and any code that passes MCP "args").

Comment thread src/index.ts
Comment on lines +163 to +188
text: [
'[Secure MCP reminder] For MCPs that need an API key, do not hardcode secrets in opencode.json.',
'Store the key in OpenBao first:',
'bao kv put -address=http://127.0.0.1:8200 -tls-skip-verify -mount=secret <mcp>/api_key key=TA_CLE_API',
`Then wire the MCP through ${OPENBAO_EXECUTABLE} (or your preferred absolute path).`,
'Use /add-secure-mcp for the guided secure setup flow.',
].join(' '),
} as unknown as Part);
},

'tool.execute.before': async (input, output) => {
if (input.tool !== 'write' && input.tool !== 'edit' && input.tool !== 'apply_patch') {
return;
}

if (!targetsOpencodeConfig(output.args)) return;

const hasSecret = hasSecretInValue(output.args);
if (!hasSecret) return;

throw new Error(
[
'Do not hardcode MCP secrets in opencode config.',
'Store the secret in OpenBao instead:',
'bao kv put -address=http://127.0.0.1:8200 -tls-skip-verify -mount=secret <mcp>/api_key key=TA_CLE_API',
`Then configure the MCP through ${OPENBAO_EXECUTABLE} (or your preferred absolute path).`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid insecure OpenBao flags in copy-paste guidance.

The runtime messages recommend http://127.0.0.1:8200 with -tls-skip-verify. For a security guard, the default copy-paste path should not normalize plaintext transport or disabled TLS verification.

Suggested wording change
         text: [
           '[Secure MCP reminder] For MCPs that need an API key, do not hardcode secrets in opencode.json.',
           'Store the key in OpenBao first:',
-          'bao kv put -address=http://127.0.0.1:8200 -tls-skip-verify -mount=secret <mcp>/api_key key=TA_CLE_API',
+          'bao kv put -mount=secret <mcp>/api_key key=<api-key>',
+          'Set BAO_ADDR/BAO_TOKEN for your OpenBao environment; only use local dev TLS overrides in local dev.',
           `Then wire the MCP through ${OPENBAO_EXECUTABLE} (or your preferred absolute path).`,
           'Use /add-secure-mcp for the guided secure setup flow.',
         ].join(' '),
@@
         [
           'Do not hardcode MCP secrets in opencode config.',
           'Store the secret in OpenBao instead:',
-          'bao kv put -address=http://127.0.0.1:8200 -tls-skip-verify -mount=secret <mcp>/api_key key=TA_CLE_API',
+          'bao kv put -mount=secret <mcp>/api_key key=<api-key>',
+          'Set BAO_ADDR/BAO_TOKEN for your OpenBao environment; only use local dev TLS overrides in local dev.',
           `Then configure the MCP through ${OPENBAO_EXECUTABLE} (or your preferred absolute path).`,
         ].join(' ')
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/index.ts` around lines 163 - 188, Update the user-facing OpenBao guidance
in the messages emitted by the hook 'tool.execute.before' (and the earlier text
array) to avoid recommending insecure defaults: replace the hardcoded
'http://127.0.0.1:8200' and the '-tls-skip-verify' flag with a secure
placeholder (e.g., '<OPENBAO_ADDRESS>' or 'https://your-openbao-host:8200') and
remove the '-tls-skip-verify' suggestion; modify the strings composed in the
text array and the Error thrown in 'tool.execute.before' (also referenced
alongside OPENBAO_EXECUTABLE) so they instruct users to use a secure HTTPS
endpoint and valid TLS verification instead of disabling it.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 new potential issues.

Open in Devin Review

Comment thread README.md

## Version

- Current package version: `0.2.0`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 README displays stale version 0.2.0 while package.json is at 0.3.0

The README at line 9 states Current package version: 0.2.0, but package.json:3 has "version": "0.3.0" and CHANGELOG.md:3 lists 0.3.0 as the latest release. This was likely missed when bumping the version in commit 89e759e. Users and integrations that check the README for the current version will see outdated information.

Suggested change
- Current package version: `0.2.0`
- Current package version: `0.3.0`
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread src/index.ts
Comment on lines +22 to +23
const HARDCODED_SECRET_RE =
/(?:ctx7sk-[A-Za-z0-9-]+|ghp_[A-Za-z0-9]+|xox[baprs]-[A-Za-z0-9-]+|AIza[0-9A-Za-z\-_]{20,}|AKIA[0-9A-Z]{16}|(?:api[_-]?key|token|secret)\s*[=:]\s*["'][^"'{][^"']*["'])/i;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 HARDCODED_SECRET_RE key=value pattern only matches quoted values

The last alternative in HARDCODED_SECRET_RE at src/index.ts:23 is (?:api[_-]?key|token|secret)\s*[=:]\s*["'][^"'{][^"']*["']. This pattern requires the value to be wrapped in quotes (" or '). Since hasSecretInValue tests raw string values from the tool args (not serialized JSON), this branch would only fire if a single string value itself contained an embedded key="value" substring. In practice, for structured tool args, the named-prefix patterns (e.g. ctx7sk-, ghp_, AKIA) do the heavy lifting. The key=value branch is effectively a catch-all for unstructured text blobs passed as args. This is not a bug — the prefix patterns cover the primary threat model — but it means secrets without a recognized prefix that appear as plain values (e.g. a bare API key string sk-proj-abc123) would not be caught.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant