Skip to content

Commit 4cccc0e

Browse files
committed
fix(images): the CSP blocked every thumbnail, and an image with no words 400'd
Three bugs from a real run, all mine. 1. THE THUMBNAIL WAS BROKEN BECAUSE NOTHING COULD RENDER IT. The webview CSP is `default-src 'none'` with no img-src, so every image — including a data: URI built from the attachment's own bytes — was blocked. I added images without touching the policy. Now `img-src data:`, and deliberately NOT https:, so the panel still cannot fetch a remote image. 2. AN IMAGE WITH NO WORDS RETURNED A 400. Anthropic rejects an empty text block outright ("text content blocks must be non-empty"), and sending an image with nothing typed produced exactly that. Both paths now include the text block only when there IS text, and fall back to the images alone. This is the case I deliberately made valid in I5 — "look at this" is implied by attaching — so it was the one shape guaranteed to be hit. 3. DRAGGING FROM FINDER DID NOTHING. VS Code's workbench intercepts OS file drops before a webview iframe sees them, so dataTransfer.files is empty — while the PATH is still there as a uri-list. The drop handler now falls back to reading the uri-list and handing the paths to the host, which reads them off disk and sends the bytes back for the same normalizer a paste uses. Size is checked host-side BEFORE base64 crosses the message bus, so a 200MB file is refused rather than serialized first. Also I6: a visible Attach-image button next to +, using showOpenDialog. The picker is the route that works regardless of what the webview is allowed to receive; the drop fallback is best-effort on top of it. Two guards needed updating rather than adding — the empty-text ternary changed the shape they matched. That is the guards doing their job; I updated them and added one that pins the 400 itself. 14 tests in imageAttach, 37 suites green.
1 parent 9e297f4 commit 4cccc0e

3 files changed

Lines changed: 125 additions & 8 deletions

File tree

‎extensions/levelcode-ai/extension.js‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1617,7 +1617,7 @@ async function agentFlow(text, imageBlocks) {
16171617
// Agent mode is the DEFAULT, so this is the path most pasted screenshots take. Blocks only when
16181618
// there IS an image — a text-only goal stays a plain string so cached prefixes keep their bytes.
16191619
const goalMsg = (imageBlocks && imageBlocks.length)
1620-
? { role: 'user', content: [...imageBlocks, { type: 'text', text }] }
1620+
? { role: 'user', content: text ? [...imageBlocks, { type: 'text', text }] : imageBlocks }
16211621
: { role: 'user', content: text };
16221622
currentCheckpoint = { turnId: ++checkpointSeq, label: (text || '').slice(0, 60), ts: Date.now(), goalMsg: goalMsg, files: new Map() };
16231623
checkpoints.push(currentCheckpoint);
@@ -1745,6 +1745,48 @@ function imageRoot() {
17451745
catch (e) { dbg('image.root.failed', { msg: String((e && e.message) || e) }); return null; }
17461746
}
17471747

1748+
/**
1749+
* Read image files from disk and hand their bytes to the webview to normalize.
1750+
*
1751+
* Two callers, one path. The picker (reliable everywhere) and a Finder drop that arrives as a
1752+
* uri-list rather than as File objects — VS Code's workbench intercepts OS file drops before a
1753+
* webview iframe sees them, so `dataTransfer.files` is often empty while the PATH is still there.
1754+
* Reading host-side covers both, and normalization still happens in the webview because that is
1755+
* the only place with a canvas.
1756+
*/
1757+
async function attachImagePaths(paths) {
1758+
const files = [];
1759+
for (const fsPath of (Array.isArray(paths) ? paths : []).slice(0, 8)) {
1760+
try {
1761+
const ext = String(path.extname(fsPath) || '').toLowerCase();
1762+
const mt = { '.png': 'image/png', '.jpg': 'image/jpeg', '.jpeg': 'image/jpeg',
1763+
'.gif': 'image/gif', '.webp': 'image/webp' }[ext];
1764+
if (!mt) { vscode.window.showWarningMessage(path.basename(fsPath) + ' is not an image LevelCode can read.'); continue; }
1765+
const buf = await fs.promises.readFile(fsPath);
1766+
// Guard before the bytes cross into the webview: a 200MB file would otherwise be
1767+
// base64-ed onto the message bus before anything got a chance to refuse it.
1768+
if (buf.length > 25 * 1024 * 1024) {
1769+
vscode.window.showWarningMessage(path.basename(fsPath) + ' is too large to attach.');
1770+
continue;
1771+
}
1772+
files.push({ base64: buf.toString('base64'), media_type: mt, name: path.basename(fsPath) });
1773+
} catch (e) {
1774+
dbg('image.read.failed', { msg: String((e && e.message) || e) });
1775+
vscode.window.showWarningMessage('Could not read ' + path.basename(fsPath));
1776+
}
1777+
}
1778+
if (files.length) { post({ type: 'attachImages', files }); }
1779+
}
1780+
1781+
/** Pick images from disk — the path that works regardless of what the webview can receive. */
1782+
async function pickImages() {
1783+
const picked = await vscode.window.showOpenDialog({
1784+
canSelectMany: true, openLabel: 'Attach',
1785+
filters: { Images: ['png', 'jpg', 'jpeg', 'gif', 'webp'] }
1786+
});
1787+
if (picked && picked.length) { await attachImagePaths(picked.map((u) => u.fsPath)); }
1788+
}
1789+
17481790
/**
17491791
* Store what the webview normalized, and return the blocks that will ride the conversation.
17501792
*
@@ -1821,8 +1863,10 @@ async function handleSend(text, images) {
18211863
// Blocks only when there is an image; a text-only turn stays a plain string so every cached
18221864
// prefix keeps the bytes it already had. Images lead — the model reads them best before the
18231865
// text that asks about them.
1866+
// An empty text block is a 400 from Anthropic ("text content blocks must be non-empty"), and an
1867+
// image sent with no words produces exactly that. Include the text block only when there is text.
18241868
conversation.push(imageBlocks.length
1825-
? { role: 'user', content: [...imageBlocks, { type: 'text', text: userContent }] }
1869+
? { role: 'user', content: userContent ? [...imageBlocks, { type: 'text', text: userContent }] : imageBlocks }
18261870
: { role: 'user', content: userContent });
18271871
post({ type: 'userMessage', text });
18281872
if (auto.names.length) { post({ type: 'autoContext', names: auto.names }); }
@@ -2359,6 +2403,8 @@ class ChatViewProvider {
23592403
// One surface for "that could not be attached" — VS Code's own, not a second one
23602404
// invented inside the transcript.
23612405
case 'notice': if (msg.text) { vscode.window.showWarningMessage(String(msg.text)); } break;
2406+
case 'pickImages': await pickImages(); break;
2407+
case 'attachImagePaths': await attachImagePaths(msg.paths); break;
23622408
case 'stop': dbg('stop.clicked', { running: commandStops.size }); for (const [, stop] of commandStops) { try { stop(); } catch (e) { /* gone */ } } if (abort) { abort.abort(); } clearApprovals(); clearQuestions(); break;
23632409
case 'stopCommand': { dbg('stopCommand', { id: msg.id }); const s = commandStops.get(msg.id); if (s) { try { s(); } catch (e) { /* gone */ } } break; }
23642410
case 'approvalResponse': {
@@ -2557,6 +2603,9 @@ function webviewCsp() {
25572603
const nonce = String(Math.random()).slice(2) + String(Date.now());
25582604
return { nonce, csp: [
25592605
"default-src 'none'",
2606+
// data: only — attached screenshots are rendered from their own bytes. Deliberately NOT
2607+
// https:, so the panel still cannot reach out to the network for an image.
2608+
"img-src data:",
25602609
"style-src 'unsafe-inline'",
25612610
"script-src 'nonce-" + nonce + "'"
25622611
].join('; ') };

‎extensions/levelcode-ai/media/chat.html‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@
6565
calc above already insets by --shell-x. Set margin-block in these rules, never margin.
6666
test/webviewCss.test.js pins this for the whole list. */
6767

68+
#attachImg .ci { width: 14px; height: 14px; display: block; }
69+
#attachImg { display: inline-flex; align-items: center; justify-content: center; }
70+
6871
/* ---- attached images ---- */
6972
/* The chip carries its own thumbnail: an attachment you cannot see is one you cannot check before
7073
sending, and a screenshot is the one attachment where the wrong one looks exactly like the right
@@ -1333,6 +1336,9 @@
13331336
<div id="toolbar">
13341337
<div class="left">
13351338
<button class="tbtn" id="attach" title="Add selected code as context (⌥⌘A)">+</button>
1339+
<button class="tbtn" id="attachImg" title="Attach an image — or paste a screenshot with ⌘V" aria-label="Attach an image">
1340+
<svg class="ci" viewBox="0 0 16 16" fill="currentColor" aria-hidden="true"><path d="M2.5 3h11l.5.5v9l-.5.5h-11l-.5-.5v-9l.5-.5zm.5 1v6.3l2.6-2.3 2.5 2.2 2.9-3.1L13 9.4V4H3zm0 8h10v-1.2l-2.5-2.6-2.9 3.1-2.5-2.2L3 11.4V12zM6 5.5a1.2 1.2 0 1 1-2.4 0 1.2 1.2 0 0 1 2.4 0z"/></svg>
1341+
</button>
13361342
<span class="sep"></span>
13371343
<span class="modewrap">
13381344
<span class="pill agent" id="mode" title="Choose Agent or Chat mode" role="button" tabindex="0" aria-haspopup="listbox" aria-expanded="false">
@@ -3178,6 +3184,14 @@
31783184
return { w: tw, h: th, media_type: 'image/webp', base64: await blobToBase64(blob), bytes: blob.size };
31793185
}
31803186

3187+
/** Host-read bytes -> a File, so a picked/dropped path goes through the same normalizer as a paste. */
3188+
function b64ToFile(b64, type, name){
3189+
const bin = atob(b64);
3190+
const u8 = new Uint8Array(bin.length);
3191+
for (let i = 0; i < bin.length; i++) { u8[i] = bin.charCodeAt(i); }
3192+
return new File([u8], name || 'image', { type: type });
3193+
}
3194+
31813195
async function attachImageFiles(files){
31823196
const list = Array.from(files || []).filter(function(f){ return f && /^image\//.test(f.type); });
31833197
if (!list.length) { return false; }
@@ -3225,15 +3239,31 @@
32253239
document.addEventListener('dragleave', function(ev){ if (!ev.relatedTarget) { document.body.classList.remove('dropping'); } });
32263240
document.addEventListener('drop', function(ev){
32273241
document.body.classList.remove('dropping');
3228-
if (!ev.dataTransfer || !ev.dataTransfer.files || !ev.dataTransfer.files.length) { return; }
3229-
const imgs = Array.from(ev.dataTransfer.files).filter(function(f){ return /^image\//.test(f.type); });
3230-
if (!imgs.length) { return; }
3242+
const dt = ev.dataTransfer; if (!dt) { return; }
3243+
const imgs = Array.from(dt.files || []).filter(function(f){ return /^image\//.test(f.type); });
3244+
if (imgs.length) { ev.preventDefault(); attachImageFiles(imgs); return; }
3245+
// VS Code's workbench intercepts OS file drops before a webview iframe sees them, so
3246+
// dataTransfer.files is usually EMPTY for a drag out of Finder — while the path is still
3247+
// there as a uri-list. Hand the paths to the host, which can read them off disk.
3248+
let uris = '';
3249+
try { uris = dt.getData('text/uri-list') || dt.getData('text/plain') || ''; } catch (e) {}
3250+
const paths = uris.split(/[\r\n]+/)
3251+
.map(function(u){ return u.trim(); })
3252+
.filter(function(u){ return u && !/^#/.test(u); })
3253+
.map(function(u){ try { return /^file:/i.test(u) ? decodeURIComponent(u.replace(/^file:\/\//i, '')) : u; } catch (e) { return u; } })
3254+
.filter(function(u){ return /\.(png|jpe?g|gif|webp)$/i.test(u); });
3255+
if (!paths.length) { return; }
32313256
ev.preventDefault();
3232-
attachImageFiles(imgs);
3257+
if (!canSeeImages) { note((canSeeImagesModel || 'This model') + ' cannot read images.'); return; }
3258+
vscode.postMessage({ type: 'attachImagePaths', paths: paths });
32333259
});
32343260

32353261
sendBtn.onclick = doSend;
32363262
document.getElementById('attach').onclick = () => vscode.postMessage({ type: 'addContext' });
3263+
document.getElementById('attachImg').onclick = function(){
3264+
if (!canSeeImages) { note((canSeeImagesModel || 'This model') + ' cannot read images. Switch to a vision model and try again.'); return; }
3265+
vscode.postMessage({ type: 'pickImages' });
3266+
};
32373267
document.getElementById('model').onclick = () => vscode.postMessage({ type: 'pickModel' });
32383268
// Approvals dropdown (mirrors the Agent/Chat menu). The host owns the flag (persists it + gates the
32393269
// danger set); selecting an option posts setAutopilot and we reflect whatever it echoes back via the
@@ -3797,6 +3827,9 @@
37973827
if (typeof m.groupActivity === 'boolean'){ if (!m.groupActivity){ closeGroup(); } groupsOn = m.groupActivity; }
37983828
renderRouting();
37993829
}
3830+
else if (m.type === 'attachImages'){
3831+
attachImageFiles((m.files || []).map(function(f){ return b64ToFile(f.base64, f.media_type, f.name); }));
3832+
}
38003833
else if (m.type === 'assistantStart'){ shown = ''; pending = ''; doneSignaled = false; flushAll = false; current = makeStream(add('assistant', '')); setStreaming(true); }
38013834
else if (m.type === 'assistantDelta'){ pending += m.text; ensurePump(); }
38023835
else if (m.type === 'assistantDone'){ doneSignaled = true; ensurePump(); }

‎extensions/levelcode-ai/test/imageAttach.test.js‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ test('HOST: images become refs in the conversation, and bytes only at request ti
122122

123123
test('CONVERSATION: blocks only when there is an image', () => {
124124
const body = fnBody(ext, 'handleSend');
125-
assert.match(body, /imageBlocks\.length\s*\n?\s*\?\s*\{ role: 'user', content: \[\.\.\.imageBlocks/,
125+
assert.match(body, /\[\.\.\.imageBlocks, \{ type: 'text', text: userContent \}\]/,
126126
'images lead the block array');
127127
assert.match(body, /:\s*\{ role: 'user', content: userContent \}/,
128128
'a text-only turn must stay a plain string, or every cached prefix churns');
@@ -137,7 +137,7 @@ test('AGENT MODE: the default path carries images too', () => {
137137
assert.match(ext, /async function agentFlow\(text, imageBlocks\)/, 'agentFlow must accept them');
138138
const body = fnBody(ext, 'agentFlow');
139139
assert.match(body, /imageBlocks && imageBlocks\.length/, 'and use them when present');
140-
assert.match(body, /content: \[\.\.\.imageBlocks, \{ type: 'text', text \}\]/, 'images lead the goal');
140+
assert.match(body, /\[\.\.\.imageBlocks, \{ type: 'text', text \}\]/, 'images lead the goal');
141141
assert.match(body, /:\s*\{ role: 'user', content: text \}/,
142142
'a text-only goal must stay a plain string, or every cached agent prefix churns');
143143
});
@@ -151,4 +151,39 @@ test('NO WORKSPACE: an image still has somewhere to live', () => {
151151
assert.ok(!/Images need a session/.test(ext), 'the old session-required refusal must be gone');
152152
});
153153

154+
test('400: an image with no words must not emit an empty text block', () => {
155+
// Anthropic rejects it outright — "text content blocks must be non-empty" — and an image sent
156+
// with no words produced exactly that. Both paths must omit the block rather than send "".
157+
const send = fnBody(ext, 'handleSend');
158+
assert.match(send, /userContent \? \[\.\.\.imageBlocks/, 'handleSend must gate the text block on there being text');
159+
assert.match(send, /:\s*imageBlocks\b/, 'handleSend must fall back to the images alone');
160+
161+
const agent = fnBody(ext, 'agentFlow');
162+
assert.match(agent, /text \? \[\.\.\.imageBlocks/, 'agentFlow must gate the text block on there being text');
163+
assert.match(agent, /:\s*imageBlocks\b/, 'agentFlow must fall back to the images alone');
164+
});
165+
166+
test('CSP: the webview is allowed to render a data: image, and nothing else', () => {
167+
// default-src 'none' blocks every image, which is why the first thumbnail rendered broken.
168+
const csp = fnBody(ext, 'webviewCsp');
169+
assert.match(csp, /"img-src data:"/, 'attached images cannot render without this');
170+
assert.ok(!/img-src[^"]*https:/.test(csp), 'the panel must not be able to fetch a remote image');
171+
});
172+
173+
test('PICKER + DROP: a Finder file reaches the same normalizer as a paste', () => {
174+
// VS Code's workbench intercepts OS file drops before a webview iframe sees them, so
175+
// dataTransfer.files is usually empty for a Finder drag while the PATH is still there.
176+
assert.match(html, /getData\('text\/uri-list'\)/, 'no uri-list fallback for the VS Code drop case');
177+
assert.match(html, /type: 'attachImagePaths'/, 'paths must be handed to the host to read');
178+
assert.match(ext, /case 'attachImagePaths'/, 'the host must accept them');
179+
assert.match(ext, /case 'pickImages'/, 'and offer a picker that works regardless');
180+
assert.match(ext, /showOpenDialog\(/, 'the picker must be a real file dialog');
181+
182+
const reader = fnBody(ext, 'attachImagePaths');
183+
assert.match(reader, /25 \* 1024 \* 1024/, 'guard the size BEFORE base64 crosses the message bus');
184+
assert.match(reader, /is not an image LevelCode can read/, 'a non-image must say so, not fail silently');
185+
assert.match(html, /function b64ToFile/, 'host bytes must become a File so both routes share normalizeImage');
186+
assert.match(html, /id="attachImg"/, 'there must be a visible way in besides paste');
187+
});
188+
154189
console.log('\nimageAttach: ' + n + ' tests passed.');

0 commit comments

Comments
 (0)