diff --git a/lib/hook-utils.sh b/lib/hook-utils.sh index b854e2598c..411c4a6731 100644 --- a/lib/hook-utils.sh +++ b/lib/hook-utils.sh @@ -279,6 +279,11 @@ hook::jq_field() { # A quoted assignment value (e.g. `TOKEN="a b" curl …`) would otherwise leak a # fragment of the value into the token, so any token carrying a quote aborts to a # bare "Bash" subject rather than risk exposing part of the value. +# +# A bare or trailing unquoted assignment that no following command consumed +# (e.g. the whole command is `TOKEN=ghp_…`) is likewise a value the subject must +# not carry, so a resolved token still shaped like a NAME=value assignment aborts +# to the bare "Bash" subject too. # SUBJECT=$(hook::extract_bash_subject "$TOOL" "$CMD") hook::extract_bash_subject() { local tool="$1" cmd="${2:-}" @@ -307,6 +312,19 @@ hook::extract_bash_subject() { printf '%s' "$tool" return 0 fi + # A resolved token still shaped like a bare/trailing assignment (no following + # command word consumed it in the strip loop) would emit the assignment's + # value — a possible credential — as the subject; bail to the bare "Bash" + # subject as with a quoted value. All valid Bash assignment forms count: + # NAME=value, append NAME+=value, and subscripted NAME[idx]=value / + # NAME[idx]+=value — the subscript matched greedily (`.*`) because Bash + # accepts nested subscripts like NAME[1+IDX[0]]=value, which a + # no-close-bracket class would miss. This runs BEFORE the basename strip so + # a path-valued assignment (TOKEN=/a/b/secret) cannot lose its "=" first. + if [[ "$first_token" =~ ^[a-zA-Z_][a-zA-Z0-9_]*(\[.*\])?\+?= ]]; then + printf '%s' "$tool" + return 0 + fi first_token="${first_token##*/}" if [[ -n "$first_token" ]]; then printf 'Bash:%s' "$first_token" diff --git a/lib/hook-utils.test.sh b/lib/hook-utils.test.sh index d632a5ccc0..d2dfacc378 100755 --- a/lib/hook-utils.test.sh +++ b/lib/hook-utils.test.sh @@ -860,6 +860,55 @@ else fi rm -f "$tel19" "$sink19" +# --- hook::extract_bash_subject: privacy-safe subject reduction -------------- +# The subject is emitted verbatim into hook-events.jsonl and any wired +# HOOK_TELEMETRY_SINK, so it must never carry an assignment VALUE (a credential). +subject_is() { + local desc="$1" tool="$2" cmd="$3" want="$4" got + got=$(hook::extract_bash_subject "$tool" "$cmd") + if [[ "$got" == "$want" ]]; then + ok "extract_bash_subject: $desc" + else + fail "extract_bash_subject ($desc): want [$want] got [$got]" + fi +} + +# Leak case (the fix): a command whose LAST token is an unquoted assignment must +# NOT emit the value — it bails to the bare "Bash" subject. +subject_is "bare trailing assignment bails to Bash" \ + "Bash" "TOKEN=ghp_realtokenvalue" "Bash" +# A path-valued trailing assignment must bail BEFORE the basename strip, or the +# value's basename would leak (TOKEN=/a/b/secret -> "secret"). +subject_is "path-valued trailing assignment bails (no basename leak)" \ + "Bash" "TOKEN=/a/b/secret" "Bash" +# Multiple assignments with no following command are all value — bail. +subject_is "trailing multi-assignment bails to Bash" \ + "Bash" "VAR=x TOKEN=secret" "Bash" +# Every valid Bash assignment form counts: append and subscripted assignments +# carry the value just the same. +subject_is "trailing append assignment bails to Bash" \ + "Bash" "TOKEN+=ghp_secret" "Bash" +subject_is "trailing subscripted assignment bails to Bash" \ + "Bash" "TOKEN[0]=ghp_secret" "Bash" +subject_is "trailing subscripted append assignment bails to Bash" \ + "Bash" "TOKEN[0]+=ghp_secret" "Bash" +subject_is "trailing nested-subscript assignment bails to Bash" \ + "Bash" "TOKEN[1+INDEX[0]]=ghp_secret" "Bash" + +# Preserved: a following real command wins the token, so the assignment prefix is +# stripped and the command name is the subject. +subject_is "leading assignment prefix then a command yields the command" \ + "Bash" "VAR=x realcmd --flag arg" "Bash:realcmd" +# Preserved: a quoted assignment value still hits the quote-bail. +subject_is "quoted assignment value bails to Bash" \ + "Bash" 'TOKEN="a b" curl https://x' "Bash" +# Preserved: an ordinary command yields its basename subject, no tail. +subject_is "ordinary command yields basename subject" \ + "Bash" "/usr/bin/git status --short" "Bash:git" +# Preserved: a non-Bash tool returns its name unchanged. +subject_is "non-Bash tool returns the tool name" \ + "Write" "irrelevant" "Write" + echo echo "PASS=$PASS FAIL=$FAIL" [[ $FAIL -eq 0 ]] diff --git a/plugins/actionlint/.claude-plugin/plugin.json b/plugins/actionlint/.claude-plugin/plugin.json index a52503bad3..3d4b7754a0 100644 --- a/plugins/actionlint/.claude-plugin/plugin.json +++ b/plugins/actionlint/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "actionlint", - "version": "0.7.1", + "version": "0.7.2", "description": "Lint GitHub Actions workflow files on edit via actionlint, surfacing findings as advisory context.", "author": { "name": "Melodic Software", diff --git a/plugins/actionlint/CHANGELOG.md b/plugins/actionlint/CHANGELOG.md index eb6a7a9664..4c5873b3d4 100644 --- a/plugins/actionlint/CHANGELOG.md +++ b/plugins/actionlint/CHANGELOG.md @@ -3,6 +3,22 @@ All notable changes to the `actionlint` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.7.2] + +### Fixed + +- **Shared `hook-utils.sh`: a bare or trailing unquoted `NAME=value` Bash + command no longer leaks the assignment value into the privacy-safe + telemetry/audit subject.** `hook::extract_bash_subject` stripped a leading + `VAR=value` prefix only when a following command word consumed it, so a + command whose LAST token was an unquoted assignment (e.g. `TOKEN=ghp_…`) + survived to the subject and emitted `Bash:TOKEN=ghp_…` into + `hook-events.jsonl` and any wired `HOOK_TELEMETRY_SINK`. A resolved token + still shaped like a shell assignment now bails to the bare `Bash` subject, + matching the existing quoted-value bail (`VAR=x cmd` still reduces to + `Bash:cmd`). Synced from `lib/hook-utils.sh`; the subject is + telemetry/audit-only, so no guard or formatter block/allow behavior changes. + ## [0.7.1] ### Fixed diff --git a/plugins/actionlint/hooks/hook-utils.sh b/plugins/actionlint/hooks/hook-utils.sh index b854e2598c..411c4a6731 100644 --- a/plugins/actionlint/hooks/hook-utils.sh +++ b/plugins/actionlint/hooks/hook-utils.sh @@ -279,6 +279,11 @@ hook::jq_field() { # A quoted assignment value (e.g. `TOKEN="a b" curl …`) would otherwise leak a # fragment of the value into the token, so any token carrying a quote aborts to a # bare "Bash" subject rather than risk exposing part of the value. +# +# A bare or trailing unquoted assignment that no following command consumed +# (e.g. the whole command is `TOKEN=ghp_…`) is likewise a value the subject must +# not carry, so a resolved token still shaped like a NAME=value assignment aborts +# to the bare "Bash" subject too. # SUBJECT=$(hook::extract_bash_subject "$TOOL" "$CMD") hook::extract_bash_subject() { local tool="$1" cmd="${2:-}" @@ -307,6 +312,19 @@ hook::extract_bash_subject() { printf '%s' "$tool" return 0 fi + # A resolved token still shaped like a bare/trailing assignment (no following + # command word consumed it in the strip loop) would emit the assignment's + # value — a possible credential — as the subject; bail to the bare "Bash" + # subject as with a quoted value. All valid Bash assignment forms count: + # NAME=value, append NAME+=value, and subscripted NAME[idx]=value / + # NAME[idx]+=value — the subscript matched greedily (`.*`) because Bash + # accepts nested subscripts like NAME[1+IDX[0]]=value, which a + # no-close-bracket class would miss. This runs BEFORE the basename strip so + # a path-valued assignment (TOKEN=/a/b/secret) cannot lose its "=" first. + if [[ "$first_token" =~ ^[a-zA-Z_][a-zA-Z0-9_]*(\[.*\])?\+?= ]]; then + printf '%s' "$tool" + return 0 + fi first_token="${first_token##*/}" if [[ -n "$first_token" ]]; then printf 'Bash:%s' "$first_token" diff --git a/plugins/ai-briefing/.claude-plugin/plugin.json b/plugins/ai-briefing/.claude-plugin/plugin.json index 9f1f3e1868..5aafadb1d1 100644 --- a/plugins/ai-briefing/.claude-plugin/plugin.json +++ b/plugins/ai-briefing/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "ai-briefing", - "version": "0.6.2", + "version": "0.6.3", "description": "Build source-backed AI-industry briefings from official vendor publications, configured RSS/Atom feeds, GitHub releases, reputable secondary reporting, and user-supplied URLs. Deduplicate, rank, and present results as markdown or optional HTML/PPTX decks, with repository-owned profile, audience, and brand configuration. Automated X/Twitter collection is disabled; Playwright is used only for deterministic local rendering.", "author": { "name": "Melodic Software", diff --git a/plugins/ai-briefing/CHANGELOG.md b/plugins/ai-briefing/CHANGELOG.md index e64c19e1e4..e096b6ec35 100644 --- a/plugins/ai-briefing/CHANGELOG.md +++ b/plugins/ai-briefing/CHANGELOG.md @@ -3,6 +3,61 @@ All notable changes to the `ai-briefing` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.6.3] + +### Security + +- **Source-URL schemes are allowlisted at every deck sink.** A shared + `lib/url-policy.js` seam now exposes `isAllowedUrlScheme`, reused at schema + validation and at each href/hyperlink sink. `http:`, `https:`, `mailto:`, and + `tel:` — the schemes a legitimate briefing may contain, inert at every sink — + are preserved and continue to render as working links. **Every other scheme is + now rejected**: the `javascript:`, `data:`, and `file:` attack vectors that + could inject script into the HTML deck or embed a local-file hyperlink in the + PPTX, and — as deliberate fail-closed hardening — rarer schemes such as `ftp:` + that the previous permissive `z.string().url()` accepted. Two layers: the Zod + schema hard-fails a deck containing a disallowed scheme (loud fail-closed on an + attack indicator), and the HTML and PPTX builders drop the individual unsafe + link (defense-in-depth on the `--skip-emit` rebuild path). +- **Link-reachability checks refuse non-global hosts (SSRF guard).** + `shouldSkipLinkCheck` now skips URLs whose literal host falls in any + non-global block of the IANA special-purpose registries, not just RFC1918: + loopback, private, link-local, shared address space (CGN), benchmarking, + documentation TEST-NETs, IETF protocol assignments, multicast, and reserved + (127/8, 10/8, 100.64/10, 172.16/12, 192.168/16, 169.254/16, 0/8, 192.0.0/24, + 192.0.2/24, 198.18/15, 198.51.100/24, 203.0.113/24, 192.88.99/24 deprecated + 6to4 relay anycast, 224/4, 240/4, + `localhost`/`*.localhost`) — the IPv4 list is complete against the registry; + the only rows omitted are those it marks globally reachable (the AS112, AMT, + PCP and TURN anycast assignments). A deny list is the correct shape for IPv4, + unlike IPv6 below: global unicast is not one prefix but 1.0.0.0 through + 223.255.255.255 minus the carve-outs, so the two ends are handled by range and + the middle needs the registry's blocks enumerated either way. Matching relies + on WHATWG URL canonicalization of + decimal/hex/octal/integer IPv4. IPv6 is judged by ALLOWLIST rather than by an + enumerated deny list: only globally reachable unicast space (`2000::/3`) + survives, and the IANA IPv6 Special-Purpose Address Registry's non-global + blocks inside it are carved back out (`2001::/23` IETF protocol assignments — + Teredo, benchmarking `2001:2::/48`, ORCHIDv2, AMT and the anycast singletons — + plus `2001:db8::/32` and `3fff::/20` documentation and `2002::/16` 6to4, which + wraps an arbitrary IPv4 tunnel endpoint). So `::`/`::1`, `fc00::/7`, + `fe80::/10`, `ff00::/8`, `100::/64`, `100:0:0:1::/64`, `5f00::/16` and every + unassigned or newly registered block are refused by default rather than read + as public — closing the class of bypass a deny list reopens each time a + prefix nobody enumerated turns out to be routable. RFC 8215's local-use + translation prefix `64:ff9b:1::/48` is refused outright, while IPv4-mapped and + NAT64 `64:ff9b::/96` forms are judged by their embedded IPv4 address. Literal + parsing also accepts RFC 4291 form 3 (a trailing dotted quad), which a + resolver can answer with and which reading as hex silently misread + (`192.168.1.1` as `0x192`). A DNS-name + host is additionally resolved at gate time (every A/AAAA record) and refused + when ANY resolved address is non-global, so a hostname whose record points + at, e.g., the cloud metadata address is never handed to the checker; an + unresolvable or unreadable answer fails closed. Residual: the checker + performs its own resolution at fetch time, so a rebind between this gate and + the fetch, or a redirect hop to a private target inside the checker, remains + outside this gate. + ## [0.6.2] ### Fixed diff --git a/plugins/ai-briefing/skills/generate/output/build/build-pptx.js b/plugins/ai-briefing/skills/generate/output/build/build-pptx.js index 1dbf59d44a..c714dc507d 100644 --- a/plugins/ai-briefing/skills/generate/output/build/build-pptx.js +++ b/plugins/ai-briefing/skills/generate/output/build/build-pptx.js @@ -5,6 +5,7 @@ import path from "node:path"; import fs from "node:fs"; import { fileURLToPath } from "node:url"; import { loadSlidesData, meetingsDir } from "./lib/paths.js"; +import { isAllowedUrlScheme } from "./lib/url-policy.js"; const __dirname = path.dirname(fileURLToPath(import.meta.url)); const { meta, theme, slides } = await loadSlidesData(); @@ -317,7 +318,7 @@ function buildNews(s, slide) { }); // ALL URLs — each as separate hyperlinked line - const urls = b.urls ?? []; + const urls = (b.urls ?? []).filter(isAllowedUrlScheme); const urlBlock = urls.flatMap((u, j) => [ ...(j > 0 ? [{ text: "\n", options: { fontSize: 4 } }] : []), { text: u, options: { fontFace: FONT_BODY, fontSize: 8.5, color: theme.accent, hyperlink: { url: u } } }, @@ -383,7 +384,7 @@ function buildCondensed(s, slide) { valign: "top", }); - const urls = b.urls ?? []; + const urls = (b.urls ?? []).filter(isAllowedUrlScheme); if (urls.length > 0) { const urlText = urls.flatMap((u, j) => [ ...(j > 0 ? [{ text: "\n", options: { fontSize: 3 } }] : []), diff --git a/plugins/ai-briefing/skills/generate/output/build/build-sections.js b/plugins/ai-briefing/skills/generate/output/build/build-sections.js index 03310c8b09..84dd253b3f 100644 --- a/plugins/ai-briefing/skills/generate/output/build/build-sections.js +++ b/plugins/ai-briefing/skills/generate/output/build/build-sections.js @@ -1,6 +1,7 @@ // Section grouping and HTML fragment generation for single-file HTML deck. import { formatUrlDisplay } from "./lib/url-display.js"; +import { isAllowedUrlScheme } from "./lib/url-policy.js"; export const escape = (s) => String(s ?? "") @@ -197,7 +198,7 @@ function renderNewsSection(sectionKey, sectionSlides, providerLogoSvg) {

${heading}