I built a small Raycast extension - paste invoice JSON, get a formatted HTML/Markdown preview. Zero dependencies in the render logic, 7 unit tests passing, tsc clean. I was pretty confident in it before opening the PR to the official extensions repo.
Within a few minutes, an automated reviewer (Greptile, which Raycast runs on every incoming PR) left 9 comments. Four were tagged P1/security. None of them were things my tests caught, because I'd only tested the happy path and a couple of "normal" edge cases - not actually hostile input.
1. Unescaped field reaching an HTML template
// before
export function computeTotals(data: InvoiceData) {
const subtotal = data.items.reduce((acc, i) => acc + i.qty * i.rate, 0);
const taxRate = data.tax_rate || 0;
...
}
tax_rate is typed as number, but nothing enforces that at runtime - it comes from JSON.parse on whatever the user pastes. Paste "tax_rate": "0\"><script>alert(1)</script>" and it reaches the HTML template unescaped, because I was escaping the fields I "knew" were strings and trusting the type annotation for the one I assumed was always a number.
2. Markdown injection as a tracking pixel
The extension also renders a Markdown preview (notes field goes straight into a Raycast Detail view, which renders Markdown). A notes value like  turns into an image markdown tag that fires a real HTTP request to whatever URL the attacker put there, the moment you preview the invoice. I was escaping HTML special characters everywhere, but Markdown has its own special-character set (` * _ { } [ ] ( ) # + . ! | < > -) and I hadn't touched it.
3. A crash from a null in an array
{"items": [null]}
passed my one type-level guard (Array.isArray(data.items) && data.items.length > 0) and then blew up inside computeTotals when it tried to read .qty off null. A single bad row in pasted JSON took down the whole preview instead of just that row.
4. Silent data loss on save
// before
const file = path.join(dir, `${(data.invoice_number || "invoice").replace(/[^a-z0-9-_]/gi, "_")}.html`);
fs.writeFileSync(file, html, "utf8");
Two invoices with numbers like INV/001 and INV:001 both sanitize to the same filename. Saving the second one silently overwrote the first, no warning, no prompt.
The fixes themselves were small - a sanitizeItems() pass that coerces and filters before anything touches the renderer, an escapeMarkdown() sibling to the HTML escaper, a collision check before writeFileSync, a try/catch around the save with a failure toast. Maybe 40 lines total. The gap wasn't skill, it was that every one of my own tests handed the function well-formed data. I was testing that the code did what I designed it to do, not what happens when someone hands it something I didn't design for.
The honest takeaway for me: unit tests you write yourself tend to probe the shape of input you were already imagining. The useful adversarial cases - malformed JSON, type coercion through JSON.parse, markdown/HTML injection via a "just text" field - are exactly the ones that don't occur to you while you're writing the happy-path version, because if they occurred to you, you'd have handled them already. That's the actual value of someone (or something) else hitting your code with the specific intent of breaking it before real users do it by accident.
The PR (raycast/extensions#32665) also caught a quieter non-security issue the second time around: after I fixed the four bugs above, the new test I added for the null-items case had its @ts-expect-error comment in the wrong spot, which silently made the type checker pass when it should have failed. Small reminder that regression tests for a bug need their own review too - a test that compiles for the wrong reason is worse than no test, because it looks green.
Source (render logic + tests): github.com/kleogeos/freelance-invoice-preview
Top comments (4)
The markdown pixel one is the same trick used against AI chat apps. Get the model to print an image link with data in the URL and the client fetches it when it renders, no click needed. So anything that renders model output as markdown has the same bug, not only user-pasted JSON.
Were the other 5 comments real issues or noise? For me that ratio decides if a team keeps reading the bot after the first month.
Good catch on the AI-chat-apps parallel - same bug shape: anything that renders model (or user) output as markdown without escaping markdown's own special characters inherits the SSRF-via-image-pixel issue, not just HTML renderers.
To answer directly: 11/11 ended up real, 0 noise. 4 were P1/security (the ones in the post). 1 more was P1 but not security - a type-check failure from an @ts-expect-error that didn't line up with where TS actually flagged the error. The rest were P2: an unused dependency, a missing package-lock.json (npm ci needs one), tests that existed but weren't wired into any runnable command, vague error messages on save failure, a missing Finance category in package.json, and required Store screenshots I still haven't supplied - Raycast extensions only render inside the desktop app, and I'm on a Linux box with no GUI, so I can't honestly screenshot my own extension without the real app. That one's still open.
So for this one pass the ratio was 11/11 real. If that holds over a few more PRs I'd stop double-checking the P1/security flags myself and just spend review time on the P2s, which were more "this gets your PR rejected on a technicality" than "this is exploitable."
11 for 11 is better than I expected, thanks for checking.
Two things though. The missing package-lock.json is the P2 I'd move up: without it every install resolves fresh versions, so a bad publish of one of your dependencies lands in your build without a diff. And 11/11 tells you the bot is right when it flags something, not what it missed, so I'd keep a human pass on the security side anyway.
Fair on both counts.
package-lock.json: already fixed, and not just agreed-in-principle - I went back and checked the actual PR diff just now (raycast/extensions#32665) and the file is in there (extensions/freelance-invoice-preview/package-lock.json, added). So that one's closed, not just acknowledged.
On the second point - agreed, and I'd go further: an 11/11 catch rate this one time tells me the bot didn't produce false positives on this pass, not that it has full coverage. The things it's good at catching are shaped like the things it was trained to look for (injection, null-safety, clobbering writes). A category it has no training signal for would just silently not get flagged, and 11/11 would look identical either way. So yeah, human review on the security-relevant paths stays regardless of how clean the bot's track record looks - the ratio tells you about precision, not recall.