fix(markdown): correct image gallery rendering (#2894)
This commit is contained in:
@@ -0,0 +1,30 @@
|
||||
# Markdown Image Grants
|
||||
|
||||
## Purpose
|
||||
|
||||
This module lets the Markdown image gallery display images that an assistant
|
||||
explicitly referenced from OpenCode's temporary directory when the UI is on a
|
||||
different machine.
|
||||
|
||||
## Contract
|
||||
|
||||
- Chat Markdown rendering is independent: assistant image syntax renders as an
|
||||
icon and filename, while the gallery only reads finalized Markdown to collect
|
||||
image candidates.
|
||||
- `POST /api/openchamber/sessions/:sessionId/markdown-image-grants` prepares up to 12
|
||||
local images in one message-level request. The server fetches the assistant
|
||||
message once and verifies every exact image source before reading files.
|
||||
- Relative and workspace-contained absolute paths resolve against the active
|
||||
directory. Other absolute paths are accepted only inside
|
||||
`os.tmpdir()/opencode` after `realpath` resolution.
|
||||
- PNG, JPEG, GIF, and WebP files are signature-checked and limited to 10 MiB.
|
||||
- Prepare requests inspect only file metadata and signatures. Workspace images
|
||||
reuse the existing authenticated `/api/fs/raw` asset route directly. Images
|
||||
under `os.tmpdir()/opencode` receive the existing path-bound `raw`
|
||||
`outsideFileGrant`; this module does not add another asset lifetime, copy, or
|
||||
storage layer. Missing files return per-source results so the gallery can
|
||||
remove only those items.
|
||||
|
||||
The routes are OpenChamber-owned and must be registered before the generic
|
||||
OpenCode proxy. Web, Electron, hosted mobile, and Capacitor use the shared
|
||||
server implementation. VS Code returns an explicit unsupported response.
|
||||
@@ -0,0 +1,215 @@
|
||||
import express from 'express';
|
||||
import { constants as fsConstants } from 'node:fs';
|
||||
import { mintOutsideFileGrant } from '../fs/routes.js';
|
||||
|
||||
const MAX_IMAGE_BYTES = 10 * 1024 * 1024;
|
||||
const MAX_IMAGE_SOURCES = 12;
|
||||
|
||||
const asString = (value) => typeof value === 'string' ? value.trim() : '';
|
||||
|
||||
const isWithin = (target, root, path) => {
|
||||
const relative = path.relative(root, target);
|
||||
return relative === '' || (!relative.startsWith('..') && !path.isAbsolute(relative));
|
||||
};
|
||||
|
||||
const parseFileSource = (source) => {
|
||||
if (/^file:\/\//i.test(source)) {
|
||||
try {
|
||||
const url = new URL(source);
|
||||
if (url.protocol !== 'file:' || (url.host && url.host !== 'localhost')) return '';
|
||||
const pathname = decodeURIComponent(url.pathname);
|
||||
return /^\/[A-Za-z]:\//.test(pathname) ? pathname.slice(1) : pathname;
|
||||
} catch {
|
||||
return '';
|
||||
}
|
||||
}
|
||||
const pathname = source.split(/[?#]/, 1)[0] || '';
|
||||
try {
|
||||
return decodeURIComponent(pathname);
|
||||
} catch {
|
||||
return pathname;
|
||||
}
|
||||
};
|
||||
|
||||
const hasImageSignature = (bytes) => {
|
||||
if (bytes.length >= 8
|
||||
&& bytes[0] === 0x89 && bytes.subarray(1, 4).toString('ascii') === 'PNG'
|
||||
&& bytes[4] === 0x0d && bytes[5] === 0x0a && bytes[6] === 0x1a && bytes[7] === 0x0a) return true;
|
||||
if (bytes.length >= 3 && bytes[0] === 0xff && bytes[1] === 0xd8 && bytes[2] === 0xff) return true;
|
||||
const header = bytes.subarray(0, 12).toString('ascii');
|
||||
return header.startsWith('GIF87a')
|
||||
|| header.startsWith('GIF89a')
|
||||
|| (header.startsWith('RIFF') && header.slice(8, 12) === 'WEBP');
|
||||
};
|
||||
|
||||
const markdownImageSources = (message) => {
|
||||
const sources = new Set();
|
||||
for (const part of Array.isArray(message?.parts) ? message.parts : []) {
|
||||
if (part?.type !== 'text' || typeof part.text !== 'string') continue;
|
||||
// Code examples must never authorize file access, even when they contain image syntax.
|
||||
let fenced = false;
|
||||
for (const line of part.text.split('\n')) {
|
||||
if (/^\s{0,3}(?:```|~~~)/.test(line)) {
|
||||
fenced = !fenced;
|
||||
continue;
|
||||
}
|
||||
if (fenced) continue;
|
||||
const visible = line.replace(/`+[^`]*`+/g, '');
|
||||
const pattern = /(?<!\\)!\[[^\]]*]\(\s*(?:<([^>\n]+)>|([^\s)\n]+))/g;
|
||||
let match = pattern.exec(visible);
|
||||
while (match) {
|
||||
sources.add(match[1] || match[2]);
|
||||
match = pattern.exec(visible);
|
||||
}
|
||||
}
|
||||
}
|
||||
return sources;
|
||||
};
|
||||
|
||||
const fetchMessage = async ({ sessionId, messageId, directory, buildOpenCodeUrl, getOpenCodeAuthHeaders }) => {
|
||||
const url = new URL(buildOpenCodeUrl(
|
||||
`/session/${encodeURIComponent(sessionId)}/message/${encodeURIComponent(messageId)}`,
|
||||
'',
|
||||
));
|
||||
url.searchParams.set('directory', directory);
|
||||
const response = await fetch(url, {
|
||||
headers: {
|
||||
accept: 'application/json',
|
||||
'x-opencode-directory': directory,
|
||||
...getOpenCodeAuthHeaders(),
|
||||
},
|
||||
signal: AbortSignal.timeout(10_000),
|
||||
});
|
||||
if (response.status === 404) return null;
|
||||
if (!response.ok) throw new Error(`OpenCode returned ${response.status}`);
|
||||
const message = await response.json().catch(() => null);
|
||||
return message?.info && Array.isArray(message.parts) ? message : null;
|
||||
};
|
||||
|
||||
const inspectImage = async ({ source, directory, approvedTempRoot, fsPromises, path }) => {
|
||||
const parsed = parseFileSource(source);
|
||||
if (!parsed) return { status: 'error' };
|
||||
const sourcePath = path.isAbsolute(parsed) ? parsed : path.resolve(directory, parsed);
|
||||
const workspaceRoot = path.resolve(directory);
|
||||
const outsideWorkspace = !isWithin(path.resolve(sourcePath), workspaceRoot, path);
|
||||
const root = outsideWorkspace ? approvedTempRoot : workspaceRoot;
|
||||
|
||||
try {
|
||||
// Resolve symlinks before comparing roots; lexical prefixes are not an authorization boundary.
|
||||
const [canonicalRoot, canonicalPath] = await Promise.all([
|
||||
fsPromises.realpath(root),
|
||||
fsPromises.realpath(sourcePath),
|
||||
]);
|
||||
if (!isWithin(canonicalPath, canonicalRoot, path)) return { status: 'error' };
|
||||
const handle = await fsPromises.open(canonicalPath, fsConstants.O_RDONLY | fsConstants.O_NOFOLLOW);
|
||||
try {
|
||||
const stats = await handle.stat();
|
||||
if (!stats.isFile() || stats.size > MAX_IMAGE_BYTES) return { status: 'error' };
|
||||
const header = Buffer.alloc(12);
|
||||
const { bytesRead } = await handle.read(header, 0, header.length, 0);
|
||||
if (!hasImageSignature(header.subarray(0, bytesRead))) return { status: 'error' };
|
||||
return {
|
||||
status: 'ready',
|
||||
path: outsideWorkspace ? canonicalPath : path.resolve(sourcePath),
|
||||
outsideWorkspace,
|
||||
};
|
||||
} finally {
|
||||
await handle.close();
|
||||
}
|
||||
} catch (error) {
|
||||
if (error?.code === 'ENOENT') return { status: 'missing' };
|
||||
if (error?.code === 'EACCES' || error?.code === 'EPERM' || error?.code === 'ELOOP') {
|
||||
return { status: 'error' };
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
};
|
||||
|
||||
export const registerMarkdownImageGrantRoutes = (app, dependencies) => {
|
||||
const {
|
||||
fsPromises,
|
||||
path,
|
||||
os,
|
||||
crypto,
|
||||
validateDirectoryPath,
|
||||
buildOpenCodeUrl,
|
||||
getOpenCodeAuthHeaders,
|
||||
approvedTempRoot = path.join(os.tmpdir(), 'opencode'),
|
||||
} = dependencies;
|
||||
|
||||
app.post(
|
||||
'/api/openchamber/sessions/:sessionId/markdown-image-grants',
|
||||
express.json({ limit: '32kb' }),
|
||||
async (req, res) => {
|
||||
const sessionId = asString(req.params.sessionId);
|
||||
const messageId = asString(req.body?.messageId);
|
||||
const sources = Array.isArray(req.body?.sources)
|
||||
? [...new Set(req.body.sources.map(asString).filter(Boolean))]
|
||||
: [];
|
||||
if (!sessionId || !messageId || sources.length === 0 || sources.length > MAX_IMAGE_SOURCES) {
|
||||
return res.status(400).json({ error: 'sessionId, messageId, and 1-12 sources are required' });
|
||||
}
|
||||
const validatedDirectory = await validateDirectoryPath(asString(req.body?.directory));
|
||||
if (!validatedDirectory.ok) {
|
||||
return res.status(400).json({ error: validatedDirectory.error || 'Invalid directory' });
|
||||
}
|
||||
|
||||
try {
|
||||
const message = await fetchMessage({
|
||||
sessionId,
|
||||
messageId,
|
||||
directory: validatedDirectory.directory,
|
||||
buildOpenCodeUrl,
|
||||
getOpenCodeAuthHeaders,
|
||||
});
|
||||
if (!message || message.info?.id !== messageId || message.info?.role !== 'assistant') {
|
||||
return res.status(404).json({ error: 'Assistant message not found' });
|
||||
}
|
||||
// Assistant text is authoritative: a remote client cannot mint grants for unreferenced paths.
|
||||
const referenced = markdownImageSources(message);
|
||||
const results = [];
|
||||
for (const source of sources) {
|
||||
if (!referenced.has(source)) {
|
||||
results.push({ source, status: 'error' });
|
||||
continue;
|
||||
}
|
||||
try {
|
||||
const inspected = await inspectImage({
|
||||
source,
|
||||
directory: validatedDirectory.directory,
|
||||
approvedTempRoot,
|
||||
fsPromises,
|
||||
path,
|
||||
});
|
||||
if (inspected.status !== 'ready') {
|
||||
results.push({ source, status: inspected.status });
|
||||
continue;
|
||||
}
|
||||
// Reuse the existing path-bound raw-file grant instead of creating another asset lifecycle.
|
||||
const grant = inspected.outsideWorkspace
|
||||
? await mintOutsideFileGrant(inspected.path, {
|
||||
scopes: ['raw'],
|
||||
fsPromises,
|
||||
path,
|
||||
crypto,
|
||||
})
|
||||
: null;
|
||||
results.push({
|
||||
source,
|
||||
status: 'ready',
|
||||
path: inspected.path,
|
||||
outsideFileGrant: grant?.outsideFileGrant,
|
||||
expiresAt: grant?.expiresAt,
|
||||
});
|
||||
} catch {
|
||||
results.push({ source, status: 'error' });
|
||||
}
|
||||
}
|
||||
return res.json({ results });
|
||||
} catch (error) {
|
||||
console.warn('[MarkdownImageGrants] failed to prepare images:', error?.message || error);
|
||||
return res.status(503).json({ error: 'Failed to prepare session images' });
|
||||
}
|
||||
},
|
||||
);
|
||||
};
|
||||
@@ -0,0 +1,175 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||
import express from 'express';
|
||||
import request from 'supertest';
|
||||
import crypto from 'node:crypto';
|
||||
import fs from 'node:fs/promises';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
import { registerMarkdownImageGrantRoutes } from './routes.js';
|
||||
|
||||
const PNG = Buffer.from(
|
||||
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8DwHwAFBQIAX8jx0gAAAABJRU5ErkJggg==',
|
||||
'base64',
|
||||
);
|
||||
const roots = [];
|
||||
|
||||
afterEach(async () => {
|
||||
vi.unstubAllGlobals();
|
||||
await Promise.all(roots.splice(0).map((root) => fs.rm(root, { recursive: true, force: true })));
|
||||
});
|
||||
|
||||
const createFixture = async ({ sources, markdown } = {}) => {
|
||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), 'openchamber-session-assets-'));
|
||||
roots.push(root);
|
||||
const approvedTempRoot = path.join(root, 'opencode');
|
||||
const directory = path.join(root, 'workspace');
|
||||
await Promise.all([
|
||||
fs.mkdir(approvedTempRoot, { recursive: true }),
|
||||
fs.mkdir(directory, { recursive: true }),
|
||||
]);
|
||||
const defaultPath = path.join(approvedTempRoot, 'image.png');
|
||||
await fs.writeFile(defaultPath, PNG);
|
||||
const requestedSources = sources ?? [new URL(`file://${defaultPath}`).toString()];
|
||||
const text = markdown ?? requestedSources.map((source) => ``).join('\n');
|
||||
const fetchMock = vi.fn(async () => new Response(JSON.stringify({
|
||||
info: { id: 'msg_1', role: 'assistant' },
|
||||
parts: [{ type: 'text', text }],
|
||||
}), { status: 200, headers: { 'content-type': 'application/json' } }));
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
|
||||
let fullReadCount = 0;
|
||||
const app = express();
|
||||
registerMarkdownImageGrantRoutes(app, {
|
||||
fsPromises: {
|
||||
...fs,
|
||||
readFile: async (...args) => {
|
||||
fullReadCount += 1;
|
||||
return fs.readFile(...args);
|
||||
},
|
||||
},
|
||||
path,
|
||||
os,
|
||||
crypto,
|
||||
approvedTempRoot,
|
||||
validateDirectoryPath: async (candidate) => candidate === directory
|
||||
? { ok: true, directory }
|
||||
: { ok: false, error: 'Invalid directory' },
|
||||
buildOpenCodeUrl: (route) => `http://opencode.test${route}`,
|
||||
getOpenCodeAuthHeaders: () => ({ authorization: 'Basic test' }),
|
||||
});
|
||||
return {
|
||||
app,
|
||||
approvedTempRoot,
|
||||
directory,
|
||||
fetchMock,
|
||||
fullReadCount: () => fullReadCount,
|
||||
root,
|
||||
sources: requestedSources,
|
||||
};
|
||||
};
|
||||
|
||||
const prepare = (app, directory, sources) => request(app)
|
||||
.post('/api/openchamber/sessions/ses_1/markdown-image-grants')
|
||||
.send({ directory, messageId: 'msg_1', sources })
|
||||
.expect(200);
|
||||
|
||||
describe('session image assets', () => {
|
||||
it('prepares workspace and OpenCode temporary images with one message fetch', async () => {
|
||||
const fixture = await createFixture({ sources: ['workspace.png'] });
|
||||
await fs.writeFile(path.join(fixture.directory, 'workspace.png'), PNG);
|
||||
const temporaryPath = path.join(fixture.approvedTempRoot, 'temporary.png');
|
||||
await fs.writeFile(temporaryPath, PNG);
|
||||
const temporarySource = new URL(`file://${temporaryPath}`).toString();
|
||||
fixture.fetchMock.mockResolvedValueOnce(new Response(JSON.stringify({
|
||||
info: { id: 'msg_1', role: 'assistant' },
|
||||
parts: [{ type: 'text', text: `\n` }],
|
||||
}), { status: 200, headers: { 'content-type': 'application/json' } }));
|
||||
|
||||
const response = await prepare(fixture.app, fixture.directory, ['workspace.png', temporarySource]);
|
||||
|
||||
expect(fixture.fetchMock).toHaveBeenCalledTimes(1);
|
||||
expect(fixture.fullReadCount()).toBe(0);
|
||||
expect(response.body.results).toHaveLength(2);
|
||||
const canonicalTemporaryPath = await fs.realpath(temporaryPath);
|
||||
expect(response.body.results[0]).toEqual({
|
||||
source: 'workspace.png',
|
||||
status: 'ready',
|
||||
path: path.join(fixture.directory, 'workspace.png'),
|
||||
});
|
||||
expect(response.body.results[1]).toEqual(expect.objectContaining({
|
||||
source: temporarySource,
|
||||
status: 'ready',
|
||||
path: canonicalTemporaryPath,
|
||||
outsideFileGrant: expect.any(String),
|
||||
expiresAt: expect.any(Number),
|
||||
}));
|
||||
});
|
||||
|
||||
it('returns partial results without letting one missing image block valid images', async () => {
|
||||
const fixture = await createFixture({ sources: ['present.png', 'deleted.png'] });
|
||||
await fs.writeFile(path.join(fixture.directory, 'present.png'), PNG);
|
||||
|
||||
const response = await prepare(fixture.app, fixture.directory, fixture.sources);
|
||||
|
||||
expect(response.body.results).toEqual([
|
||||
expect.objectContaining({ source: 'present.png', status: 'ready' }),
|
||||
{ source: 'deleted.png', status: 'missing' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('resolves encoded workspace paths without treating query or fragment text as a filename', async () => {
|
||||
const source = 'screen%20shot.png?version=1#preview';
|
||||
const fixture = await createFixture({ sources: [source] });
|
||||
await fs.writeFile(path.join(fixture.directory, 'screen shot.png'), PNG);
|
||||
|
||||
const response = await prepare(fixture.app, fixture.directory, fixture.sources);
|
||||
|
||||
expect(response.body.results).toEqual([
|
||||
expect.objectContaining({ source, status: 'ready' }),
|
||||
]);
|
||||
});
|
||||
|
||||
it('rejects a source that the message does not reference', async () => {
|
||||
const fixture = await createFixture({ markdown: 'No image here.' });
|
||||
const response = await prepare(fixture.app, fixture.directory, fixture.sources);
|
||||
expect(response.body.results).toEqual([{ source: fixture.sources[0], status: 'error' }]);
|
||||
});
|
||||
|
||||
it('does not authorize image syntax inside fenced or inline code', async () => {
|
||||
const fixture = await createFixture({
|
||||
markdown: '```md\n\n```\n``',
|
||||
});
|
||||
const sources = ['FENCED', 'INLINE'];
|
||||
|
||||
const response = await prepare(fixture.app, fixture.directory, sources);
|
||||
|
||||
expect(response.body.results).toEqual(sources.map((source) => ({ source, status: 'error' })));
|
||||
});
|
||||
|
||||
it('rejects paths outside the workspace and approved temporary root', async () => {
|
||||
const fixture = await createFixture();
|
||||
const outsidePath = path.join(fixture.root, 'outside.png');
|
||||
await fs.writeFile(outsidePath, PNG);
|
||||
const source = new URL(`file://${outsidePath}`).toString();
|
||||
fixture.fetchMock.mockResolvedValueOnce(new Response(JSON.stringify({
|
||||
info: { id: 'msg_1', role: 'assistant' },
|
||||
parts: [{ type: 'text', text: `` }],
|
||||
}), { status: 200, headers: { 'content-type': 'application/json' } }));
|
||||
|
||||
const response = await prepare(fixture.app, fixture.directory, [source]);
|
||||
expect(response.body.results).toEqual([{ source, status: 'error' }]);
|
||||
});
|
||||
|
||||
it('rejects non-image bytes and symlink escapes per source', async () => {
|
||||
const fixture = await createFixture({ sources: ['invalid.png', 'linked.png'] });
|
||||
await fs.writeFile(path.join(fixture.directory, 'invalid.png'), 'not an image');
|
||||
await fs.writeFile(path.join(fixture.root, 'outside.png'), PNG);
|
||||
await fs.symlink(path.join(fixture.root, 'outside.png'), path.join(fixture.directory, 'linked.png'));
|
||||
|
||||
const response = await prepare(fixture.app, fixture.directory, fixture.sources);
|
||||
expect(response.body.results).toEqual([
|
||||
{ source: 'invalid.png', status: 'error' },
|
||||
{ source: 'linked.png', status: 'error' },
|
||||
]);
|
||||
});
|
||||
});
|
||||
@@ -15,6 +15,7 @@ import { registerProjectIconRoutes } from './project-icon-routes.js';
|
||||
import { registerScheduledTaskRoutes } from '../scheduled-tasks/routes.js';
|
||||
import { registerOpenChamberSessionRoutes } from '../openchamber-sessions/routes.js';
|
||||
import { registerOpenChamberControlRoutes } from '../openchamber-control/routes.js';
|
||||
import { registerMarkdownImageGrantRoutes } from '../markdown-image-grants/routes.js';
|
||||
import { registerSkillRoutes } from './skill-routes.js';
|
||||
import { registerPluginRoutes } from './plugin-routes.js';
|
||||
import { getNpmInfo, clearCache as clearNpmCache } from './npm-registry.js';
|
||||
@@ -190,6 +191,16 @@ export const createFeatureRoutesRuntime = (dependencies) => {
|
||||
|
||||
registerOpenChamberControlRoutes(app, { controlService: openChamberControlService });
|
||||
|
||||
registerMarkdownImageGrantRoutes(app, {
|
||||
fsPromises,
|
||||
path,
|
||||
os,
|
||||
crypto,
|
||||
validateDirectoryPath,
|
||||
buildOpenCodeUrl,
|
||||
getOpenCodeAuthHeaders,
|
||||
});
|
||||
|
||||
registerConfigEntityRoutes(app, {
|
||||
resolveProjectDirectory,
|
||||
resolveOptionalProjectDirectory,
|
||||
|
||||
Reference in New Issue
Block a user