Skip to content

cline#112

Merged
rachellerathbone merged 16 commits intomainfrom
ag-01
May 1, 2026
Merged

cline#112
rachellerathbone merged 16 commits intomainfrom
ag-01

Conversation

@rachellerathbone
Copy link
Copy Markdown
Contributor

What

Brief description of changes

Why

Why this change was needed

How

Brief technical approach

Testing

How to verify the changes

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 1, 2026

multicorn-ops review

Persona Role Primary Status Summary
Jordan Security Auditor yes Concern Several security issues: fail-open design is documented but dangerous in regulated environments, API key read from plaintext config, browser-open URL injection risk, and audit log integrity is not guaranteed.
Priya Open Source Contributor yes Concern Test coverage reporting removed without explanation; new plugin code lacks visible test files.
Marcus Design-Conscious Developer yes Passed No UI changes in this PR; backend hook scripts and configuration only.
Sarah Non-Technical Decision-Maker yes Concern CHANGELOG uses technical jargon and implementation details that non-technical users cannot act on.
The Team Acquisition Due Diligence yes Concern Coverage regression, missing tests for new plugin code, fail-open audit design, and inconsistent toolName field handling signal tech debt and reliability risk.
Alex Accessibility Advocate yes Passed No UI or interactive elements in this PR; backend hooks and documentation only.
Yuki International User yes Concern Documentation contains idioms, unclear terminology, and non-actionable error messages for non-native English speakers.

Concerns

Jordan (Security Auditor)

  • plugins/cline/README.md:52 - Fail-open is explicitly by design: if config is missing, HTTP is used on non-local, or the API errors, actions are allowed. In a regulated environment this means Shield can be silently bypassed by any network disruption, misconfiguration, or deliberate API outage. This must be a configurable fail-closed mode.
  • plugins/cline/hooks/scripts/shared.cjs - API key is read from plaintext ~/.multicorn/config.json with no mention of file permission enforcement (chmod 600). On a multi-user system or in a CI environment, this key is trivially readable by any process running as the same user or with filesystem access.
  • plugins/cline/hooks/scripts/pre-tool-use.cjs:155 - openBrowser passes the consent URL to execFileSync('cmd.exe', ['/c', 'start', '', url]). Although execFileSync is safer than execSync, the URL is constructed from config.baseUrl and agentName which are read from a user-controlled config file. A malicious config.json with a crafted agentName or baseUrl containing special characters could inject additional cmd.exe arguments or navigate to an attacker-controlled page during a consent flow.
  • plugins/cline/hooks/scripts/pre-tool-use.cjs:99 - consentUrl constructs a URL using agentName from config, service, and actionType without URL-encoding validation beyond URLSearchParams. If agentName contains characters that survive URLSearchParams encoding but are interpreted specially by the dashboard (e.g., HTML injection in an approval UI), this could be a stored XSS vector in the consent screen.
  • plugins/cline/hooks/scripts/post-tool-use.cjs:59 - post-tool-use reads toolName from toolUse.tool (not toolUse.toolName), while pre-tool-use reads from toolUse.toolName first. The CHANGELOG notes Cline v3.81+ sends toolName not tool. This inconsistency means post-hook tool mapping may silently fail on newer Cline versions, producing incorrect or missing audit log entries — an audit log integrity issue.
  • plugins/cline/hooks/scripts/shared.cjs - The AUTH_HEADER value 'X-Multicorn-Key' and the API key are sent over HTTPS (enforced for non-local), but there is no certificate pinning or mention of certificate validation. In an enterprise environment with TLS inspection proxies, the API key could be intercepted by a corporate MITM proxy without any warning to the user.
  • plugins/cline/hooks/scripts/pre-tool-use.cjs:218 - Any non-201/202 HTTP status code (including 400, 401, 403, 500) results in blocking the action with a generic permission message rather than fail-open. However a 401 (invalid API key) silently blocks the user with a misleading 'needs permission' message instead of indicating a configuration/authentication error. This can mask a compromised or revoked API key scenario.
  • plugins/cline/hooks/scripts/shared.cjs - The scrubbing logic for parameters and results (buildScrubbedParametersJson, scrubResultForMetadata) is not visible in the truncated diff but is security-critical. If file content or command output is not fully redacted before being sent to the Shield API, sensitive data (credentials, source code, PII) will be exfiltrated to the remote audit endpoint. This needs explicit review and test coverage.

Priya (Open Source Contributor)

  • .github/workflows/ci.yml:208 - Changed from pnpm test:coverage to pnpm test removes coverage reporting in CI. No explanation in CHANGELOG or commit message visible. Coverage is critical for open-source trust.
  • plugins/cline/hooks/scripts/pre-tool-use.cjs - New ~270-line hook script with complex logic (HTTP, browser launch, consent flow) but no corresponding test file visible in diff. Security-critical code needs tests.
  • plugins/cline/hooks/scripts/post-tool-use.cjs - New audit logging hook script but no test file visible. Parameter scrubbing logic should be tested.
  • plugins/cline/hooks/scripts/shared.cjs - Shared utilities module (303 lines) extracted but no tests visible. Tool mapping, config loading, HTTP calls need test coverage.

Sarah (Non-Technical Decision-Maker)

  • CHANGELOG.md:8 - "PreToolUse/PostToolUse hooks" and "Cline v3.81+" are implementation jargon. CEO needs: what problem this solves and what user must do. Example: 'Added AI agent safety checks for VS Code' would be clearer.
  • CHANGELOG.md:12 - "Licence headers on all plugin scripts" is not a user-facing feature. Belongs in internal notes, not CHANGELOG visible to customers.
  • CHANGELOG.md:18 - "CLI wizard installs Cline hooks to ~/Documents/Cline/Hooks/" uses Unix path syntax. Windows-using CEOs see 'Documents' and have no context. Say: 'Cline plugin is installed automatically when you run the setup wizard.'
  • CHANGELOG.md:26 - "Consent flow no longer polls for approval (blocks immediately with consent URL to avoid Cline's 30-second hook timeout)" is internal optimization detail, not a user benefit. Simplify: 'Improved speed of AI agent consent requests.'
  • plugins/cline/README.md:1 - Title 'Multicorn Shield Cline plugin' uses product jargon. Consider: 'Safety Controls for Cline in VS Code' or 'How to Set Up AI Agent Approvals.'

The Team (Acquisition Due Diligence)

  • .github/workflows/ci.yml:208 - Coverage collection was removed from CI (pnpm test:coverage replaced with pnpm test). Combined with switching from istanbul to v8, there is now no coverage enforcement gate in CI. For an acqui-hire evaluation, this signals either a deliberate coverage regression or that the new plugin code has no tests and would fail coverage thresholds.
  • plugins/cline/hooks/scripts/post-tool-use.cjs:59 - post-tool-use reads toolName from toolUse.tool, while pre-tool-use prefers toolUse.toolName (Cline v3.81+). This asymmetry is a latent bug: post-hook will silently emit empty toolName on newer Cline versions, corrupting audit log entries. No tests appear to cover this behavioral difference.
  • plugins/cline - No test files are present in the diff for any of the three new hook scripts (pre-tool-use.cjs, post-tool-use.cjs, shared.cjs). The hooks contain non-trivial logic (URL construction, tool mapping, HTTP calls, scrubbing, browser opening, config validation) with zero visible test coverage.
  • plugins/cline/README.md:52 - Fail-open is the documented and intentional behavior. For due diligence, this means the product's core security governance value proposition can be silently disabled by any transient error. This is an architectural decision that needs explicit product-level justification and ideally a configurable fail-closed mode for enterprise customers.
  • CHANGELOG.md:7 - Version and date are placeholders ([X.Y.Z] - YYYY-MM-DD). This suggests the PR is being merged before release tagging is finalized, which is a minor release process hygiene issue but signals incomplete release discipline.
  • package.json:100 - Switching from @vitest/coverage-istanbul to @vitest/coverage-v8 changes coverage instrumentation behavior and can produce different (often lower) numbers. Combined with removing the coverage CI step, there is no way to know if coverage has regressed.

Yuki (International User)

  • plugins/cline/README.md:8 - Phrase 'Keep these three scripts together whenever you install by hand' uses idiomatic 'by hand' (非ネイティブには不明確). Say: 'If you install manually instead of using the CLI wizard, keep these three files together.'
  • plugins/cline/README.md:17 - Path ~/Documents/Cline/Hooks/ uses ~ notation. International users may not know this means 'home directory.' Write: '(your home folder)/Documents/Cline/Hooks/' or add: 'On Linux/Mac: /home/username/Documents/... On Windows: C:\Users\username\Documents...'
  • plugins/cline/README.md:28 - Field description 'Non-local URLs must use https:// or the hooks disable Shield (fail-open)' uses passive voice and 'fail-open' jargon. Clearer: 'If baseUrl is not localhost and does not use https://, Shield will allow all actions (fail-open). Example: baseUrl: "https://api.example.com" (not http://).'
  • plugins/cline/README.md:53 - 'fail-open by design' is jargon. Explain action: 'If Shield API is unreachable or misconfigured, actions proceed anyway (fail-open). This prevents accidental lockout. Deploy Shield as a policy tool, not as a security barrier to prevent jailbreak.'
  • plugins/cline/hooks/scripts/pre-tool-use.cjs:234 - Error message 'Shield API unreachable' is too vague for debugging. Add: 'Shield API unreachable (check apiKey in ~/.multicorn/config.json and baseUrl is HTTPS). Allowing action.'
  • plugins/cline/hooks/scripts/pre-tool-use.cjs:225 - Error message 'Shield: ${config.agentName} needs ${service}:${actionType} permission' uses colon notation that may confuse non-developers. Example output: 'Shield: agent1 needs filesystem:write permission.' Better: 'Shield: Agent "agent1" needs permission to: Write files.'
  • plugins/cline/README.md:59 - Troubleshooting step 'temporarily add stderr output' is too vague. Provide exact example: 'Add console.error(HOOK_PREFIX, "debug message"); after line X in pre-tool-use.cjs, then check Cline output in VS Code.'

Open-Source Readiness Checklist

Code Quality

  • All functions have clear, descriptive names — Functions like dashboardOrigin, dashboardHintUrl, consentUrl, blockedMessage, openBrowser, respond, buildScrubbedParametersJson, loadConfig, mapToolName, postJson, readStdin are all clearly named.
  • No hardcoded secrets, API keys, internal URLs, or employee names in code or comments — Default URL 'https://api.multicorn.ai' is a public product URL, not a secret. No API keys or employee names found.
  • [~] No // TODO without a public issue reference — Diff is truncated; no TODOs visible in the reviewed portion.
  • No commented-out code blocks — No commented-out code blocks observed in the diff.
  • No debug logging (console.log, println) left in — Only stderr warnings used for operational messages; no console.log calls found.
  • [~] All any types eliminated (TypeScript) — The new plugin scripts are .cjs (CommonJS JS, not TypeScript); JSDoc types are used. The TypeScript source files are not changed in this diff.
  • Error handling is complete — no swallowed exceptions, no empty catch blocks — Several catch blocks are empty or only do 'ignore' (e.g. openBrowser catch{}, dashboardOrigin catch{}, blockedMessage inner catch{}). These silently swallow errors without logging, making debugging difficult. Also, the post-tool-use hook has 'catch{} respond()' patterns that swallow parse errors silently.
  • No Atlassian-internal references, no proprietary patterns or terminology — No Atlassian references found. All references are to Multicorn/Shield which appear to be the project's own product.

Testing

  • All new code has tests — The CI workflow was changed from 'pnpm test:coverage' to 'pnpm test', dropping coverage enforcement. The three new .cjs hook scripts (pre-tool-use.cjs, post-tool-use.cjs, shared.cjs) have no corresponding test files visible in the diff.
  • Coverage meets or exceeds repo minimum — CI was explicitly changed to drop coverage reporting (pnpm test:coverage → pnpm test), and @vitest/coverage-istanbul was replaced with @vitest/coverage-v8. This suggests coverage may no longer be measured or enforced in CI.
  • [~] Tests pass locally and in CI — Cannot determine from diff alone whether tests pass; CI results not visible.
  • Edge cases and error paths are tested — No test files for new hook scripts are visible in the diff. Edge cases (invalid JSON, missing config, HTTP errors, Windows/Linux/macOS browser open) lack visible test coverage.
  • [~] No flaky tests — Cannot determine from diff alone.

Security

  • No secrets in code, comments, config files, or git history — No secrets detected. Example config shows placeholder 'mcs_your_key_here'.
  • All user input is validated — Hook input (stdin JSON) is parsed defensively with null checks and type guards throughout. Tool names, parameters, and result fields are validated before use.
  • [~] Dependencies audited — no known vulnerabilities — Cannot determine from diff alone; no lockfile changes visible.
  • HTTPS enforced for all external communication — HTTPS enforcement for non-localhost URLs is explicitly mentioned in CHANGELOG and README. The shared.cjs logic (partially visible) and README document that non-local URLs must use https://.
  • API keys/tokens never logged — API key is sent only in the X-Multicorn-Key header; no logging of the key value is observed in the diff.

Documentation

  • README.md is accurate and up to date — A new plugins/cline/README.md is added with setup instructions, config options, security model, and troubleshooting. CHANGELOG is updated.
  • [~] CONTRIBUTING.md is accurate and up to date — CONTRIBUTING.md not touched in this diff; cannot assess its accuracy for the new plugin.
  • CHANGELOG.md updated with this change — CHANGELOG.md has a new [X.Y.Z] section documenting Added, Changed, Removed, and Security changes for this PR.
  • New public APIs have JSDoc/KDoc with examples — JSDoc annotations are present on all exported functions in shared.cjs and hook scripts.
  • Any new config options are documented — New config fields (apiKey, baseUrl, agents) are documented in plugins/cline/README.md with a table and example.
  • [~] Architecture decisions documented in ADR if significant — No ADR directory visible in diff; fail-open design and consent flow changes are significant but no ADR is included. Cannot verify if project uses ADRs.

Open Source Hygiene

  • Licence header present in source files (if required by licence) — All three new .cjs scripts begin with '// Copyright (c) Multicorn AI Pty Ltd. MIT License. See LICENSE file.'
  • [~] CODE_OF_CONDUCT.md present — Not touched in diff; cannot verify presence in repo.
  • [~] Issue templates are current — Not touched in diff; cannot verify.
  • [~] PR template is current — Not touched in diff; cannot verify.
  • No internal company references or links — References are to public product URLs (multicorn.ai, github.com/cline/cline). No internal-only links observed.
  • Package name and description are correct in package.json — package.json is updated to include plugins/cline in the files array; no name/description changes that appear incorrect.
  • [~] Repository topics/tags are set on GitHub — Cannot determine from diff alone.

Advisory only. Does not block merge. Actions logged to Shield as pr_review and oss_check.

@rachellerathbone rachellerathbone merged commit 62dbbd2 into main May 1, 2026
3 checks passed
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