fix(security): systemic fix for T6 formula injection via centralized safe-writes#129
Merged
Conversation
Captures the redefinition of T6 as a vulnerability class (not a single call site), the centralized safe-writes.ts architecture, the full call-site inventory, and the CI enforcement mechanism agreed on with the user across this design session. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Grounding markdown is included in the safe-write path (not carved out as an exception), consistent with the blanket-sanitization decision. - Unify writeSafeValue/writeSafeValueGrid on the same unknown-typed, inline typeof guard instead of a separately named adapter — removes an arbitrary string/unknown split between the two plain-value primitives while keeping current call sites behaviorally unchanged. - Fix markdownlint table-separator formatting. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
8 tasks: build safe-writes.ts (TDD, all 4 primitives + relocated sanitizeForCell/writeColumn/findOrCreateColumn), migrate extractText, runBatchAI, formatMarkdownSelection, and sampleRowsToEvaluation onto it, add an ESLint no-restricted-syntax enforcement rule, update the threat model, and manually verify against the dev sheet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…emic-t6-safe-writes
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Found while adding Task 6's ESLint rule: two raw setValue() calls that create the output/grounding column headers were missed by the original call-site audit. Amending the plan before fixing, per user sign-off. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
src/server/through a newsrc/server/safe-writes.tsmodule (four primitives —writeSafeValue/writeSafeValueGridfor plain text,writeSafeRichText/writeSafeRichTextGridfor rich text), closing the AI-75/AI-76 gaps plus three previously-unknown gaps found during this work (formatMarkdownSelection,sampleRowsToEvaluation's copy, andrunBatchAI's column-header creation).no-restricted-syntaxrule forbidding any rawsetValue/setValues/setRichTextValue/setRichTextValuescall insrc/server/**/*.tsoutsidesafe-writes.ts, so a future write site can't silently reintroduce this exact recurring bug class.docs/superpowers/specs/2026-07-15-t6-systemic-safe-writes-design.md,docs/superpowers/plans/2026-07-15-t6-systemic-safe-writes-plan.md) for reviewers who want the "why" behind each architectural decision.Manual QA
=SUM(1,2). Confirm the output cell displays the literal text=SUM(1,2), not the computed value3.=IMPORTDATA("evil.com")as plain text (typed directly into the Doc, not as a live formula). Confirm the output cell displays it as literal text.'so it doesn't evaluate), select it, and run "Format Markdown Selection." Confirm the cell still displays literal text afterward, not a live formula._evaluationsheet. Confirm the copied cell still displays as literal text there, not a live, recomputed formula.Security
Threat model updated in this PR (see
docs/threat_models/ssi-toolkit-threat-model.md— T6 rewrite, R45, superseded R8/R21/R22, consolidated Open Items).Notes
This branch was implemented via subagent-driven development: 9 sub-tasks (including one gap discovered and fixed mid-implementation —
runBatchAI's column-header creation, found while adding the ESLint rule), each independently reviewed against its brief, plus a final whole-branch review (verdict: ready to merge, no Critical/Important findings). Before merging pastdevelop, please run the 5 manual QA steps above against the dev sheet — the automated verification is solid but the live-sheet behavior (items 1-4 in particular, which are the actual security-relevant checks) has not been human- or browser-verified yet.