Skip to content

fix: harden telemetry data fallback in bash-lint and markdown-formatter#20

Merged
kyle-sexton merged 1 commit into
mainfrom
fix/telemetry-fallback-hardening
Jun 29, 2026
Merged

fix: harden telemetry data fallback in bash-lint and markdown-formatter#20
kyle-sexton merged 1 commit into
mainfrom
fix/telemetry-fallback-hardening

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

What

Follow-up to #19. Propagates two cross-plugin items I flagged when shipping biome-format:

1. Telemetry fallback hardening (the real change). bash-lint and markdown-formatter carried the same build_data_json jq-failure fallback that biome-format had before review hardened it: printf '{"tool":"%s","file":"%s","findings":[]}' "$TOOL" "$FILE_REL". A path containing a quote or backslash could corrupt the telemetry envelope. Both now emit a fixed empty-shape object instead. The path is near-unreachable (fires only if jq -n fails, and hook::emit_telemetry already drops the envelope when jq is absent), so dropping the values is harmless and strictly safer than risking malformed JSON. All three plugins' build_data_json are now consistent.

Patch-bumped so consumers pull the change via claude plugin update: bash-lint 0.1.0 → 0.1.1, markdown-formatter 0.1.1 → 0.1.2.

2. MultiEdit matcher — investigated, no change (moot). Codex flagged that Write|Edit omits MultiEdit. Verified against the current tools reference: the only file-modification tools are Edit, Write, NotebookEditMultiEdit is gone (merged into Edit), and the permission-format table confirms Edit(...) applies to "Edit, Write, NotebookEdit". So Write|Edit is already complete; NotebookEdit only touches .ipynb, which none of these plugins' extensions match. No matcher change warranted.

Verification

shellcheck, typos, editorconfig-checker, comment-hygiene all pass on the changed files. Both test suites green against real tools (bash-lint 31, markdown-formatter 41). claude plugin validate --strict passes for both plugins and the catalog.

🤖 Generated with Claude Code

Propagate the build_data_json hardening from biome-format (#19) to the
two sibling plugins that carry the identical pattern. The jq-failure
fallback no longer interpolates TOOL/FILE_REL — a path containing a
quote or backslash could corrupt the telemetry envelope. Emit a fixed
empty-shape object instead. The path is near-unreachable (it fires only
if `jq -n` fails, and hook::emit_telemetry already drops the envelope
when jq is absent), so dropping the values is harmless and strictly
safer than risking malformed JSON.

Patch-bump both so `claude plugin update` pulls the change:
bash-lint 0.1.0 -> 0.1.1, markdown-formatter 0.1.1 -> 0.1.2.

Note: the MultiEdit matcher follow-up was investigated and is moot —
the current Claude Code tools reference lists only Edit/Write/
NotebookEdit as file-modification tools (MultiEdit is gone), so the
existing Write|Edit matcher is already complete.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WWt8nQzh8BAoZXRZRt2ARU
@claude

claude Bot commented Jun 29, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 13s —— View job


PR Review

  • Gather context (PR diff, changed files)
  • Review bash-lint changes
  • Review markdown-formatter changes
  • Review version bumps in plugin.json files
  • Check alignment with CLAUDE.md guidelines
  • Post findings

Summary

Clean, minimal security hardening. The fix is correct and the rationale is well-documented. No blocking issues.


Security Fix — build_data_json fallback

The core change in both plugins replaces:

|| printf '{"tool":"%s","file":"%s","findings":[]}' "$TOOL" "$FILE_REL"

with:

|| printf '{"tool":"","file":"","findings":[]}'

This is correct. The old form was vulnerable to JSON injection: a path containing ", \, or control characters (all legal on POSIX filesystems) would silently emit malformed JSON. Concrete examples:

  • docs/"quoted".sh{"tool":"bash-lint","file":"docs/"quoted".sh","findings":[]} — structurally broken
  • Windows paths with backslashes — broken escape sequences

The fixed form emits a valid, constant envelope. Since the fallback only fires if jq -n itself fails (and hook::emit_telemetry already drops the envelope when jq is absent), losing the field values is harmless, as the PR body correctly notes.

All three plugins (biome-format, bash-lint, markdown-formatter) now have identical build_data_json implementations — the consistency goal is met.

Permalinks:


Minor Observation (non-blocking)

biome-format.sh aligns its --arg flags without extra spaces (--arg tool "$TOOL"), while the two updated plugins use extra spaces for column alignment (--arg tool "$TOOL"). This is purely cosmetic and both styles are valid — only worth a mention if you want the three files to be character-for-character identical, not just semantically identical.


Version Bumps

Both patch bumps are appropriate for a bug fix:


CLAUDE.md Alignment

  • Repo-agnostic: no hardcoded paths introduced. ✅
  • Plugin-form-safe: no reach-outs outside plugin root. ✅
  • No PII/secrets. ✅
  • Explicit semver versioned with a meaningful bump. ✅

Verdict: LGTM. The change is correct, safe, and consistent across all three plugins.

@kyle-sexton
kyle-sexton merged commit e1a1b3a into main Jun 29, 2026
16 checks passed
@kyle-sexton
kyle-sexton deleted the fix/telemetry-fallback-hardening branch June 29, 2026 19:27
kyle-sexton added a commit that referenced this pull request Jul 13, 2026
…findings (#156)

Post-publish behavior fixes for the `ai-briefing` plugin, surfaced by
bot review on the publish PR (#122). All were pre-existing engine
characteristics carried over in the migration; none block plugin
load/validate.

## Ships

- **Wire `grok-capture` (Wave 0 S0).** Registered `cmdGrokCapture` in
the runner dispatch (thin adapter over `grok-capture-agent.js`). `init
--grok-preload` now probes the Grok CLI and sets `config.grok_preload` /
`grok_preload_requested` — degrades to `grok_degraded` on a
missing/unsigned CLI, or hard-fails with `--require-grok`. `next-handle`
surfaces `grok_preload`; S3 `synthesize` merges Grok captures with
Chrome (`mergeTweetLists`, Chrome URLs win on dedup). Non-blocking:
preload-disabled or a capture error records `S0_grok_capture: skipped`
and exits 0 so Chrome Wave 1 always proceeds. Added
`grok-wave0-orchestration.test.js` to the `scripts` test script and
un-deferred the docs (SKILL.md standing default #20,
`references/runner-architecture.md`).
- **`source_type` no longer hard-coded `"twitter"`** in `state.js`
`appendToSeenItems` — honors `item.source_type ?? "twitter"`. This
runner is the Wave-1 X (twitter) loop, so the default stays correct for
its own items; the change generalizes the writer so the
main-session-driven RSS/Perplexity/GitHub waves (which append to the
same store per the documented `twitter|blog|changelog|github` schema)
can record their own source. No other in-plugin caller sets a
non-twitter source today — the threading is the forward-compatible fix,
deliberately, not a missed one.
- **Single model constant.** `lib/models.js` `BRIEFING_AGENT_MODEL`;
`synthesize-agent.js` + `categorize-agent.js` route through it (one edit
to bump).
- **`output/build/validate.js`** wraps `loadSlidesData()` in try/catch —
prints "run `emit-slides-data.js` first" and exits non-zero instead of a
raw dynamic-import stack.
- **JS-string injection hardening** in `chrome-extract.js`
`buildExtractorJs`: `handle` + `cutoffIso` are escaped for single-quoted
JS-string context (`\` then `'`) before substitution, via function
replacers so a `$` in the value can't form a replacement pattern.

Bumps plugin `version` 0.2.0 → 0.3.0 (behavior change → consumers
receive it on `/plugin marketplace update`).

## Verification

- `scripts` suite: `npm test` — all green incl. new
`grok-wave0-orchestration.test.js` (env-isolated from leaked
`CLAUDE_PLUGIN_DATA`).
- `output/build` suite: `node --test` — 2 pass.
- S3 Chrome-wins merge verified directly (dry-run skips the merge path).
- Escaping verified against a malicious `evil'];window.__pwned=1;//`
handle; backslash-doubling confirmed.
- `validate.js` diagnostic path exercised (no `slides-data.js` → clean
message, exit 1).
- `claude plugin validate plugins/ai-briefing --strict` — passed.

Refs melodic-software/medley#1457

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Touches browser-injected extractor JS and optional external Grok CLI
orchestration; failures are designed to degrade without blocking Chrome
Wave 1, but S3 merge semantics and seen-items `source_type` affect dedup
and downstream briefing content.
> 
> **Overview**
> **ai-briefing 0.3.0** wires optional **Grok Wave 0** into the
per-profile runner and folds in several publish-review fixes.
> 
> **Grok Wave 0** registers `grok-capture` in the runner. `init
--grok-preload` probes the Grok CLI, sets `config.grok_preload` /
`grok_degraded`, and supports `--require-grok` for hard failure.
`cmdGrokCapture` is non-blocking (skip + exit 0 on disabled preload or
errors). S3 **synthesize** merges Grok and Chrome posts via
`mergeTweetLists` (Chrome wins on URL dedup). Docs and CLI usage drop
the “not yet wired” deferral; tests add
`grok-wave0-orchestration.test.js`.
> 
> **Other behavior changes:** `appendToSeenItems` uses `item.source_type
?? "twitter"` for forward-compatible RSS/GitHub waves.
**`BRIEFING_AGENT_MODEL`** in `models.js` centralizes the S3/S4 `claude
-p` model. **`buildExtractorJs`** escapes handle/cutoff for
single-quoted JS literals. **`validate.js`** fails clearly when
`slides-data.js` is missing.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
c602645. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
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