Skip to content

Commit 633d0a8

Browse files
committed
fix(web): #105 review — a sign-in link nobody asked for signs nobody in; search, diagram links, completions and storage work in a tab
Sixteen findings: four from the code-quality check, twelve from Copilot. All held up against the code; each substantive one is fixed with a test that fails on the code as reviewed. Signing in - callback.js forwarded every parameter of the link — `token=` and `refresh=` included — to the extension, whose old token-in-the-link branch stored them, and the page's start-up delivers what callback.html left even when no sign-in was pending. A crafted link to callback.html, followed by opening the editor, signed the user in to the link-maker's account. callback.html now passes on a sign-in to levelcode.levelcode-ai/auth/callback and only its one-time code; and in a browser (new capability `host.caps.unaskedSignIn`, off there) the extension takes a code only while a sign-in of its own is pending — never a token in a link. The desktop is unchanged. - A sign-in started from a folder or workspace opened from the computer came back to `/`, the scratch workspace. The editor notes the address it leaves (minus `signin=1`); callback.html returns there after checking it is the editor page of this site. - With no persistent secret store (no IndexedDB or WebCrypto, or a refused write), the PKCE verifier lived in memory and was gone when the tab came back from the sign-in. The tab's secrets now ride through that one navigation in sessionStorage and are taken back, and removed, at the next start. - Two tabs opened at once on a fresh profile could each create and store a different sealing key; each then read the other's sealed session as garbage and cleared it. The key is stored in one readwrite transaction, only if none is there, and every tab uses the key that is. The agent in a tab - The content scans that stand in for ripgrep (agent.js scanSearch, extension.js contentHits) capped the file list across the whole workspace before filtering to the folder searched: in a two-folder workspace the other folder could use up the cap. Both now search under the folder (RelativePattern) and cap that. - Diagram links were checked with fs.statSync/realpathSync, which in the browser build reach a shim with no workspace in it, so every link was dropped. Drawing now checks against the files the extension has indexed; a click checks the file system itself (host.withStats, the async-loader pattern of host.withReads). - Pending Keep/Undo reviews are read back asynchronously in a browser; a chat that asked for its cards first was sent none and never got them. They are sent once they are read. - Inline completion refused every document that is not `file` or `untitled` — all of the scratch workspace. It now asks host.isUserDocument, which in a browser also accepts the workspace folders' own schemes. The scratch workspace - A rename or copy over an existing folder deleted only the folder's own entry: its files stayed under the new name, visible to search. The whole destination subtree goes in the same transaction; a move over a folder that contains the source is refused. - With IndexedDB unavailable, activation returned before registering "Open a folder from your computer" — the one way out the warning names. It is registered first. Hosting - The generated nginx block answered 404 at `/`: `try_files $uri` looks for a file, and an exact location keeps the request from the fallback. The root rule names /index.html. - The build id hashed a list of inputs and missed the rest — the declarative built-ins (themes, grammars), the web client's CSS, workers and nls — so a changed file was served stale under the same immutable URL. The build is now staged first and named after a hash of everything staged. Checked by building: two builds give the same id; a change to the client's CSS alone gives a new one (the reviewed script gave the same id). Code quality: an unused SYSTEM_TOKENS_EST (unused on develop too), an unused import and two unused helpers in the browser tests. Tests: new suites callback, signin, headers, search-scope and review-rehydrate; main, host and workspace extend theirs (fake-idb.js is a small IndexedDB stand-in for them). The unit gate passes, 75 files, on Node 24 and Node 18. In Chrome against a build with these changes: e2e 28/28, e2e-signin-tab 20/20 (its left-behind-code case now expects the code to be delivered and NOT exchanged), e2e-byok 5/5. Not run: a real nginx (Docker was unavailable), Safari, Firefox.
1 parent 54b479f commit 633d0a8

24 files changed

Lines changed: 1050 additions & 115 deletions

‎docs/WEB.md‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,28 +96,39 @@ The extension's existing PKCE flow, with the browser's pieces in place of the OS
9696
1 extension accountSignIn(): verifier → SecretStorage; builds
9797
<account>/ai/login?redirect_uri=<callback>&code_challenge=<S256>
9898
2 extension host.openAuth(uri) → command levelcode.web.openAuthUrl (registered by main.js)
99-
3 main.js checks the address is <account>/ai/…, marks the tab (sessionStorage), location.assign
99+
3 main.js checks the address is <account>/ai/…, notes where the tab is (sessionStorage), location.assign
100100
4 account already signed in there → mints a one-time code bound to the challenge, 302 to
101101
<editor>/callback.html?vscode-reqid=N&vscode-scheme=levelcode
102102
&vscode-authority=levelcode.levelcode-ai&vscode-path=%2Fauth%2Fcallback&code=…
103103
(not signed in → the login page first)
104-
5 callback writes localStorage "vscode-web.url-callbacks[N]" (dated), sees the mark → goes back to /
104+
5 callback checks it is a sign-in to levelcode.levelcode-ai/auth/callback, writes localStorage
105+
"vscode-web.url-callbacks[N]" (dated, the code only), goes back to where the tab was
105106
6 main.js at start-up, once the workbench is listening, delivers a fresh result as
106107
levelcode://levelcode.levelcode-ai/auth/callback?code=… to the URL service
107-
7 extension handleAuthCallback → POST /api/levelcode/v1/auth/exchange {code, verifier} → tokens
108+
7 extension handleAuthCallback: a sign-in is pending (a verifier is stored) →
109+
POST /api/levelcode/v1/auth/exchange {code, verifier} → tokens
108110
```
109111

110112
Why the tab leaves instead of opening a pop-up: a window opened from a chain that began in a
111113
webview is the first thing a strict browser (Safari, a phone) refuses. Leaving the tab and
112114
returning needs no permission. The PKCE verifier lives in SecretStorage, so it survives the
113-
navigation.
115+
navigation — and where the browser lets nothing persist (no IndexedDB or WebCrypto, or a refused
116+
write), the tab's secrets ride through that one navigation in sessionStorage and are taken back,
117+
and removed, when the editor starts again. The tab comes back to the address it left: a folder or
118+
workspace opened from the computer is opened again, not the scratch workspace.
114119

115120
- **`?signin=1`** (the account site's "Open LevelCode" link) makes step 1 happen by itself, once:
116121
`main.js` removes the parameter, then delivers `levelcode://…/launch`, which focuses the chat and
117122
starts the sign-in only when there is no session.
118123
- **A result is delivered only if it is fresh** (written in the last two minutes — the one-time
119124
code lives for a minute) and is removed either way. One left by a callback page whose editor
120125
tab is gone is delivered when the editor next starts.
126+
- **`callback.html` is a page anyone can link to**, and what it leaves is delivered at the next start.
127+
So it passes on a LevelCode sign-in and nothing else: for `levelcode.levelcode-ai/auth/callback`
128+
only, and only the one-time `code` — never a token in the link (the old flow, which the desktop
129+
still accepts) or the rest of the query. The extension then takes a code only while a sign-in of
130+
its own is pending (`host.caps.unaskedSignIn` is off in a browser): a link nobody asked for cannot
131+
sign the user in to someone else's account.
121132
- **`levelcode.web.openAuthUrl` is an open-redirect if it is loose**, and any extension in the page
122133
can call it. It goes only to `<account origin>/ai/…`.
123134
- The address handed over is `encodeURI(uri.toString(true))` — the string the editor's own opener
@@ -144,7 +155,7 @@ node web/serve.mjs --dist dist-web # http://127.0.0.1:8800
144155
| `--webview-origin 'https://{{uuid}}.view.example.com'` (`LEVELCODE_WEB_WEBVIEW_ORIGIN`) | Self-host the webview origin. Without it the page uses Code-OSS's CDN at this build's own commit. Needs wildcard DNS and TLS — see below. |
145156
| `--ext-host-origin 'https://{{uuid}}.ext.example.com'` (`LEVELCODE_WEB_EXTHOST_ORIGIN`) | Run the extension host on an origin of its own. Optional while no third-party code is loaded. |
146157
| `--static <dir>` | Use a prebuilt client (skips the gulp build). |
147-
| `--id <id>` | Override the build id (default: a fingerprint of the client, the page and the extension). |
158+
| `--id <id>` | Override the build id (default: a hash of everything the build put under `/_/<id>/`, staged first and named after it). |
148159

149160
**Dev server** (`web/serve.mjs`): `--static <vscode-web dir> --extensions <dir> --extension-names
150161
levelcode-ai,levelcode-web --account <origin>`, plus `--product <json>` and `--config-defaults
@@ -234,7 +245,9 @@ causing the effect). Use another registrable domain.
234245
| Editor settings, layout | the workbench's own storage | editor origin |
235246

236247
Clearing the browser's site data removes all of it. A private window may discard it on close; the
237-
secret store then falls back to memory for the session.
248+
secret store then falls back to memory for the session. The key the secrets are sealed with is created
249+
once per browser profile: two tabs opened together on a fresh profile store it in one IndexedDB
250+
transaction, so both end with the same key.
238251

239252
## Keeping the desktop and the browser in step
240253

‎extensions/levelcode-ai/agent.js‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,6 @@ const TOOLS = [
7878
{ name: 'use_skill', description: 'Load an expert playbook (SKILL.md) for a task type, chosen from the "Available skills" list in your system prompt. Returns the skill\'s step-by-step instructions as the tool result — then follow them. Read-only and instant (no approval). Call it once, early, when the goal matches a skill\'s description.', input_schema: { type: 'object', properties: { name: { type: 'string', description: 'the exact skill name from the Available skills list' } }, required: ['name'] } }
7979
];
8080

81-
// Rough token estimates (chars/4) for the static prompt segments, so the context popover can break
82-
// down "what's filling the window" — system + tools are sent on every request, the rest is messages.
83-
const SYSTEM_TOKENS_EST = Math.round(SYSTEM_BASE_HOST.length / 4);
84-
8581
/** Append an "Available skills" name+description menu to the base prompt. Bodies NEVER go here —
8682
* progressive disclosure means only name+description are ever in the prompt (~100 words for a dozen). */
8783
function buildSystem(menu) {
@@ -167,7 +163,8 @@ async function scanSearch(term, root) {
167163
const out = [];
168164
let size = 0;
169165
let uris = [];
170-
try { uris = await host.findFiles('**/*', FILE_EXCLUDES, 2000); } catch { return ''; }
166+
// Scoped to `root` BEFORE the cap: across the whole workspace, another folder's files could use up all 2,000.
167+
try { uris = await host.findFiles(new vscode.RelativePattern(host.uriFor(root), '**/*'), FILE_EXCLUDES, 2000); } catch { return ''; }
171168
const rootPrefix = root.endsWith('/') ? root : root + '/';
172169
for (const uri of uris) {
173170
if (size >= 6000) { break; }

‎extensions/levelcode-ai/extension.js‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -931,7 +931,8 @@ async function contentHits(terms, dir) {
931931
if (!dir) { return hits; }
932932
const prefix = dir.endsWith('/') ? dir : dir + '/';
933933
let uris = [];
934-
try { uris = await host.findFiles('**/*', '{**/node_modules/**,**/.git/**,**/out/**,**/dist/**,**/*.map,**/*.min.*}', 400); } catch { return hits; }
934+
// Scoped to `dir` BEFORE the cap: across the whole workspace, another folder's files could use up all 400.
935+
try { uris = await host.findFiles(new vscode.RelativePattern(host.uriFor(dir), '**/*'), '{**/node_modules/**,**/.git/**,**/out/**,**/dist/**,**/*.map,**/*.min.*}', 400); } catch { return hits; }
935936
const needles = terms.map((t) => t.toLowerCase());
936937
const deadline = Date.now() + 4000; // the same budget rgFiles gives ripgrep
937938
for (const uri of uris) {
@@ -1094,7 +1095,10 @@ function recordDiagramStat(ev) {
10941095
const diagramFolders = () => (vscode.workspace.workspaceFolders || []).map((f) => ({ name: f.name, root: f.uri.fsPath }));
10951096
/** This conversation's diagrams. One for the life of the extension; reset with the conversation. */
10961097
const diagrams = createDiagrams({
1097-
resolveLink: (link) => diagramLinks.resolveLink(link, diagramFolders()),
1098+
// A diagram's links are checked as it is drawn, and that cannot wait. Where files can only be looked at
1099+
// asynchronously (LevelCode in a browser) they are checked against the files the extension has indexed
1100+
// (buildFileIndex); the desktop reads the disk, as before. The click checks the file itself.
1101+
resolveLink: (link) => diagramLinks.resolveLink(link, diagramFolders(), { fs: host.knownFilesView([...workspaceFiles.values()].map((u) => u.path)) || undefined }),
10981102
onStat: recordDiagramStat
10991103
});
11001104
/** True once a diagram's spec has left the model's context (a compaction, or a resume that did not fit) —
@@ -1117,7 +1121,7 @@ async function openDiagramLink(id, nodeId) {
11171121
const node = record && record.spec ? record.spec.nodes.find((n) => n.id === String(nodeId)) : null;
11181122
if (!node || !node.link) { dbg('diagram.link.unknown', { id }); return; }
11191123
// Checked again at the moment of the click: the file may have moved, or become a link, since it was drawn.
1120-
const r = diagramLinks.resolveLink(node.link, diagramFolders());
1124+
const r = await host.withStats((fs) => diagramLinks.resolveLink(node.link, diagramFolders(), { fs }));
11211125
if (!r.ok) { vscode.window.showWarningMessage('LevelCode: that link cannot be opened — ' + r.reason + '.'); dbg('diagram.link.refused', { reason: r.reason }); return; }
11221126
recordDiagramStat({ type: 'link' });
11231127
try {
@@ -3567,10 +3571,16 @@ async function handleAuthCallback(uri) {
35673571
try {
35683572
const q = new URLSearchParams(uri.query || '');
35693573
const code = q.get('code');
3574+
const verifier = ctx ? await ctx.secrets.get(ACCOUNT_VERIFIER_KEY) : null;
3575+
// Where the callback is a page anyone can link to (LevelCode in a browser), a link nobody asked for must not
3576+
// sign the user in to someone else's account: only a code that answers a sign-in this editor started is taken.
3577+
if (!host.caps.unaskedSignIn && (!code || !verifier)) {
3578+
dbg('account.callback', { ignored: code ? 'no sign-in pending' : 'not a sign-in code' });
3579+
return;
3580+
}
35703581
// Preferred: PKCE code → exchange for tokens using the verifier we stashed at sign-in.
35713582
if (code) {
35723583
const endpoint = cloudApiUrl(); // token exchange is a BACKEND (Rails) call, not the website
3573-
const verifier = ctx ? await ctx.secrets.get(ACCOUNT_VERIFIER_KEY) : null;
35743584
try {
35753585
const res = await fetch(endpoint + '/api/levelcode/v1/auth/exchange', {
35763586
method: 'POST',

‎extensions/levelcode-ai/host.js‎

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,12 @@ const caps = Object.freeze({
3535
mcpStdio: !isBrowser,
3636
/** Search file contents with the ripgrep binary the editor ships. */
3737
ripgrep: !isBrowser,
38+
/**
39+
* Take a sign-in result that no sign-in of this editor asked for: the old token-in-the-link form, or a code
40+
* with no sign-in pending. The desktop does what it always did. In a browser the callback is a page of the
41+
* editor's own site that anyone can link to, so only a code answering a sign-in this browser started counts.
42+
*/
43+
unaskedSignIn: !isBrowser,
3844
});
3945

4046
const TEXT_SNIFF_BYTES = 8000;
@@ -63,6 +69,21 @@ function uriFor(abs) {
6369
return best ? best.uri.with({ path: abs }) : vscode.Uri.file(abs);
6470
}
6571

72+
/**
73+
* Is this document one of the user's own files — the kind inline completion is offered in? On the desktop a
74+
* `file` or an `untitled` document, as ever. In the browser also a document inside a workspace folder, whatever
75+
* its scheme: the scratch workspace's files are `levelcode-scratch:` ones.
76+
* @param {vscode.Uri} uri
77+
*/
78+
function isUserDocument(uri) {
79+
if (uri.scheme === 'file' || uri.scheme === 'untitled') { return true; }
80+
if (!isBrowser) { return false; }
81+
return folders().some((f) => {
82+
const root = f.uri.path.endsWith('/') ? f.uri.path : f.uri.path + '/';
83+
return f.uri.scheme === uri.scheme && (f.uri.authority || '') === (uri.authority || '') && (uri.path === f.uri.path || uri.path.startsWith(root));
84+
});
85+
}
86+
6687
/* ----- reading and probing ------------------------------------------------------------------- */
6788

6889
/** @param {string} abs @returns {Promise<Uint8Array>} rejects when the file is missing or unreadable. */
@@ -141,6 +162,66 @@ async function withReads(run) {
141162
return result;
142163
}
143164

165+
/* ----- checking paths synchronously --------------------------------------------------------- */
166+
167+
/**
168+
* The part of Node's `fs` a synchronous path check uses — `statSync` and `realpathSync` — answered from `lookup`.
169+
* In the browser nothing is a symbolic link, so a path's real path is the path.
170+
* @param {(abs: string) => ({isFile: boolean, isDirectory: boolean}|null)} lookup
171+
*/
172+
function statView(lookup) {
173+
const missing = (p) => Object.assign(new Error('ENOENT: no such file or directory, ' + p), { code: 'ENOENT' });
174+
return {
175+
statSync(p) {
176+
const s = lookup(String(p));
177+
if (!s) { throw missing(p); }
178+
return { isFile: () => s.isFile, isDirectory: () => s.isDirectory };
179+
},
180+
realpathSync(p) { if (!lookup(String(p))) { throw missing(p); } return String(p); },
181+
};
182+
}
183+
184+
/**
185+
* Run a check written against `fs.statSync` / `fs.realpathSync` (a diagram's code links: diagram/links.js) where
186+
* files can only be looked at asynchronously. Like withReads: the check runs once to learn which paths it asks
187+
* about, those are stat'd, and it runs again with the answers (at most four passes).
188+
* On the desktop it simply runs once against `fs`, as before.
189+
* @template T
190+
* @param {(fs: { statSync: Function, realpathSync: Function }) => T} run
191+
* @returns {Promise<T>}
192+
*/
193+
async function withStats(run) {
194+
if (!isBrowser) { return run(nodeFs); }
195+
/** @type {Map<string, {isFile: boolean, isDirectory: boolean}|null>} */
196+
const known = new Map();
197+
let result;
198+
for (let pass = 0; pass < 4; pass++) {
199+
const asked = new Set();
200+
result = run(statView((abs) => {
201+
if (known.has(abs)) { return known.get(abs); }
202+
asked.add(abs);
203+
return null;
204+
}));
205+
if (asked.size === 0) { break; }
206+
await Promise.all([...asked].map(async (p) => { known.set(p, await stat(p)); }));
207+
}
208+
return result;
209+
}
210+
211+
/**
212+
* The same view over files already known to exist, for a check that cannot wait for the file system (a diagram's
213+
* links are checked as it is drawn). Folders are the files' parents. Null on the desktop: the check reads the disk.
214+
* @param {Iterable<string>} files absolute paths
215+
*/
216+
function knownFilesView(files) {
217+
if (!isBrowser) { return null; }
218+
const set = new Set(files);
219+
const dirs = new Set();
220+
for (const f of set) { for (let d = f.slice(0, f.lastIndexOf('/')); d; d = d.slice(0, d.lastIndexOf('/'))) { dirs.add(d); } }
221+
dirs.add('/');
222+
return statView((abs) => (set.has(abs) ? { isFile: true, isDirectory: false } : dirs.has(abs) ? { isFile: false, isDirectory: true } : null));
223+
}
224+
144225
/* ----- sending the user to sign in ------------------------------------------------------------- */
145226

146227
/** Registered by the page (web/main.js); it takes the tab to the account site and the sign-in comes back to it. */
@@ -175,7 +256,7 @@ async function openAuth(uri) {
175256
/**
176257
* `vscode.workspace.findFiles`, which is what the editor's own search service answers. The browser
177258
* build uses it too: LevelCode's scratch workspace registers a file search provider.
178-
* @param {string} glob
259+
* @param {vscode.GlobPattern} glob a string, or a vscode.RelativePattern to search under one folder
179260
* @param {string} exclude
180261
* @param {number} max
181262
*/
@@ -185,7 +266,7 @@ function findFiles(glob, exclude, max) {
185266

186267
module.exports = {
187268
isBrowser, caps,
188-
uriFor, readBytes, readText, readTextOrNull, stat, exists, isBinary, withReads, findFiles, openAuth,
269+
uriFor, isUserDocument, readBytes, readText, readTextOrNull, stat, exists, isBinary, withReads, withStats, knownFilesView, findFiles, openAuth,
189270
// For tests.
190271
_path: path,
191272
};

‎extensions/levelcode-ai/inlineComplete.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
const vscode = require('vscode');
1212
const { sendWithAuthRetry } = require('./providers/authRetry');
13+
const host = require('./host');
1314

1415
const PREFIX_MAX = 2500; // chars of context before the cursor (smaller = faster first token)
1516
const SUFFIX_MAX = 1000; // chars of context after the cursor
@@ -98,7 +99,7 @@ function registerInlineComplete(context, deps) {
9899
async provideInlineCompletionItems(document, position, ctx, token) {
99100
const cfg = aiConfig();
100101
if (!cfg.get('completions.enabled', true)) { return null; }
101-
if (document.uri.scheme !== 'file' && document.uri.scheme !== 'untitled') { return null; }
102+
if (!host.isUserDocument(document.uri)) { return null; } // file and untitled; in a browser, the workspace's own schemes too
102103

103104
// Debounce: if the user keeps typing, this token is cancelled and we bail.
104105
const debounce = Math.max(0, cfg.get('completions.debounce', 150));

‎extensions/levelcode-ai/reviewSession.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,9 @@ function registerReview(context, post, dbg, recordTouch) {
386386
try { current.set(s.uri, ((await host.readTextOrNull(vscode.Uri.parse(s.uri).fsPath)) || '').replace(/^/, '')); } catch { /* */ }
387387
}
388388
rehydrate((uri, s) => current.get(s.uri) || '');
389+
// The chat may have asked for the cards (resync, on `ready`) while these reads were still out, and was
390+
// sent none. Send them now; a chat that is not listening yet asks again when it is.
391+
if (pending.size) { resync(); }
389392
})();
390393
}
391394

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,8 @@ function boot(over) {
136136
const env = {
137137
vscode,
138138
// The host module (host.js) as the desktop sees it: workspace paths are files on disk.
139-
host: { isBrowser: false, caps: { shell: true, mcpStdio: true, ripgrep: true }, uriFor: (abs) => vscode.Uri.file(abs) },
139+
host: { isBrowser: false, caps: { shell: true, mcpStdio: true, ripgrep: true, unaskedSignIn: true }, uriFor: (abs) => vscode.Uri.file(abs),
140+
withStats: async (run) => run(fs), knownFilesView: () => null },
140141
fs, path, os, catalog, sessionMemory, diagrams, diagramLinks, diagramExport, diagramText, diagramTool, diagramBundle: bundle,
141142
diagramAscii: (over && over.ascii) || ascii,
142143
aiConfig: () => ({ get: (k, d) => (k in settings ? settings[k] : d) }),

0 commit comments

Comments
 (0)