openvibers/openvibe.codes Commits
Codes: the API agent cannot read through symlinks, /auth/me is not rate-limited, every sign-out method is same-origin (#35)
Changes
@@@ -34,15 +34,38 @@ const LEVELS = { read: 1, edit: 2, full: 3 };const SHOWN_AS = { list_dir: 'LS', read_file: 'Read', grep: 'Grep', write_file: 'Write', edit_file: 'Edit', run_command: 'Bash' };const SKIP_DIRS = new Set(['.git', 'node_modules', 'dist', 'build', '.next', 'coverage', '.cache']);++/**++ * The real path of abs, with any part that does not exist yet kept as written; null when a component is++ * not a directory. Every existing component is realpath'd, so a symlink cannot hide where the path goes.++ */++function realPath(abs) {++ let p = path.resolve(abs);++ const tail = [];++ for (;;) {++ try { return path.join(fs.realpathSync(p), ...tail); }++ catch (err) {++ if (err.code !== 'ENOENT' && err.code !== 'ENOTDIR') return null;++ const parent = path.dirname(p);++ if (parent === p) return null;++ tail.unshift(path.basename(p));++ p = parent;++ }++ }++}++/** A path inside the working directory, or an Error that says why not. */function inside(cwd, p, { write = false } = {}) {const root = fs.realpathSync(cwd);const abs = path.resolve(root, String(p || '.'));if (abs !== root && !abs.startsWith(`${root}${path.sep}`)) return new Error(`${p} is outside the working directory`);-- const rel = path.relative(root, abs);++ // Resolve symlinks before trusting the textual check: `link/.env` or `link` → a path outside the++ // working directory would otherwise read and write through the link.++ const real = realPath(abs);++ if (real === null || (real !== root && !real.startsWith(`${root}${path.sep}`))) return new Error(`${p} is outside the working directory`);++ const rel = path.relative(root, real);if (/(^|[\\/])\.env(\.|$)/.test(rel)) return new Error('.env files are not read or written');if (write && /(^|[\\/])\.git([\\/]|$)/.test(rel)) return new Error('.git is not written');-- return abs;++ return real;}function runTool(name, args, cwd) {@@@ -71,7 +94,8 @@ function runTool(name, args, cwd) {const hits = [];const walk = (p) => {if (hits.length >= 200) return;-- const st = fs.statSync(p);++ const st = fs.lstatSync(p);++ if (st.isSymbolicLink()) return; // a link can point anywhere: never followedif (st.isDirectory()) { for (const n of fs.readdirSync(p)) if (!SKIP_DIRS.has(n) && !/^\.env/.test(n)) walk(path.join(p, n)); return; }if (st.size > 1_000_000) return;const text = fs.readFileSync(p, 'utf8');@@@ -111,7 +111,9 @@ async function createApp(opts = {}) {app.use(sso.middleware());// ── Sign-in (OAuth2 + PKCE client of OpenVibe.Network) ──-- app.use('/auth/', rateLimit({ windowMs: 15 * 60_000, limit: 60, standardHeaders: true, legacyHeaders: false }));++ // The limit is on the sign-in routes only: the shared navbar asks /auth/me on every page view, so++ // counting it here would 429 (and render the visitor signed out) after 60 views in 15 minutes.++ app.use(['/auth/login', '/auth/callback', '/auth/logout'], rateLimit({ windowMs: 15 * 60_000, limit: 60, standardHeaders: true, legacyHeaders: false }));app.use('/auth', sso.routes());{ const legal = require('openvibe-shared/legal'); app.get(legal.PATHS, legal.handler({ id: 'codes', service: 'codes', host: 'openvibe.codes', name: 'OpenVibe.Codes', profile: 'ugc' })); }@@@ -227,7 +227,11 @@ function createSso({ config, keys, fetchImpl = globalThis.fetch, now = () => Dat+ `<p>Sign out of OpenVibe.Codes?</p><button type="submit">Sign out</button> <a href="/">Cancel</a></form>`);});r.post('/logout', (req, res, next) => (sameOriginRequest(req) ? next() : res.status(403).type('text/plain').send('Sign-out must come from this site.')));++ // The GET (confirm page) and POST (403) handlers above only cover those two methods; this runs for++ // every other one too, so it repeats the same-origin check rather than signing out on a cross-site++ // DELETE (or any other verb).r.all('/logout', async (req, res) => {++ if (!sameOriginRequest(req)) return res.status(403).type('text/plain').send('Sign-out must come from this site.');const rt = req.cookies && req.cookies[REFRESH_COOKIE];if (rt) {fetchImpl(`${config.networkInternalUrl}/oauth/revoke`, {@@@ -46,6 +46,12 @@ const toolResults = (events) => events.filter((e) => e.type === 'user').flatMap(fs.writeFileSync(path.join(work, 'src', 'a.js'), 'const name = 1;\nmodule.exports = name;\n');fs.writeFileSync(path.join(work, '.env'), 'SECRET=1\n');fs.writeFileSync(path.join(dir, 'outside.txt'), 'outside');++ fs.mkdirSync(path.join(dir, 'outside-dir'), { recursive: true });++ fs.writeFileSync(path.join(dir, 'outside-dir', 'secret.txt'), 'OUTSIDE-SECRET-KEY\n');++ // Symlinks inside the working directory: they resolve textually inside it, but point out (or at .env).++ fs.symlinkSync(path.join(dir, 'outside.txt'), path.join(work, 'outside-link'));++ fs.symlinkSync(path.join(work, '.env'), path.join(work, 'env-link'));++ fs.symlinkSync(path.join(dir, 'outside-dir'), path.join(work, 'dir-link'));const env = { PATH: '/nonexistent', OPENVIBE_CODES_HOME: path.join(dir, 'home'), OPENVIBE_CODES_BASE_URL: api.url, OPENVIBE_CODES_MODEL: 'tiny', OPENVIBE_CODES_API_KEY: 'k-123' };const h = createHarness({ env, store: createStore({ env }) });@@@ -90,6 +96,24 @@ const toolResults = (events) => events.filter((e) => e.type === 'user').flatMap(assert.ok(!fs.existsSync(path.join(work, '.git')));});++ await check('a symlink inside the working directory cannot carry a read or write past it', async () => {++ api.set([{ calls: [++ ['read_file', { path: 'outside-link' }],++ ['read_file', { path: 'env-link' }],++ ['read_file', { path: 'dir-link/secret.txt' }],++ ['write_file', { path: 'dir-link/planted.txt', content: 'x' }],++ ['grep', { pattern: 'OUTSIDE-SECRET-KEY' }],++ ] }, { content: 'ok' }]);++ const events = await collect(h.run({ prompt: 'escape', agent: 'openai-compatible', cwd: work, permission: 'full', handoff: false }));++ const res = toolResults(events);++ assert.strictEqual(res.length, 5, JSON.stringify(res.map((c) => c.content)));++ assert.ok(res.slice(0, 4).every((c) => c.is_error), JSON.stringify(res.map((c) => c.content)));++ assert.ok(res.slice(0, 4).every((c) => /outside the working directory|\.env files are not read/.test(c.content)), JSON.stringify(res.map((c) => c.content)));++ assert.strictEqual(res[4].content, 'no matches', 'grep did not follow the linked directory');++ assert.ok(!res.some((c) => /SECRET=1|OUTSIDE-SECRET-KEY/.test(c.content)), 'nothing behind a link came back');++ assert.ok(!fs.existsSync(path.join(dir, 'outside-dir', 'planted.txt')), 'no write through the link');++ });++await check('run_command exists only at the full level, and runs in the working directory', async () => {api.set([{ calls: [['run_command', { command: 'pwd' }]] }, { content: 'ok' }]);const events = await collect(h.run({ prompt: 'where', agent: 'openai-compatible', cwd: work, permission: 'full', handoff: false }));@@@ -58,12 +58,28 @@ const { signJwt } = require('./helpers/mocks');assert.match(cross.text, /method="post"/);const crossPost = await s.get('/auth/logout', { as: staff, method: 'POST', headers: { 'sec-fetch-site': 'cross-site' } });assert.strictEqual(crossPost.status, 403);++ // The GET/POST handlers only cover those verbs; any other (DELETE here) must be refused too.++ const crossDelete = await s.get('/auth/logout', { as: staff, method: 'DELETE', headers: { 'sec-fetch-site': 'cross-site' } });++ assert.strictEqual(crossDelete.status, 403, 'a cross-site DELETE is refused');++ assert.ok(!/codes_at=;|codes_at=deleted/i.test(String(crossDelete.headers.get('set-cookie') || '')), 'the session cookie is kept');const same = await s.get('/auth/logout?next=/improve', { as: staff, headers: { 'sec-fetch-site': 'same-origin' } });assert.strictEqual(same.status, 303);assert.strictEqual(same.headers.get('location'), '/improve');assert.match(String(same.headers.get('set-cookie') || ''), /codes_at=;/);});++ await check('/auth/me is not counted by the sign-in limiter; the sign-in routes still are', async () => {++ // The shared navbar asks /auth/me on every page view; it must not eat (or be eaten by) the++ // 60-per-15-minutes sign-in budget (server/app.js).++ for (let i = 0; i < 65; i++) {++ const me = await s.get('/auth/me');++ assert.strictEqual(me.status, 200, `/auth/me request ${i + 1} answered ${me.status}`);++ }++ let last = 0;++ for (let i = 0; i < 61; i++) last = (await s.get('/auth/login')).status;++ assert.strictEqual(last, 429, 'the sign-in route is still limited');++ });++await s.close();done();})();
@@ -34,15 +34,38 @@ const LEVELS = { read: 1, edit: 2, full: 3 };
@@ -34,15 +34,38 @@ const LEVELS = { read: 1, edit: 2, full: 3 };
const SHOWN_AS = { list_dir: 'LS', read_file: 'Read', grep: 'Grep', write_file: 'Write', edit_file: 'Edit', run_command: 'Bash' };
const SHOWN_AS = { list_dir: 'LS', read_file: 'Read', grep: 'Grep', write_file: 'Write', edit_file: 'Edit', run_command: 'Bash' };
const SKIP_DIRS = new Set(['.git', 'node_modules', 'dist', 'build', '.next', 'coverage', '.cache']);
const SKIP_DIRS = new Set(['.git', 'node_modules', 'dist', 'build', '.next', 'coverage', '.cache']);
+/**
+ * The real path of abs, with any part that does not exist yet kept as written; null when a component is
+ * not a directory. Every existing component is realpath'd, so a symlink cannot hide where the path goes.
+ */
+function realPath(abs) {
+ let p = path.resolve(abs);
+ const tail = [];
+ for (;;) {
+ try { return path.join(fs.realpathSync(p), ...tail); }
+ catch (err) {
+ if (err.code !== 'ENOENT' && err.code !== 'ENOTDIR') return null;
+ const parent = path.dirname(p);
+ if (parent === p) return null;
+ tail.unshift(path.basename(p));
+ p = parent;
+ }
+ }
+}
+
/** A path inside the working directory, or an Error that says why not. */
/** A path inside the working directory, or an Error that says why not. */
function inside(cwd, p, { write = false } = {}) {
function inside(cwd, p, { write = false } = {}) {
const root = fs.realpathSync(cwd);
const root = fs.realpathSync(cwd);
const abs = path.resolve(root, String(p || '.'));
const abs = path.resolve(root, String(p || '.'));
if (abs !== root && !abs.startsWith(`${root}${path.sep}`)) return new Error(`${p} is outside the working directory`);
if (abs !== root && !abs.startsWith(`${root}${path.sep}`)) return new Error(`${p} is outside the working directory`);
- const rel = path.relative(root, abs);
+ // Resolve symlinks before trusting the textual check: `link/.env` or `link` → a path outside the
+ // working directory would otherwise read and write through the link.
+ const real = realPath(abs);
+ if (real === null || (real !== root && !real.startsWith(`${root}${path.sep}`))) return new Error(`${p} is outside the working directory`);
+ const rel = path.relative(root, real);
if (/(^|[\\/])\.env(\.|$)/.test(rel)) return new Error('.env files are not read or written');
if (/(^|[\\/])\.env(\.|$)/.test(rel)) return new Error('.env files are not read or written');
if (write && /(^|[\\/])\.git([\\/]|$)/.test(rel)) return new Error('.git is not written');
if (write && /(^|[\\/])\.git([\\/]|$)/.test(rel)) return new Error('.git is not written');
- return abs;
+ return real;
}
}
function runTool(name, args, cwd) {
function runTool(name, args, cwd) {
@@ -71,7 +94,8 @@ function runTool(name, args, cwd) {
@@ -71,7 +94,8 @@ function runTool(name, args, cwd) {
const hits = [];
const hits = [];
const walk = (p) => {
const walk = (p) => {
if (hits.length >= 200) return;
if (hits.length >= 200) return;
- const st = fs.statSync(p);
+ const st = fs.lstatSync(p);
+ if (st.isSymbolicLink()) return; // a link can point anywhere: never followed
if (st.isDirectory()) { for (const n of fs.readdirSync(p)) if (!SKIP_DIRS.has(n) && !/^\.env/.test(n)) walk(path.join(p, n)); return; }
if (st.isDirectory()) { for (const n of fs.readdirSync(p)) if (!SKIP_DIRS.has(n) && !/^\.env/.test(n)) walk(path.join(p, n)); return; }
if (st.size > 1_000_000) return;
if (st.size > 1_000_000) return;
const text = fs.readFileSync(p, 'utf8');
const text = fs.readFileSync(p, 'utf8');
@@ -111,7 +111,9 @@ async function createApp(opts = {}) {
@@ -111,7 +111,9 @@ async function createApp(opts = {}) {
app.use(sso.middleware());
app.use(sso.middleware());
// ── Sign-in (OAuth2 + PKCE client of OpenVibe.Network) ──
// ── Sign-in (OAuth2 + PKCE client of OpenVibe.Network) ──
- app.use('/auth/', rateLimit({ windowMs: 15 * 60_000, limit: 60, standardHeaders: true, legacyHeaders: false }));
+ // The limit is on the sign-in routes only: the shared navbar asks /auth/me on every page view, so
+ // counting it here would 429 (and render the visitor signed out) after 60 views in 15 minutes.
+ app.use(['/auth/login', '/auth/callback', '/auth/logout'], rateLimit({ windowMs: 15 * 60_000, limit: 60, standardHeaders: true, legacyHeaders: false }));
app.use('/auth', sso.routes());
app.use('/auth', sso.routes());
{ const legal = require('openvibe-shared/legal'); app.get(legal.PATHS, legal.handler({ id: 'codes', service: 'codes', host: 'openvibe.codes', name: 'OpenVibe.Codes', profile: 'ugc' })); }
{ const legal = require('openvibe-shared/legal'); app.get(legal.PATHS, legal.handler({ id: 'codes', service: 'codes', host: 'openvibe.codes', name: 'OpenVibe.Codes', profile: 'ugc' })); }
@@ -227,7 +227,11 @@ function createSso({ config, keys, fetchImpl = globalThis.fetch, now = () => Dat
@@ -227,7 +227,11 @@ function createSso({ config, keys, fetchImpl = globalThis.fetch, now = () => Dat
+ `<p>Sign out of OpenVibe.Codes?</p><button type="submit">Sign out</button> <a href="/">Cancel</a></form>`);
+ `<p>Sign out of OpenVibe.Codes?</p><button type="submit">Sign out</button> <a href="/">Cancel</a></form>`);
});
});
r.post('/logout', (req, res, next) => (sameOriginRequest(req) ? next() : res.status(403).type('text/plain').send('Sign-out must come from this site.')));
r.post('/logout', (req, res, next) => (sameOriginRequest(req) ? next() : res.status(403).type('text/plain').send('Sign-out must come from this site.')));
+ // The GET (confirm page) and POST (403) handlers above only cover those two methods; this runs for
+ // every other one too, so it repeats the same-origin check rather than signing out on a cross-site
+ // DELETE (or any other verb).
r.all('/logout', async (req, res) => {
r.all('/logout', async (req, res) => {
+ if (!sameOriginRequest(req)) return res.status(403).type('text/plain').send('Sign-out must come from this site.');
const rt = req.cookies && req.cookies[REFRESH_COOKIE];
const rt = req.cookies && req.cookies[REFRESH_COOKIE];
if (rt) {
if (rt) {
fetchImpl(`${config.networkInternalUrl}/oauth/revoke`, {
fetchImpl(`${config.networkInternalUrl}/oauth/revoke`, {
@@ -46,6 +46,12 @@ const toolResults = (events) => events.filter((e) => e.type === 'user').flatMap(
@@ -46,6 +46,12 @@ const toolResults = (events) => events.filter((e) => e.type === 'user').flatMap(
fs.writeFileSync(path.join(work, 'src', 'a.js'), 'const name = 1;\nmodule.exports = name;\n');
fs.writeFileSync(path.join(work, 'src', 'a.js'), 'const name = 1;\nmodule.exports = name;\n');
fs.writeFileSync(path.join(work, '.env'), 'SECRET=1\n');
fs.writeFileSync(path.join(work, '.env'), 'SECRET=1\n');
fs.writeFileSync(path.join(dir, 'outside.txt'), 'outside');
fs.writeFileSync(path.join(dir, 'outside.txt'), 'outside');
+ fs.mkdirSync(path.join(dir, 'outside-dir'), { recursive: true });
+ fs.writeFileSync(path.join(dir, 'outside-dir', 'secret.txt'), 'OUTSIDE-SECRET-KEY\n');
+ // Symlinks inside the working directory: they resolve textually inside it, but point out (or at .env).
+ fs.symlinkSync(path.join(dir, 'outside.txt'), path.join(work, 'outside-link'));
+ fs.symlinkSync(path.join(work, '.env'), path.join(work, 'env-link'));
+ fs.symlinkSync(path.join(dir, 'outside-dir'), path.join(work, 'dir-link'));
const env = { PATH: '/nonexistent', OPENVIBE_CODES_HOME: path.join(dir, 'home'), OPENVIBE_CODES_BASE_URL: api.url, OPENVIBE_CODES_MODEL: 'tiny', OPENVIBE_CODES_API_KEY: 'k-123' };
const env = { PATH: '/nonexistent', OPENVIBE_CODES_HOME: path.join(dir, 'home'), OPENVIBE_CODES_BASE_URL: api.url, OPENVIBE_CODES_MODEL: 'tiny', OPENVIBE_CODES_API_KEY: 'k-123' };
const h = createHarness({ env, store: createStore({ env }) });
const h = createHarness({ env, store: createStore({ env }) });
@@ -90,6 +96,24 @@ const toolResults = (events) => events.filter((e) => e.type === 'user').flatMap(
@@ -90,6 +96,24 @@ const toolResults = (events) => events.filter((e) => e.type === 'user').flatMap(
assert.ok(!fs.existsSync(path.join(work, '.git')));
assert.ok(!fs.existsSync(path.join(work, '.git')));
});
});
+ await check('a symlink inside the working directory cannot carry a read or write past it', async () => {
+ api.set([{ calls: [
+ ['read_file', { path: 'outside-link' }],
+ ['read_file', { path: 'env-link' }],
+ ['read_file', { path: 'dir-link/secret.txt' }],
+ ['write_file', { path: 'dir-link/planted.txt', content: 'x' }],
+ ['grep', { pattern: 'OUTSIDE-SECRET-KEY' }],
+ ] }, { content: 'ok' }]);
+ const events = await collect(h.run({ prompt: 'escape', agent: 'openai-compatible', cwd: work, permission: 'full', handoff: false }));
+ const res = toolResults(events);
+ assert.strictEqual(res.length, 5, JSON.stringify(res.map((c) => c.content)));
+ assert.ok(res.slice(0, 4).every((c) => c.is_error), JSON.stringify(res.map((c) => c.content)));
+ assert.ok(res.slice(0, 4).every((c) => /outside the working directory|\.env files are not read/.test(c.content)), JSON.stringify(res.map((c) => c.content)));
+ assert.strictEqual(res[4].content, 'no matches', 'grep did not follow the linked directory');
+ assert.ok(!res.some((c) => /SECRET=1|OUTSIDE-SECRET-KEY/.test(c.content)), 'nothing behind a link came back');
+ assert.ok(!fs.existsSync(path.join(dir, 'outside-dir', 'planted.txt')), 'no write through the link');
+ });
+
await check('run_command exists only at the full level, and runs in the working directory', async () => {
await check('run_command exists only at the full level, and runs in the working directory', async () => {
api.set([{ calls: [['run_command', { command: 'pwd' }]] }, { content: 'ok' }]);
api.set([{ calls: [['run_command', { command: 'pwd' }]] }, { content: 'ok' }]);
const events = await collect(h.run({ prompt: 'where', agent: 'openai-compatible', cwd: work, permission: 'full', handoff: false }));
const events = await collect(h.run({ prompt: 'where', agent: 'openai-compatible', cwd: work, permission: 'full', handoff: false }));
@@ -58,12 +58,28 @@ const { signJwt } = require('./helpers/mocks');
@@ -58,12 +58,28 @@ const { signJwt } = require('./helpers/mocks');
assert.match(cross.text, /method="post"/);
assert.match(cross.text, /method="post"/);
const crossPost = await s.get('/auth/logout', { as: staff, method: 'POST', headers: { 'sec-fetch-site': 'cross-site' } });
const crossPost = await s.get('/auth/logout', { as: staff, method: 'POST', headers: { 'sec-fetch-site': 'cross-site' } });
assert.strictEqual(crossPost.status, 403);
assert.strictEqual(crossPost.status, 403);
+ // The GET/POST handlers only cover those verbs; any other (DELETE here) must be refused too.
+ const crossDelete = await s.get('/auth/logout', { as: staff, method: 'DELETE', headers: { 'sec-fetch-site': 'cross-site' } });
+ assert.strictEqual(crossDelete.status, 403, 'a cross-site DELETE is refused');
+ assert.ok(!/codes_at=;|codes_at=deleted/i.test(String(crossDelete.headers.get('set-cookie') || '')), 'the session cookie is kept');
const same = await s.get('/auth/logout?next=/improve', { as: staff, headers: { 'sec-fetch-site': 'same-origin' } });
const same = await s.get('/auth/logout?next=/improve', { as: staff, headers: { 'sec-fetch-site': 'same-origin' } });
assert.strictEqual(same.status, 303);
assert.strictEqual(same.status, 303);
assert.strictEqual(same.headers.get('location'), '/improve');
assert.strictEqual(same.headers.get('location'), '/improve');
assert.match(String(same.headers.get('set-cookie') || ''), /codes_at=;/);
assert.match(String(same.headers.get('set-cookie') || ''), /codes_at=;/);
});
});
+ await check('/auth/me is not counted by the sign-in limiter; the sign-in routes still are', async () => {
+ // The shared navbar asks /auth/me on every page view; it must not eat (or be eaten by) the
+ // 60-per-15-minutes sign-in budget (server/app.js).
+ for (let i = 0; i < 65; i++) {
+ const me = await s.get('/auth/me');
+ assert.strictEqual(me.status, 200, `/auth/me request ${i + 1} answered ${me.status}`);
+ }
+ let last = 0;
+ for (let i = 0; i < 61; i++) last = (await s.get('/auth/login')).status;
+ assert.strictEqual(last, 429, 'the sign-in route is still limited');
+ });
+
await s.close();
await s.close();
done();
done();
})();
})();