Skip to content

fix(mcp): a schema field is served to ChatGPT ungated, so audit it - #284

Open
milstan wants to merge 2 commits into
milstan/rules-batch-5from
milstan/rules-audit-fields
Open

milstan wants to merge 2 commits into
milstan/rules-batch-5from
milstan/rules-audit-fields

Conversation

@milstan

@milstan milstan commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Answers the review question on #279-#282: is PromptForge still the source of truth? For tool descriptions, yes. For parameter and result-field text, no — and it never was, 98,775 characters of it were already hand-written on main. That turned out not to be the real problem.

The real problem

buildServer swaps only the description string for the commerce-free surface — packages/mcp/src/server.ts:859-862. inputSchema and outputSchema are served byte-for-byte identical on both surfaces.

So a sentence on a field is text the {{commerce}} gate cannot reach, however complete the gate looks. Seven audits ask "does this rule still reach the model" and none of them read that channel.

What this does

Change Why
schemaText() in _agent-text.ts Reads every description on a schema node, depth-first
New audit no-purchase-offer-in-schema.test.ts Asserts no field sells on what /chatgpt/mcp serves
Deletes one offer from scan_portfolio_signals It caught a live one

The patterns match an OFFER in either word order rather than every mention of the word. account_status's quota field has to be able to name the wire key topup, and a tool has to be able to recognise a purchase the user made elsewhere.

The leak it caught

leadbay_scan_portfolio_signals's quota_exceeded output field ended "Offer wait-for-reset OR top-up". It ships to ChatGPT today, where OpenAI's plugin guidelines forbid promoting a purchase.

It costs the Claude surface nothing to remove: the tool's description already carries that offer, correctly wrapped in {{commerce}}. The field was an ungated duplicate of a gated paragraph.

Also in here

leadbay_account_status's notifications field named leadbay_pull_leads and leadbay_research_lead_by_id, but dropped leadbay_get_contacts and leadbay_pull_followups when #279 moved that block off the deleted inbox snippet. Both are back. That is the one place in the five PRs where I can show meaning genuinely thinned rather than moved.

Not done here

The other six audits on the helper still read descriptions only. Each needs its own pass to decide whether the rule it guards can live on a field at all — a rule the model must read BEFORE choosing a tool cannot.

Verified

pnpm -r build, pnpm -r typecheck, pnpm -r test green: components 29, core 229, promptforge 9, mcp 157. The new audit fails on main and on #282, and passes here.

🤖 Generated with Claude Code

Milan's review of #279-#282 asked whether PromptForge is still the source of
truth. Checking it surfaced the real gap, and it predates that work.

`buildServer` swaps only the `description` string for the commerce-free surface
(server.ts:859-862). `inputSchema` and `outputSchema` go out byte-for-byte
identical on both, so a sentence on a field is text the `{{commerce}}` gate
cannot reach however complete the gate looks. Seven audits ask "does this rule
reach the model" and none of them read that channel.

Three things here:

  * `schemaText()` in the audit helper reads every `description` on a schema
    node, depth-first.
  * A new audit asserts no parameter or result field sells on the surface
    /chatgpt/mcp serves. Its patterns catch an OFFER in either word order, not
    every mention: `account_status`'s `quota` field has to name the wire key
    `topup`, and a tool has to be able to recognise a purchase made elsewhere.
  * It caught one, live today: `leadbay_scan_portfolio_signals`' `quota_exceeded`
    field ended "Offer wait-for-reset OR top-up". The description already
    carries that offer wrapped in `{{commerce}}`, so the field was an ungated
    duplicate and deleting it costs the Claude surface nothing.

Also restored: the notifications field on `leadbay_account_status` named
leadbay_pull_leads and leadbay_research_lead_by_id but had dropped
leadbay_get_contacts and leadbay_pull_followups when #279 moved that block out
of the deleted inbox snippet. Both are back.

The other six audits on the helper still read descriptions only. Each needs its
own pass to decide whether the rule it guards can live on a field at all.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@claude claude 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.

Both fixes address real leaks correctly: the scan-portfolio-signals field text now matches the gated description, and the new schema-text audit closes a real hole (schema descriptions bypass {{commerce}} since buildServer only swaps the description string). Two issues to fix before merge:

  1. account-status.ts's notification hint now points the bulk_enrich case at leadbay_get_contacts, which is advanced-gated — a regression from an always-available-tool hint to one that doesn't exist in default deployments.
  2. The new no-purchase-offer-in-schema.test.ts audit only scans composite tools, missing granular tools that can also reach the ChatGPT surface when LEADBAY_MCP_ADVANCED=1 — no live offender today, but the coverage gap undercuts the guarantee the test claims.

Comment thread packages/core/src/composite/account-status.ts Outdated
Comment thread packages/mcp/test/audit/no-purchase-offer-in-schema.test.ts
…that exists

Both findings on #284 confirmed and fixed.

leadbay_get_contacts is in granularReadTools, so it only exists when
LEADBAY_MCP_ADVANCED=1. The notifications hint pointed at it for the
bulk_enrich case, which on a default deployment names a tool the model does not
have. The old inbox snippet had the same defect and I restored it faithfully.
It now names leadbay_research_lead_by_id, which is composite and returns the
lead's contacts.

The new audit scanned composite tools only. http-server.ts:357 reads
LEADBAY_MCP_ADVANCED from the environment, so the deployment serving ChatGPT
can register granular tools too. It scans all four arrays now. No granular tool
offers a purchase today; the audit is what keeps it that way.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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