fix(tasks): close structural masked links and the date-token arity hole in Slack blocks - #815
Merged
sroussey merged 2 commits intoAug 16, 2026
Conversation
…le in Slack blocks Block Kit says `<url|label>` a second way, structurally, and the delabeling remedy was purely lexical: a button, a rich_text `link` element and an overflow `option` each carry a `url` beside a label field with no `<` in the payload at all, so an attacker-chosen label masked an attacker-chosen destination in the DEFAULT config while the README claimed blocks links were reduced to their bare URL. The reduction now runs in the object branch of the deep walk, under the same `policy.stripLabels` that drives the lexical one, and keys on SHAPE rather than on a list of element types: an overflow `option` carries no `type` discriminator, so a type set cannot reach it, and a shape rule covers whatever url-bearing element Slack adds next. It is narrow because only Block Kit objects that ARE links carry a plain `url` beside a label — `image` uses `image_url`/`alt_text`, `video` uses `title_url`. The destination is kept and the label overwritten, never the reverse: a button stripped of its `url` with no live `action_id` behind it is an availability change, the same argument the broadcast rewrite already makes for rewriting rather than deleting. The date-token exemption had the same shape of hole. Slack's token is `<!date^ts^token_string^optional_link|fallback>`, so the four-field form carries both a label and a destination, and both regexes exempted it on a two-character prefix. The exemption is now the safe ARITY, built from one shared source string so the broadcast escape and the delabeler cannot drift, and each rule is independently correct because `stripSlackLinkLabels` is exported and callable alone. Its structural twin — the rich_text `date` element's optional `url` — is the one case where deleting is right, since the label comes from `format`/`fallback` and a date renders fine unlinked. Two deliberate behavior tightenings: a legitimately linked date token now needs `allow_markup`, and a date token whose fallback contains a `<` is a shape the matcher cannot finish verifying, so it is escaped whole rather than exempted with something unverified inside it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lgxtp7mQECdh7F2UT9CVwN
Coverage Report
File CoverageNo changed files found. |
…-slack-structural-links The base branch merged origin/main (main's streaming rework of FetchUrlTask, plus the re-applied bounded Retry-After parser). No conflicts here. Verified: bun scripts/test.ts task vitest -> 68 files green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lgxtp7mQECdh7F2UT9CVwN
sroussey
deleted the
claude/optimistic-goldberg-d74u5n-slack-structural-links
branch
August 24, 2026 18:48
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
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.
Stacked fix onto PR #744 — this branch targets
claude/notify-merge-main, notmain, so it reviews and merges as a follow-up to that PR rather than duplicating its diff.Two findings that must ship together: the second one's structural half (the rich_text
dateelement'surl) is implemented BY the first one's walk, and both rewrite the same two regexes and the same object branch.[HIGH] Structural masked links in
blocksbypassed delabelingThe masked-link remedy was purely lexical —
MASKED_LINKover string leaves — but Block Kit expresses the same link structurally, and the deep walk rewrote onlySTRUCTURAL_BROADCAST_TYPES(broadcast, usergroup). So a structural masked link passed untouched in the default config (noallow_markup, noallow_mentions), while the README claimed blocks links are reduced to their bare URL:{ "type": "button", "text": { "type": "plain_text", "text": "Deploy succeeded" }, "url": "https://evil.example/pwn" }There is no
<anywhere in that payload for the lexical rule to match, and Slack renders the attacker-chosen label over the attacker-chosen destination — the exact phishing primitiveescapeSlackTextandstripSlackLinkLabelsexist to remove.The reduction now runs in the object branch of
neutralizeSlackBroadcastsDeep, after the child map is built, gated on the samepolicy.stripLabelsthat drivesstripSlackLinkLabels. Reduced form is identical in both halves: keep the destination, drop the label.The rule is shape-driven, not type-enumerated, and that is the key decision. An overflow
optioncarriesurl+textwith notypediscriminator at all, so a type set cannot cover it; and a shape rule cannot miss a new element type Slack adds. It stays narrow because only Block Kit objects that ARE links carry a plainurlbeside a label —imageusesimage_url/alt_text,videousestitle_url— so a false positive has no shape to arrive in today.url+ stringtext(rich_textlink)text = urlurl+text: { text: string }(button, accessory, overflow option, any composition-object label)text.text = urlurl, no label field,type === "date"urlurl, no label field, anything elseThe
urlis never deleted from a button: a button with no url and no liveaction_idhandler behind it is an availability change — the same argument the broadcast rewrite already makes for rewriting rather than deleting. Thedateelement is the one case where deleting is right, because its label comes fromformat/fallbackand a date renders fine unlinked.[MEDIUM] The
<!date^…>exemption re-admitted an attacker-chosen link + labelBoth regexes matched a two-character prefix (
<!date^,<!) rather than the token's safe arity. Slack's date token is<!date^ts^token_string^optional_link|fallback>—token_stringcarries the rendered label andoptional_linkis a URL — so this survived verbatim in a blocks leaf with no flag set:The code comment justifying the exemption ("the
|part is a FALLBACK, not a label masking a destination") is true only of the two-field form.Decision: narrow the exemption to the safe arity. Rejected alternative: parse the token and strip only
optional_link— that requires rebuilding the token inside a lexical pass, fails open on any malformed arity, and gains nothing, since a token with no link cannot phish.Both regexes are now built from one shared source string so they cannot drift:
MASKED_LINKis narrowed too rather than relying on the broadcast pass having escaped everything else first —stripSlackLinkLabelsis exported and callable alone, so each rule must be independently correct.Verified against this branch's own cases:
<!date^1700000000^{date_short}|Nov 14>stays exempt; the four-field attack fails the lookahead at the third^(which[^<>^|]*cannot consume) and is escaped;<!DATE^…>and a non-numeric timestamp still fail closed.Two behavior tightenings
allow_markup. The four-field form is markup carrying a destination, so it belongs on the markup rung like every other link.<no longer matches the exemption and is escaped whole. This breaks the existing test"a broadcast inside a date token's fallback is still escaped", which assertedposted).toContain("<!date^"). It is updated deliberately — it now asserts<!date,<!channel, and that no live<!survives. A token whose shape the matcher cannot fully verify fails closed. The regex was not weakened to keep the old assertion.Tests
New
structural masked links in blocksdescribe: a rich_text link element is reduced to its destination; a button accessory's label cannot mask its url (accessory.urlandaction_idunchanged); an overflow option's label cannot mask its url (notypekey — pins the shape rule against a type-list regression); an image block is NOT treated as a link (narrowness guard);allow_markupkeeps a structural link intact andallow_mentionsleaves it verbatim (gate pins).New
date token aritydescribe: a date token carrying an optional link is escaped in blocks by default; the structuralrich_textdate element's optional link is stripped (urlkey absent, timestamp/format/fallback preserved); a date token with a non-numeric timestamp is escaped.The existing query-string test also gained an explicit byte-identical assertion on the
urlleaf, so that invariant stays pinned under the new rewrite. The two-field date token in blocks, and the three other date-token cases, keep passing — which is the whole point of narrowing rather than deleting.Schema description for
blocksandpackages/tasks/README.mdupdated to say the reduction covers both the<url|label>text form and the structural url+label form.Verification
bun scripts/test.ts task vitest— 67 files passed, 1117 passed / 24 skipped. All 7 new/changed assertions were confirmed failing on the base branch first, each for the stated reason.bun run format— no changes inpackages/tasksorpackages/test(it also reformats pre-existing unrelated files underexamples/andproviders/; those were reverted and are not in this diff).Generated by Claude Code