From cfba02f883c4990610a4c79556abbdf70f2441f3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 06:00:35 +0000 Subject: [PATCH] fix: stop stripping header `level` from sent Block Kit payloads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `toSlackBlocks` deleted the `level` field from every header block on the way out, on the assumption that it was a builder-only extension Slack would reject. That assumption was wrong: `level` is a real Slack field on the header block — an optional integer 1-4 — so the builder silently dropped the heading level from every message it sent. The bug was invisible right up to the send. The preview and the JSON drawer both read the working draft, which still carries `level`; only the payload handed to `chat.postMessage` / `chat.update` had it removed. Confirmation that `level` is legitimate: `@tightknitai/slack-block-kit-validator` lists it among the header block's allowed keys under `additionalProperties: false` and validates it as an integer with minimum 1 / maximum 4. An unknown property would be rejected outright. Drop the header carve-out and keep `sanitizeBlock` — the URL scrubbing and retrieval-only-key stripping are unrelated and still needed. Since validation runs on `toSlackBlocks(...)` output, `level` now reaches the validator for the first time: values 1-4 pass, and out-of-range values correctly surface as validation errors instead of being discarded. Also corrects the JSDoc, the `HeaderLevel` type docs, the editor help text, and the README, all of which repeated the same wrong assumption. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018Ao4W1Fn3JoatgX3RS6h68 --- README.md | 2 +- .../editors/block-editor.stories.tsx | 16 ++++++++ src/components/editors/header-editor.tsx | 4 +- src/lib/to-slack-blocks.ts | 33 ++++++++-------- src/types.ts | 10 +++-- test/public-api.test.ts | 38 ++++++++++++++++++- 6 files changed, 78 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index f121d21..7aeaeeb 100644 --- a/README.md +++ b/README.md @@ -447,7 +447,7 @@ Helpers and send-flow primitives also exported: ```ts import { - toSlackBlocks, // strips builder-only fields (e.g. header `level`) before sending + toSlackBlocks, // scrubs unsafe URLs + retrieval-only fields before sending encodeBlocksToString, // base64url-encode a blocks array (for URL state) decodeBlocksFromString, defaultPalette, // the built-in palette — spread to customize diff --git a/src/components/editors/block-editor.stories.tsx b/src/components/editors/block-editor.stories.tsx index 74b88a8..7db2539 100644 --- a/src/components/editors/block-editor.stories.tsx +++ b/src/components/editors/block-editor.stories.tsx @@ -332,6 +332,22 @@ export const TypingHeaderTextProducesValidBlock: Story = { } }; +// Regression: header `level` is a real Slack field, so picking a level in +// the editor must survive `toSlackBlocks` and reach the wire payload. +// It used to be stripped on the way out, silently discarding the choice. +export const SelectingHeaderLevelProducesValidBlock: Story = { + args: { block: variant('structure_header') }, + play: async ({ canvasElement, args }) => { + const canvas = within(canvasElement); + await userEvent.click(await canvas.findByLabelText(/^h2$/i)); + await expect(args.onChange).toHaveBeenCalled(); + const latest = expectLastOnChangeIsValid(args.onChange as ReturnType); + expect(latest.type).toBe('header'); + expect(latest).toHaveProperty('level', 2); + expect(toSlackBlocks([latest])[0]).toHaveProperty('level', 2); + } +}; + export const TypingMarkdownTextProducesValidBlock: Story = { args: { block: variant('markdown_list') }, play: async ({ canvasElement, args }) => { diff --git a/src/components/editors/header-editor.tsx b/src/components/editors/header-editor.tsx index a0c7a36..0961525 100644 --- a/src/components/editors/header-editor.tsx +++ b/src/components/editors/header-editor.tsx @@ -13,7 +13,7 @@ const HEADER_LEVELS: HeaderLevel[] = [1, 2, 3, 4]; /** * Editor form for header blocks. Slack enforces plain text only. * Shows a character count against the 150-char Slack limit, plus an - * optional 1-4 level selector (builder-only extension; Slack ignores it). + * optional 1-4 heading level selector. * @param props - editor props * @param props.block - the header block to edit * @param props.onChange - called with the updated block payload @@ -44,7 +44,7 @@ export function HeaderEditor({ block, onChange }: BlockEditorProps - + { diff --git a/src/lib/to-slack-blocks.ts b/src/lib/to-slack-blocks.ts index 55364d0..fc83f34 100644 --- a/src/lib/to-slack-blocks.ts +++ b/src/lib/to-slack-blocks.ts @@ -2,24 +2,23 @@ import type { SupportedBlock } from '../types'; import { sanitizeBlock } from './sanitize-blocks'; /** - * Strip builder-only fields from blocks so they conform to Slack's - * Block Kit schema, and scrub dangerous URI schemes from any - * `url`/`image_url` fields. Currently removes the cosmetic `level` - * field from header blocks (a builder-only extension Slack would - * reject) and routes every URL/image-url through the allowlist in - * `lib/url-safety.ts` so a payload that round-trips through the - * builder cannot carry `javascript:`/`data:text/html` URIs to a - * downstream consumer or to the Slack API. + * Prepare blocks for the Slack API: scrub dangerous URI schemes from any + * `url`/`image_url` fields and drop the read-only metadata Slack attaches + * on retrieval but rejects on send. Every URL/image-url is routed through + * the allowlist in `lib/url-safety.ts` so a payload that round-trips + * through the builder cannot carry `javascript:`/`data:text/html` URIs to + * a downstream consumer or to the Slack API. + * + * Header `level` is passed through: it is a real Slack field (an integer + * 1-4, see https://docs.slack.dev/reference/block-kit/blocks/header-block), + * not a builder-only extension. Earlier versions stripped it here, which + * silently dropped the heading level from every sent message. Out-of-range + * values are left intact so they surface as validation errors rather than + * being quietly discarded. + * * @param blocks - the working draft blocks - * @returns blocks with builder-only fields removed and URLs scrubbed + * @returns blocks with retrieval-only fields removed and URLs scrubbed */ export function toSlackBlocks(blocks: SupportedBlock[]): SupportedBlock[] { - return blocks.map((block) => { - const safe = sanitizeBlock(block); - if (safe.type === 'header' && 'level' in safe) { - const { level: _omit, ...rest } = safe; - return rest as SupportedBlock; - } - return safe; - }); + return blocks.map((block) => sanitizeBlock(block)); } diff --git a/src/types.ts b/src/types.ts index 0a1285d..ee50877 100644 --- a/src/types.ts +++ b/src/types.ts @@ -407,15 +407,17 @@ export interface ContainerBlock { } /** - * Header heading level shown in the preview. Slack's API has no - * `level` field on header blocks, so this is a builder-only extension - * that round-trips on the block payload but is otherwise cosmetic. + * Header heading level. A real Slack field on header blocks — an optional + * integer 1-4 that Slack renders at the corresponding heading size — so it + * round-trips through {@link SupportedBlock} and is sent to the API. + * @see https://docs.slack.dev/reference/block-kit/blocks/header-block */ export type HeaderLevel = 1 | 2 | 3 | 4; /** * Header block as edited by the builder. Extends Slack's HeaderBlock - * with an optional {@link HeaderLevel}. + * with an optional {@link HeaderLevel}, which the bundled + * `slack-web-api-client` types predate. */ export type SupportedHeaderBlock = HeaderBlock & { level?: HeaderLevel }; diff --git a/test/public-api.test.ts b/test/public-api.test.ts index 6f49320..dcbb0ea 100644 --- a/test/public-api.test.ts +++ b/test/public-api.test.ts @@ -6,7 +6,10 @@ import { decodeBlocksFromString, encodeBlocksToString } from '../src/lib/url-sta import type { SupportedBlock } from '../src/types'; describe('toSlackBlocks', () => { - it('strips the builder-only `level` field from header blocks', () => { + // Regression: `level` is a real Slack header field (integer 1-4), not a + // builder-only extension. It used to be stripped here, which silently + // dropped the heading level from every message the builder sent. + it('preserves the `level` field on header blocks', () => { const input: SupportedBlock[] = [ { type: 'header', @@ -17,7 +20,40 @@ describe('toSlackBlocks', () => { const [out] = toSlackBlocks(input); expect(out.type).toBe('header'); + expect(out).toEqual(input[0]); + }); + + it('emits header `level` in a payload the validator accepts', () => { + for (const level of [1, 2, 3, 4]) { + const payload = toSlackBlocks([ + { + type: 'header', + level, + text: { type: 'plain_text', text: 'Heading', emoji: true } + } as SupportedBlock + ]); + expect(payload[0]).toHaveProperty('level', level); + expect(validateBlockKit(payload, { target: 'blocks' }).valid).toBe(true); + } + }); + + it('leaves an out-of-range header `level` intact so it surfaces as a validation error', () => { + const payload = toSlackBlocks([ + { + type: 'header', + level: 6, + text: { type: 'plain_text', text: 'Heading', emoji: true } + } as unknown as SupportedBlock + ]); + expect(payload[0]).toHaveProperty('level', 6); + expect(validateBlockKit(payload, { target: 'blocks' }).valid).toBe(false); + }); + + it('leaves a header block without a `level` untouched', () => { + const input: SupportedBlock[] = [{ type: 'header', text: { type: 'plain_text', text: 'Heading', emoji: true } }]; + const [out] = toSlackBlocks(input); expect('level' in out).toBe(false); + expect(out).toBe(input[0]); }); it('passes non-header blocks through unchanged', () => {