feat: per-site MCP serverInfo instructions (BLOCK-19)#16
Merged
Conversation
…K-19)
Lets site admins paste plain-text rules under Settings → Block MCP and
every connected MCP client receives them at handshake — no per-session
rediscovery of conventions like callout className mapping or code-block
theme. The TypeScript server fetches the addendum at startup and
appends to the hard-coded baseline before `McpServer` accepts the first
request.
PHP plugin (gk-block-api → 1.7.0):
- New `Instructions` service (`includes/class-instructions.php`) owns
option storage, sanitize, length cap (2,000 chars), updated-at
timestamp, and the per-IP rate-limit bucket.
- New REST route `GET /gk-block-api/v1/instructions` — public by design
(clients fetch before any auth-gated tool call). Returns `{ addendum,
length, max_length, updated_at }` with `Cache-Control: public,
max-age=60`. Rate-limited at 30 req/min per remote IP via a sliding
60-second window; IPs are SHA-256-hashed before becoming transient
keys (PII minimization).
- New admin section at Settings → Block MCP. Textarea with live char
counter, in-page "public data" warning, and `maxlength` attribute.
Settings API registers the option under the existing option group so
the form's nonce + manage_options cap already cover the save path.
- Reset-to-defaults + uninstall sweeps the new options and the per-IP
rate-limit transients.
TypeScript MCP server (@gravitykit/block-mcp → 1.7.0):
- New `src/instructions.ts` with `BASELINE`, `sanitizeAddendum`,
`combineInstructions`, `fetchAddendum`, `getInstructions`. The
baseline string moves out of `src/index.ts` into the module so the
source of truth is single.
- `main()` calls `getInstructions(WORDPRESS_URL)` before connecting the
transport and upgrades the `McpServer._instructions` field in place.
Fetch failures (timeout, 4xx/5xx, DNS, malformed JSON) log to stderr
and fall back to baseline-only — startup never blocks on the network.
- `BLOCK_MCP_INSTRUCTIONS_OFF=1` env var disables the fetch entirely
for offline testing and isolation.
- Defense in depth: TS-side sanitize re-applies the cap and strips C0
controls, DEL, Bidi overrides, and zero-width characters before the
string reaches the SDK — guards against a compromised WP install
pushing prompt-injection payloads through an invisible-character
attack.
Tests:
- 26 PHPUnit tests for `Instructions` (option round-trip, sanitize
variations, length cap, timestamp tracking, per-IP rate limit, no
raw IP in option_name, IPv6 support, defense-in-depth re-sanitize).
- 8 PHPUnit integration tests for `GET /instructions` (response shape,
unauthenticated access, Cache-Control header, dirty-option
re-sanitize, 429 on rate-limit, per-IP isolation, updated_at
contract).
- 37 vitest tests for `src/instructions.ts` (BASELINE invariants,
sanitize coverage, combine semantics, fetchAddendum behavior with
axios mocked, env-var opt-out, every failure path returns empty).
Full suites green: 626 PHP / 9,198 assertions; 506 TS. phpcs clean,
tsc --noEmit clean, esbuild build clean.
Closes BLOCK-19.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR adds a complete feature for WordPress site administrators to configure custom MCP server instructions. The Node.js server fetches a per-site addendum at startup, combines it with a baseline, and serves the result to MCP clients. The WordPress plugin provides admin UI, REST API, backend storage with sanitization and length enforcement, and per-IP rate limiting. ChangesMCP Server Site-Specific Instructions
🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/index.ts`:
- Around line 409-423: The current code mutates the private field
server.server._instructions (inner._instructions) which is an internal SDK
detail; instead fetch the instructions string and instantiate the public
constructor with those instructions (e.g. use new McpServer({ instructions }))
or pass instructions via the public options API when creating the Server
instance rather than writing to inner._instructions; locate the use of
server.server/inner and replace the mutation with code that reads the
instructions and constructs/initializes the MCP server through the public
McpServer/McpServer constructor or factory method so no private field is
mutated.
In `@src/instructions.ts`:
- Around line 175-188: The startup fetch using axios.get (the call that passes
FETCH_TIMEOUT_MS, maxRedirects, headers and validateStatus) is vulnerable to
cross-host redirects and unbounded response bodies; add a maxContentLength to
cap payload size for this JSON endpoint and prevent cross-host redirects by
either setting maxRedirects: 0 and implementing manual Location header
validation after the response (use the url/origin of the request to ensure any
Location stays same-host), or register an axios response interceptor that
rejects responses whose redirect Location header points to a different host;
update the axios.get options and attach the interceptor logic where the fetch is
performed and ensure sanitizeAddendum() still truncates but is no longer relied
on for safety.
In `@wordpress-plugin/gk-block-api/includes/class-instructions.php`:
- Around line 49-50: The MAX_LENGTH constant is defined as a character cap but
current length/truncation uses byte-based functions; update any places that
enforce or truncate to MAX_LENGTH (e.g., the instruction text truncation logic
that reads strlen/substr) to use mb_strlen(..., 'UTF-8') and mb_substr(...,
'UTF-8') so multibyte characters aren’t split; apply the same change to REST
response metadata generation (class-rest-controller.php handling of the
contract) and the admin UI counter logic (class-settings-page.php counter
display) so all character counts and truncations are UTF-8 safe.
In `@wordpress-plugin/gk-block-api/readme.txt`:
- Around line 99-101: Update the inconsistent version metadata by bumping all
five release locations together to 1.7.0: set the plugin header "Version" field
to 1.7.0, update the GK_BLOCK_API_VERSION constant to 1.7.0, change the
readme.txt "Stable tag" line to 1.7.0, update the Upgrade Notice section to
reference 1.7.0, and update the Changelog entry to reflect 1.7.0; commit these
five changes atomically so the plugin header, GK_BLOCK_API_VERSION, readme
Stable tag, Upgrade Notice, and Changelog remain consistent.
🪄 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: 6ad05ec0-6c21-45bc-a160-ac8fca715a41
⛔ Files ignored due to path filters (1)
dist/index.cjsis excluded by!**/dist/**
📒 Files selected for processing (12)
package.jsonsrc/__tests__/unit/instructions.test.tssrc/index.tssrc/instructions.tswordpress-plugin/gk-block-api/gk-block-api.phpwordpress-plugin/gk-block-api/includes/class-instructions.phpwordpress-plugin/gk-block-api/includes/class-rest-controller.phpwordpress-plugin/gk-block-api/includes/class-settings-page.phpwordpress-plugin/gk-block-api/readme.txtwordpress-plugin/gk-block-api/tests/Instructions/InstructionsTest.phpwordpress-plugin/gk-block-api/tests/REST/InstructionsRouteTest.phpwordpress-plugin/gk-block-api/uninstall.php
…8, stable tag)
Four reviewer findings; three valid, one already-correct-except-for-one-line.
1. src/index.ts no longer mutates the SDK's private `_instructions`
field. McpServer is now constructed inside `main()` AFTER fetching
the per-site addendum, passing the combined string to the public
constructor option. Handler registration moved into
`registerHandlers(server)` so the deferred construction doesn't
force a sequencing change for tool/resource/prompt setup.
2. src/instructions.ts hardens the addendum fetch:
- `maxRedirects: 0` disables follow-the-Location-header entirely so
a compromised or misconfigured site can't bounce us to a
different origin. 3xx responses surface as axios errors and fall
through to baseline-only.
- `maxContentLength: 16 KB` (FETCH_MAX_BYTES) caps the response body
at the HTTP layer. The expected JSON payload sits well under
10 KB; 16 KB gives headroom while still preventing slowloris /
memory-pressure DoS from a hostile server. `sanitizeAddendum`
remains as defense in depth — both checks active.
3. PHP plugin switches length operations from byte-based `strlen` /
`substr` to UTF-8 character-based `mb_strlen` / `mb_substr` so a
string of 2,000 emoji (4 bytes each = 8 KB) is accepted, the
`length` field in the REST response matches `MAX_LENGTH` semantics
(characters, not bytes), and truncation never lands mid-codepoint
(which would produce invalid UTF-8 and break downstream JSON
parsers). Touched: class-instructions.php (3 sites), class-rest-
controller.php (length field), class-settings-page.php (counter).
4. readme.txt `Stable tag` was still `1.6.1` despite the earlier 1.7.0
bump landing in the plugin header, the GK_BLOCK_API_VERSION
constant, Upgrade Notice, and Changelog. Single-line correction;
all five release locations are now in sync.
New tests:
- `test_sanitize_truncates_multibyte_at_character_boundary` — feeds
`MAX_LENGTH + 100` copies of U+1F600 (😀, 4 bytes each) and asserts
the result is exactly `MAX_LENGTH` chars AND that `mb_check_encoding`
confirms the bytes are still valid UTF-8.
- `test_sanitize_preserves_emoji_below_cap` — round-trips an emoji-
flavored markdown list verbatim.
- `disables redirect following (maxRedirects: 0)` — pins the new axios
option.
- `caps response body size via maxContentLength` — pins the size cap
inside the bracket (> MAX_ADDENDUM_LENGTH, ≤ 64 KB).
- `treats a 302 redirect as a fetch failure` — confirms 3xx falls
through to baseline-only via the existing catch path.
Fixed three pre-existing tests with wrong expectations: shortcode
test only validates registered shortcodes (the others survive
strip_shortcodes by design), control-char test no longer assumes ANSI
escape printable suffix is stripped, dirty-option test matches
wp_strip_all_tags actual behavior of removing script tag content.
Full suites green: PHPUnit 626 / 9,198 assertions, vitest 509,
phpcs clean, tsc --noEmit clean, esbuild build clean.
Refs BLOCK-19.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/instructions.ts (1)
142-144:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTruncate by Unicode code points, not UTF-16 code units.
Lines 142–144 use
lengthandslice, which count UTF-16 code units. Astral characters (emoji, multi-byte codepoints) count as 2 and can be split mid-surrogate pair, corrupting the result at the boundary.- if (s.length > MAX_ADDENDUM_LENGTH) { - s = s.slice(0, MAX_ADDENDUM_LENGTH); - } + const chars = Array.from(s); + if (chars.length > MAX_ADDENDUM_LENGTH) { + s = chars.slice(0, MAX_ADDENDUM_LENGTH).join(''); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/instructions.ts` around lines 142 - 144, The current truncation uses s.length and s.slice which operate on UTF-16 code units and can split surrogate pairs; change the truncation to operate on Unicode code points by converting the string to an array of code points (e.g., Array.from(s) or [...s]), slice that array to MAX_ADDENDUM_LENGTH, then join back into a string and assign to s (keep the MAX_ADDENDUM_LENGTH symbol and the variable s unchanged so the rest of the code still works).wordpress-plugin/gk-block-api/includes/class-settings-page.php (1)
469-495:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winThe admin-side character budget doesn't match the backend contract.
The server validates using
mb_strlen(..., 'UTF-8')which counts Unicode code points, butmaxlengthandta.value.lengthboth count UTF-16 code units. Emojis will show the wrong remaining budget and get rejected early in the browser even though the server still accepts up to 2,000 code points.💡 Proposed fix
- maxlength="<?php echo esc_attr( (string) $instructions_max ); ?>" class="large-text code" @@ - if (!ta || !count) return; - ta.addEventListener('input', function () { count.textContent = String(ta.value.length); }); + if (!ta || !count) return; + var max = <?php echo (int) $instructions_max; ?>; + function syncCount() { + var chars = Array.from(ta.value); + if (chars.length > max) { + ta.value = chars.slice(0, max).join(''); + chars = Array.from(ta.value); + } + count.textContent = String(chars.length); + } + ta.addEventListener('input', syncCount); + syncCount(); })();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wordpress-plugin/gk-block-api/includes/class-settings-page.php` around lines 469 - 495, The frontend counts UTF-16 code units (maxlength attribute and ta.value.length) while the backend and display use mb_strlen (code points), causing emoji/astral characters to misreport and be prematurely blocked; remove or avoid relying on the HTML maxlength and change the client JS for the textarea with id "gk-block-api-instructions" to count and enforce the same code-point limit ($instructions_max) using Array.from(...) or the spread operator ([...ta.value]).length, update the counter element "gk-block-api-instructions-count" to show Array.from(ta.value).length, and when enforcing the limit trim the value by code points (e.g. Array.from(ta.value).slice(0, $instructions_max).join('')) so the client matches the server's mb_strlen behavior for $instructions_val/Instructions::OPTION_KEY.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/instructions.ts`:
- Around line 142-144: The current truncation uses s.length and s.slice which
operate on UTF-16 code units and can split surrogate pairs; change the
truncation to operate on Unicode code points by converting the string to an
array of code points (e.g., Array.from(s) or [...s]), slice that array to
MAX_ADDENDUM_LENGTH, then join back into a string and assign to s (keep the
MAX_ADDENDUM_LENGTH symbol and the variable s unchanged so the rest of the code
still works).
In `@wordpress-plugin/gk-block-api/includes/class-settings-page.php`:
- Around line 469-495: The frontend counts UTF-16 code units (maxlength
attribute and ta.value.length) while the backend and display use mb_strlen (code
points), causing emoji/astral characters to misreport and be prematurely
blocked; remove or avoid relying on the HTML maxlength and change the client JS
for the textarea with id "gk-block-api-instructions" to count and enforce the
same code-point limit ($instructions_max) using Array.from(...) or the spread
operator ([...ta.value]).length, update the counter element
"gk-block-api-instructions-count" to show Array.from(ta.value).length, and when
enforcing the limit trim the value by code points (e.g.
Array.from(ta.value).slice(0, $instructions_max).join('')) so the client matches
the server's mb_strlen behavior for $instructions_val/Instructions::OPTION_KEY.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ed0c2589-a2b7-4309-99f3-9a101405b3d7
⛔ Files ignored due to path filters (1)
dist/index.cjsis excluded by!**/dist/**
📒 Files selected for processing (8)
src/__tests__/unit/instructions.test.tssrc/index.tssrc/instructions.tswordpress-plugin/gk-block-api/includes/class-instructions.phpwordpress-plugin/gk-block-api/includes/class-rest-controller.phpwordpress-plugin/gk-block-api/includes/class-settings-page.phpwordpress-plugin/gk-block-api/readme.txtwordpress-plugin/gk-block-api/tests/Instructions/InstructionsTest.php
✅ Files skipped from review due to trivial changes (1)
- wordpress-plugin/gk-block-api/readme.txt
…on (BLOCK-19)
Two reviewer follow-ups; both valid against current code.
1. Settings page (`class-settings-page.php`): the HTML `maxlength`
attribute and `ta.value.length` count UTF-16 code units, while the
server (`mb_strlen`, `Instructions::MAX_LENGTH`) counts UTF-8 code
points. An astral character like 😀 is one code point but two
UTF-16 code units, so the previous client implementation blocked
input at ~1000 emoji while the server would accept 2000. Removed
the HTML `maxlength` attribute, switched the counter to
`Array.from(ta.value).length`, and added input-time enforcement
that trims by code points (`Array.from(...).slice(0, max).join('')`)
so the client matches the server character-for-character.
2. `src/instructions.ts` sanitize truncation: `s.length` and
`s.slice(0, MAX_ADDENDUM_LENGTH)` operate on UTF-16 code units and
can land in the middle of a surrogate pair, leaving a lone high
surrogate that downstream JSON serializers either reject or mangle.
Switched to `Array.from(s)` + array slice + `.join('')` so
truncation always lands on a code-point boundary.
New test:
- `truncates emoji-heavy input at code-point boundaries` —
`'😀'.repeat(MAX_ADDENDUM_LENGTH + 100)` → result has exactly
`MAX_ADDENDUM_LENGTH` code points and every code point is the full
emoji (never a lone surrogate).
Validated:
- vitest: 510 tests pass (was 509; +1 surrogate-pair test).
- PHPUnit: 626 / 9,198 assertions pass.
- phpcs: clean.
- tsc --noEmit: clean.
- esbuild build: clean.
Refs BLOCK-19.
… CBP blocks Two bugs in the kevinbatdorf/code-block-pro enricher surfaced while inserting an English chat-prompt code block via edit_block_tree replace-block on www.gravitykit.com: 1. Explicit `language: 'plaintext'` was treated as "no preference, infer." The original logic collapsed missing + 'plaintext' to the same code path, then ran inferLanguage(). A chat prompt containing the word "from" twice tripped the SQL signal and rendered with mis-coloured English keywords ("Set", "Block", "from", "URL", "and", "Password"). The fix differentiates three caller intents: • Missing / '' / 'auto' → run inferLanguage() • 'plaintext' / 'text' / 'plain' / 'txt' / 'none' → plaintext, NO inference • Anything else → use verbatim Callers that want auto-detect now opt in via 'auto' or by omitting the attribute; explicit 'plaintext' is respected. 2. When a CBP block arrived with no `innerHTML` (the normal case when inserted via the API — e.g. edit_block_tree replace-block), the enricher only populated the `codeHTML` attribute. The wrapper div was never built, so the block saved successfully but rendered as a blank gap on the front-end. The fix adds a build-from-scratch branch that emits a minimal wrapper: <div class="wp-block-kevinbatdorf-code-block-pro" style="font-family:…;font-size:…;line-height:…; background-color:…;color:…"> {codeHTML} {optional <textarea> for copy-button JS} </div> Style attributes mirror CBP save()'s inline-style behaviour; the copy-button textarea is included only when `copyButton:true`. Tests: 10 new vitest cases in src/__tests__/unit/enrichers/cbp-enricher.test.ts covering both fixes. Notable additions: • respects explicit plaintext language without inference • respects 'text', 'plain', 'txt', 'none' as plaintext aliases (it.each) • infers language when attribute is missing • infers language when attribute is 'auto' • regression: English prose with "from … from …" does not detect as SQL • builds wrapper innerHTML when block has none • inlines wrapper style from font / colour attributes • includes copy-textarea when copyButton is true • omits copy-textarea when copyButton is false • no-op contract: fully-enriched block (codeHTML + innerHTML both current) returns same object reference Existing 'returns original block reference when codeHTML already matches' test updated to pass a real fully-enriched block (codeHTML + innerHTML) — partial enrichment now correctly triggers wrapper rebuild instead of returning the half-built block as-is. Bumps @gravitykit/block-mcp to 1.7.1 (TS-server patch — plugin unchanged). Full vitest suite green: 520 tests; tsc --noEmit clean; esbuild bundle rebuilt.
Add a "Tagging the release" subsection to AGENTS.md under Versioning &
Releases. Captures the existing-in-practice rules so the next maintainer
doesn't have to reverse-engineer them from git log:
- Format: v{plugin-version} (matches plugin, not MCP server)
- Annotated, never lightweight
- Tag message = readme Upgrade Notice + Highlights bullets
- Push the tag explicitly (git push origin <tag>)
- Tag the merge commit on main; never the feature branch tip
- GitHub release optional, attaches to the tag
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/__tests__/unit/enrichers/cbp-enricher.test.ts (1)
137-144: ⚡ Quick winAdd a regression test for
</textarea>payload escaping.This suite now validates copy-textarea presence, but it does not yet lock in the new escape behavior for textarea-breaking payloads. Add one case where
codecontains</textarea>and assert it stays encoded ininnerHTML.Proposed test addition
+ it('escapes textarea-breaking payloads when copyButton is enabled', async () => { + const block: BlockDef = { + name: 'kevinbatdorf/code-block-pro', + attributes: { + code: '</textarea><script>alert(1)</script>', + language: 'javascript', + copyButton: true, + }, + }; + const result = await enrichBlock(block); + expect(result.innerHTML).toMatch( + /<textarea[^>]*><\/textarea><script>alert\(1\)<\/script><\/textarea>/, + ); + expect(result.innerHTML).not.toContain('</textarea><script>'); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/unit/enrichers/cbp-enricher.test.ts` around lines 137 - 144, Add a regression test to ensure payloads containing "</textarea>" are HTML-escaped in the enriched block: update the test suite that uses BlockDef and enrichBlock to add a case where attributes.code includes the literal string "</textarea>" (e.g., "const x = '</textarea>';") and assert that the returned result.innerHTML does not contain an unescaped "</textarea>" but instead contains the escaped form (so the assertion checks innerHTML does not match the raw "</textarea>" and/or matches the escaped sequence). Ensure the test is named clearly (e.g., "escapes closing textarea in code payload") and follows the existing pattern used in the 'includes copy-textarea when copyButton is enabled' test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/enrichers.ts`:
- Around line 268-280: The wrapper HTML currently interpolates unescaped attrs
values (attrs.fontFamily, attrs.fontSize, attrs.lineHeight, attrs.bgColor,
attrs.textColor, attrs.className) into style="" and class="" and into the hidden
textarea, which allows attribute/context injection; fix by escaping/encoding
these values before building styleParts, classNameExtra and copyTextarea (e.g.
run each value through a single helper like escapeAttr or encodeAttr that
replaces &, <, >, " and ' with HTML entities), use the escaped values when
pushing into styleParts and when composing classNameExtra, and ensure the
textarea content uses encodedCode (or is escaped similarly) before interpolating
into updatedInnerHTML so updatedInnerHTML, copyTextarea, styleAttr and
classNameExtra contain only safe, encoded strings.
---
Nitpick comments:
In `@src/__tests__/unit/enrichers/cbp-enricher.test.ts`:
- Around line 137-144: Add a regression test to ensure payloads containing
"</textarea>" are HTML-escaped in the enriched block: update the test suite that
uses BlockDef and enrichBlock to add a case where attributes.code includes the
literal string "</textarea>" (e.g., "const x = '</textarea>';") and assert that
the returned result.innerHTML does not contain an unescaped "</textarea>" but
instead contains the escaped form (so the assertion checks innerHTML does not
match the raw "</textarea>" and/or matches the escaped sequence). Ensure the
test is named clearly (e.g., "escapes closing textarea in code payload") and
follows the existing pattern used in the 'includes copy-textarea when copyButton
is enabled' test.
🪄 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: 774ea0a5-4e82-4de1-b7a3-e6f7bbe7b4e5
⛔ Files ignored due to path filters (1)
dist/index.cjsis excluded by!**/dist/**
📒 Files selected for processing (3)
package.jsonsrc/__tests__/unit/enrichers/cbp-enricher.test.tssrc/enrichers.ts
Comment on lines
+268
to
+280
| if (typeof attrs.fontFamily === 'string') styleParts.push(`font-family:${attrs.fontFamily}`); | ||
| if (typeof attrs.fontSize === 'string') styleParts.push(`font-size:${attrs.fontSize}`); | ||
| if (typeof attrs.lineHeight === 'string') styleParts.push(`line-height:${attrs.lineHeight}`); | ||
| if (typeof attrs.bgColor === 'string') styleParts.push(`background-color:${attrs.bgColor}`); | ||
| if (typeof attrs.textColor === 'string') styleParts.push(`color:${attrs.textColor}`); | ||
| const styleAttr = styleParts.length ? ` style="${styleParts.join(';')}"` : ''; | ||
| const classNameExtra = typeof attrs.className === 'string' && (attrs.className as string).trim() !== '' | ||
| ? ` ${(attrs.className as string).trim()}` | ||
| : ''; | ||
| const copyTextarea = attrs.copyButton | ||
| ? `<textarea style="display:none" aria-hidden="true">${encodedCode}</textarea>` | ||
| : ''; | ||
| updatedInnerHTML = `<div class="wp-block-kevinbatdorf-code-block-pro${classNameExtra}"${styleAttr}>${codeHTML}${copyTextarea}</div>`; |
There was a problem hiding this comment.
Escape attribute-interpolated values before building wrapper HTML.
Line 268–273 and Line 274–280 inject attrs.* values directly into style="" and class="". A crafted quote in className/style fields can break attribute context and inject arbitrary attributes/scripts into stored innerHTML.
Proposed fix
+ const escapeAttr = (value: string): string =>
+ value
+ .replace(/&/g, '&')
+ .replace(/"/g, '"')
+ .replace(/</g, '<')
+ .replace(/>/g, '>');
const styleParts: string[] = [];
- if (typeof attrs.fontFamily === 'string') styleParts.push(`font-family:${attrs.fontFamily}`);
- if (typeof attrs.fontSize === 'string') styleParts.push(`font-size:${attrs.fontSize}`);
- if (typeof attrs.lineHeight === 'string') styleParts.push(`line-height:${attrs.lineHeight}`);
- if (typeof attrs.bgColor === 'string') styleParts.push(`background-color:${attrs.bgColor}`);
- if (typeof attrs.textColor === 'string') styleParts.push(`color:${attrs.textColor}`);
+ if (typeof attrs.fontFamily === 'string') styleParts.push(`font-family:${escapeAttr(attrs.fontFamily)}`);
+ if (typeof attrs.fontSize === 'string') styleParts.push(`font-size:${escapeAttr(attrs.fontSize)}`);
+ if (typeof attrs.lineHeight === 'string') styleParts.push(`line-height:${escapeAttr(attrs.lineHeight)}`);
+ if (typeof attrs.bgColor === 'string') styleParts.push(`background-color:${escapeAttr(attrs.bgColor)}`);
+ if (typeof attrs.textColor === 'string') styleParts.push(`color:${escapeAttr(attrs.textColor)}`);
const styleAttr = styleParts.length ? ` style="${styleParts.join(';')}"` : '';
const classNameExtra = typeof attrs.className === 'string' && (attrs.className as string).trim() !== ''
- ? ` ${(attrs.className as string).trim()}`
+ ? ` ${escapeAttr((attrs.className as string).trim())}`
: '';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (typeof attrs.fontFamily === 'string') styleParts.push(`font-family:${attrs.fontFamily}`); | |
| if (typeof attrs.fontSize === 'string') styleParts.push(`font-size:${attrs.fontSize}`); | |
| if (typeof attrs.lineHeight === 'string') styleParts.push(`line-height:${attrs.lineHeight}`); | |
| if (typeof attrs.bgColor === 'string') styleParts.push(`background-color:${attrs.bgColor}`); | |
| if (typeof attrs.textColor === 'string') styleParts.push(`color:${attrs.textColor}`); | |
| const styleAttr = styleParts.length ? ` style="${styleParts.join(';')}"` : ''; | |
| const classNameExtra = typeof attrs.className === 'string' && (attrs.className as string).trim() !== '' | |
| ? ` ${(attrs.className as string).trim()}` | |
| : ''; | |
| const copyTextarea = attrs.copyButton | |
| ? `<textarea style="display:none" aria-hidden="true">${encodedCode}</textarea>` | |
| : ''; | |
| updatedInnerHTML = `<div class="wp-block-kevinbatdorf-code-block-pro${classNameExtra}"${styleAttr}>${codeHTML}${copyTextarea}</div>`; | |
| const escapeAttr = (value: string): string => | |
| value | |
| .replace(/&/g, '&') | |
| .replace(/"/g, '"') | |
| .replace(/</g, '<') | |
| .replace(/>/g, '>'); | |
| if (typeof attrs.fontFamily === 'string') styleParts.push(`font-family:${escapeAttr(attrs.fontFamily)}`); | |
| if (typeof attrs.fontSize === 'string') styleParts.push(`font-size:${escapeAttr(attrs.fontSize)}`); | |
| if (typeof attrs.lineHeight === 'string') styleParts.push(`line-height:${escapeAttr(attrs.lineHeight)}`); | |
| if (typeof attrs.bgColor === 'string') styleParts.push(`background-color:${escapeAttr(attrs.bgColor)}`); | |
| if (typeof attrs.textColor === 'string') styleParts.push(`color:${escapeAttr(attrs.textColor)}`); | |
| const styleAttr = styleParts.length ? ` style="${styleParts.join(';')}"` : ''; | |
| const classNameExtra = typeof attrs.className === 'string' && (attrs.className as string).trim() !== '' | |
| ? ` ${escapeAttr((attrs.className as string).trim())}` | |
| : ''; | |
| const copyTextarea = attrs.copyButton | |
| ? `<textarea style="display:none" aria-hidden="true">${encodedCode}</textarea>` | |
| : ''; | |
| updatedInnerHTML = `<div class="wp-block-kevinbatdorf-code-block-pro${classNameExtra}"${styleAttr}>${codeHTML}${copyTextarea}</div>`; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/enrichers.ts` around lines 268 - 280, The wrapper HTML currently
interpolates unescaped attrs values (attrs.fontFamily, attrs.fontSize,
attrs.lineHeight, attrs.bgColor, attrs.textColor, attrs.className) into style=""
and class="" and into the hidden textarea, which allows attribute/context
injection; fix by escaping/encoding these values before building styleParts,
classNameExtra and copyTextarea (e.g. run each value through a single helper
like escapeAttr or encodeAttr that replaces &, <, >, " and ' with HTML
entities), use the escaped values when pushing into styleParts and when
composing classNameExtra, and ensure the textarea content uses encodedCode (or
is escaped similarly) before interpolating into updatedInnerHTML so
updatedInnerHTML, copyTextarea, styleAttr and classNameExtra contain only safe,
encoded strings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes BLOCK-19.
Summary
Lets site admins paste plain-text rules under Settings → Block MCP and every connected MCP client receives them at handshake — no per-session rediscovery of conventions like callout className mapping, code-block theme choice, or doc structure rules. The TypeScript server fetches the addendum at startup and appends to the hard-coded baseline before
McpServeraccepts the first request.Concrete motivation: GravityKit BetterDocs callouts use
core/groupwithis-style-callout-*classNames, notcore/quote. Without this feature, every fresh Claude session ships the wrong block and a human corrects it after publish. With it, the rule lives in one option row and reaches every client at initialize.What's in the change
PHP plugin (gk-block-api → 1.7.0)
Instructionsservice class — option storage, sanitize, 2,000-char length cap, updated-at timestamp, per-IP rate-limit bucket.GET /gk-block-api/v1/instructions— public by design (clients fetch before any auth-gated tool call). Returns{ addendum, length, max_length, updated_at }withCache-Control: public, max-age=60. Rate-limited at 30 req/min per remote IP; IPs are SHA-256-hashed before becoming transient keys.manage_optionscap already cover the save path.TypeScript MCP server (@gravitykit/block-mcp → 1.7.0)
src/instructions.ts—BASELINE,sanitizeAddendum,combineInstructions,fetchAddendum,getInstructions. The baseline string moves out ofsrc/index.tsinto the module so the source of truth is single.main()callsgetInstructions(WORDPRESS_URL)before connecting the transport and upgrades the underlyingServer._instructionsfield in place. Fetch failures log to stderr and fall back to baseline-only — startup never blocks on the network.BLOCK_MCP_INSTRUCTIONS_OFF=1env var disables the fetch entirely.Security pass
settings_fields()); existing pattern.manage_options; WPoptions.phpenforces the same cap on save.esc_textarea,esc_attr,esc_htmlon output; description HTML run throughwp_kseswith an allow-list.wp_strip_all_tagsstrips HTML/PHP/script tags AND their content;strip_shortcodesremoves shortcodes (which are never executed on this value anyway); C0/C1 control chars + DEL stripped.rate_limit_exceededcode.Cache-Control: public, max-age=60so legitimate clients don't hammer the endpoint.uninstall.php.update_optionwrites from sibling plugins or database restores; TS client re-sanitizes the fetched payload.Test plan
71 new tests, full suites green.
tests/Instructions/InstructionsTest.php(option round-trip, sanitize variations, length cap, timestamp tracking, per-IP rate limit, no raw IP in option_name, IPv6 support, defense-in-depth re-sanitize).tests/REST/InstructionsRouteTest.php(response shape, unauthenticated access, Cache-Control header, dirty-option re-sanitize, 429 on rate-limit, per-IP isolation, updated_at contract).src/__tests__/unit/instructions.test.ts(BASELINE invariants, sanitize coverage incl. Bidi + zero-width strip, combine semantics, fetchAddendum with axios mocked, env-var opt-out, every failure path returns empty, defense-in-depth truncation).composer lint(phpcs): clean.npx tsc --noEmit: clean.npm run build(esbuild): clean.Files
New (5)
src/instructions.ts— TS module.src/__tests__/unit/instructions.test.ts— vitest suite.wordpress-plugin/gk-block-api/includes/class-instructions.php— PHP service.wordpress-plugin/gk-block-api/tests/Instructions/InstructionsTest.php— unit tests.wordpress-plugin/gk-block-api/tests/REST/InstructionsRouteTest.php— REST tests.Modified (8)
src/index.ts—getInstructions()call beforeconnect(); baseline imported from the new module.package.json—1.6.0 → 1.7.0.dist/index.cjs— rebuilt bundle.wordpress-plugin/gk-block-api/gk-block-api.php— Version header + constant1.6.1 → 1.7.0.wordpress-plugin/gk-block-api/includes/class-rest-controller.php— new route + handler.wordpress-plugin/gk-block-api/includes/class-settings-page.php— Instructions section, register_setting, reset sweep.wordpress-plugin/gk-block-api/readme.txt— Upgrade Notice + Changelog for 1.7.0.wordpress-plugin/gk-block-api/uninstall.php— sweep new options + per-IP transients.Summary by CodeRabbit
New Features
Tests
Chores
Documentation