diff --git a/packages/opencode/src/altimate/workspace/api-client.ts b/packages/opencode/src/altimate/workspace/api-client.ts index a10b04ec8d..e3b349bc07 100644 --- a/packages/opencode/src/altimate/workspace/api-client.ts +++ b/packages/opencode/src/altimate/workspace/api-client.ts @@ -162,10 +162,27 @@ async function req( * exchanges and covers the request body too, so a call that uploads * megabytes (a skill bundle) needs its own. */ timeoutMs?: number + /** Act as THIS credential rather than resolving the ambient one. + * + * `creds()` reads the credentials afresh on every call, so a caller that + * needs its request and its own bookkeeping to be about the same principal + * cannot get that by reading them itself — the request would resolve them + * again, and an account switch in between makes the two disagree. Comparing + * before and after does not close it either: A→B→A passes the comparison + * while the request was served as B. Passing the captured credential is the + * only form that cannot drift. + * + * Callers that pass this have already read the credential, so the + * `isConfigured()` gate inside `creds()` — a file-existence check on the + * same file they just read — is skipped. The one behavioural difference: + * deleting the credentials file mid-flight no longer aborts THIS request. + * It still completes as the principal it captured, and the next call fails + * at its own credential read. */ + actAs?: { url: string; instance: string; apiKey: string } } = {}, ): Promise { const timeoutMs = opts.timeoutMs ?? REQUEST_TIMEOUT_MS - const { url, instance, apiKey } = await creds() + const { url, instance, apiKey } = opts.actAs ?? (await creds()) const qs = opts.query ? "?" + new URLSearchParams(opts.query).toString() : "" const basePath = opts.base ?? "/datamate-project-bindings" const target = `${url}${basePath}${subpath}${qs}` @@ -530,7 +547,13 @@ export namespace WorkspaceApi { * gets. (M5) Filters out non-integer / non-positive ids so a corrupt row * doesn't reach the picker as a "NaN" label that the caller then binds * against. */ - export async function listDatamates(): Promise { + /** `actAs` pins the request to a specific credential — see `req`'s `actAs`. Omitted, this + * resolves the ambient credential as every other call does. */ + export async function listDatamates(actAs?: { + url: string + instance: string + apiKey: string + }): Promise { // Accept THREE response envelopes — today's ``{datamates: [...]}``, a // bare ``[...]``, and a generic ``{data: [...]}`` — so a backend // contract change (or compat layer) doesn't silently empty the picker. @@ -538,6 +561,7 @@ export namespace WorkspaceApi { type Row = { id: number | string; name: string; memory_enabled?: boolean; user_id?: number } const body = await req("GET", "/", { base: "/datamates", + ...(actAs ? { actAs } : {}), }) let rows: Row[] if (Array.isArray(body)) { diff --git a/packages/opencode/src/altimate/workspace/pin.ts b/packages/opencode/src/altimate/workspace/pin.ts new file mode 100644 index 0000000000..b0dbe3ee9b --- /dev/null +++ b/packages/opencode/src/altimate/workspace/pin.ts @@ -0,0 +1,138 @@ +// altimate_change - new file +// +// The IDE extension's workspace pin. +// +// `altimate-code serve` is launched by the VS Code / Cursor extension, which already knows which +// datamate the user picked in its panel. That selection is what should govern the session's skills +// and memory — not whatever binding this project happens to carry on the backend. The extension +// hands it over in the child's environment and this module turns it back into a `CachedBinding` +// for `state.ts`'s `resolveBindingOutcome` to return. +// +// Why the environment, and not a `serve` argument: `resolveBindingOutcome` is reached from +// per-turn prompt assembly and from every memory write, neither of which has a path back to the +// command's parsed `args`. `session-context.ts` and `serve.ts`'s `ALTIMATE_CODE_SERVE` both made +// the same call, for the same reason — it has to be readable from every module realm. A `serve` +// flag can still be added as sugar, so long as its handler writes these vars before anything else +// runs. +// +// Why NOT the existing `ALTIMATE_RESOLVED_WORKSPACE_*` namespace, which looks like the obvious +// home: `launch-resolve.ts` sets `ALTIMATE_RESOLVED_WORKSPACE_ID` **alone** for the TUI's +// `--workspace ` flag — no name, no root. Reusing that namespace would make every such TUI +// session look like a half-populated pin, and the fail-closed rule below would then break +// `--workspace` outright. The two mechanisms are kept apart deliberately, and `readPin` additionally +// stands down outside `serve`. +import { realpathSync } from "node:fs" +import path from "node:path" +import { Filesystem } from "@/util/filesystem" +import { Log } from "@/altimate/util/log" + +const log = Log.create({ service: "workspace-pin" }) + +const ENV_ID = "ALTIMATE_PINNED_WORKSPACE_ID" +const ENV_NAME = "ALTIMATE_PINNED_WORKSPACE_NAME" +const ENV_ROOT = "ALTIMATE_PINNED_WORKSPACE_ROOT" + +export interface ValidPin { + kind: "valid" + datamateId: number + datamateName: string + /** The directory `serve` was launched for. The pin applies to this tree and nothing else. */ + root: string +} + +/** + * `absent` and `invalid` are deliberately NOT the same answer. + * + * Collapsing them — the shape `getResolvedWorkspaceId` uses, where anything unparseable returns + * `null` — would make a malformed pin fall through to ordinary cache/server resolution, which can + * legitimately return a DIFFERENT workspace. Silently doing work against a workspace the user did + * not pick is the one outcome this feature must never produce, so a pin that is present but broken + * fails closed instead. + */ +export type PinState = { kind: "absent" } | { kind: "invalid"; reason: string } | ValidPin + +/** + * Whether `directory` is the pinned root or lives underneath it. + * + * Delegates to `Filesystem.containsReal`, which resolves symlinks and — critically — walks up to + * the nearest existing ancestor when the path itself does not exist yet, rejecting `..` segments + * along the way. An earlier version here compared `realpathSync` output with a LEXICAL fallback + * when resolution failed, which a not-yet-created path under a symlinked ancestor defeated: + * `/link/new`, with `link -> /outside`, resolved to nothing, fell back to the literal string, + * and passed the prefix test. Since the directory arrives from the caller-supplied + * `x-opencode-directory` header on an unsecured server, that was enough to attribute an outside + * project's skills and memory to the pinned workspace. + */ +export function withinRoot(directory: string, root: string): boolean { + return resolveWithinRoot(directory, root) !== null +} + +/** + * `withinRoot`, but returning the CANONICAL directory it validated — or `null` when the directory + * is not contained. + * + * Exists because containment is checked once, early, and the caller then does async work + * (credentials, a network round trip) before it needs the directory again. Re-deriving it from the + * caller-supplied string at that point re-opens the window: a symlink swapped in between would be + * resolved the second time and not the first, so the path that was authorised and the path that is + * used need not be the same one. Callers keep this value and use it instead of the raw argument. + * + * The canonical form is the one `resolveProjectIdentifier` would compute — `realpath` where it + * resolves, the normalised absolute path otherwise, so a directory that does not exist yet (which + * `containsReal` accepts, having walked to its nearest existing ancestor) still yields something + * stable to carry forward. + */ +export function resolveWithinRoot(directory: string, root: string): string | null { + if (!Filesystem.containsReal(root, directory)) return null + try { + return realpathSync(directory) + } catch { + return path.resolve(directory) + } +} + +/** + * Read the pin out of the environment. + * + * Returns `absent` outside `serve`: the pin is the extension's channel, and the TUI has its own + * (`--workspace`, via `launch-resolve.ts`). Keeping them from ever being live in the same process + * is cheaper than reasoning about what should win. + */ +export function readPin(env: NodeJS.ProcessEnv = process.env): PinState { + if (env["ALTIMATE_CODE_SERVE"] !== "1") return { kind: "absent" } + + const rawId = env[ENV_ID] + const name = env[ENV_NAME] + const root = env[ENV_ROOT] + + // `absent` means the extension set NOTHING. Tested on key presence, not truthiness: three + // present-but-empty variables are a broken pin, not the absence of one, and collapsing them into + // `absent` let a malformed pin fall through to ordinary cache/server resolution — the exact + // fall-open this function exists to prevent. + if (rawId === undefined && name === undefined && root === undefined) { + return { kind: "absent" } + } + + // Partial or empty is invalid, never "good enough". The extension sets all three or none; + // anything else means something rewrote the environment and we no longer know what was intended. + if (!rawId || !name || !root) { + return { kind: "invalid", reason: "pin is partially set or empty" } + } + + const datamateId = Number(rawId) + if (!Number.isSafeInteger(datamateId) || datamateId <= 0) { + return { kind: "invalid", reason: `datamate id ${JSON.stringify(rawId)} is not a positive integer` } + } + if (!path.isAbsolute(root)) { + return { kind: "invalid", reason: "pinned root is not an absolute path" } + } + + return { kind: "valid", datamateId, datamateName: name, root } +} + +/** `readPin`, with the refusal logged once at the point it is taken. */ +export function readPinLogged(env: NodeJS.ProcessEnv = process.env): PinState { + const pin = readPin(env) + if (pin.kind === "invalid") log.warn("ignoring workspace pin and failing closed", { reason: pin.reason }) + return pin +} diff --git a/packages/opencode/src/altimate/workspace/state.ts b/packages/opencode/src/altimate/workspace/state.ts index 2b320aae3f..f7520ec36d 100644 --- a/packages/opencode/src/altimate/workspace/state.ts +++ b/packages/opencode/src/altimate/workspace/state.ts @@ -10,6 +10,7 @@ // ``Global.Path.state`` at 0o600 — chmod is applied post-write since // ``Filesystem.writeJsonAtomic`` does not chmod (see filesystem.ts:294 for // why; codex round-2 flagged this gap). +import { createHash } from "node:crypto" import { chmodSync, existsSync, readFileSync, realpathSync } from "node:fs" import path from "node:path" import { AltimateApi } from "@/altimate/api/client" @@ -19,6 +20,9 @@ import { Log } from "@/altimate/util/log" // Type-only: the value side is imported dynamically in resolveBinding to keep // this module's import graph free of the API client at load time. import type { Binding, ProjectBindingLookup } from "./api-client" +// altimate_change — the IDE extension's workspace pin; see ./pin.ts +import { readPinLogged, resolveWithinRoot, type ValidPin } from "./pin" +import { resolveProjectIdentifier } from "./detect" const CACHE_VERSION = 1 @@ -42,6 +46,17 @@ export interface CachedBinding { * seed has not run, errored, or was skipped because memory was off — all of * which must stay retryable, so a later warm sweeps again. */ seededAt?: number + /** altimate_change — this binding came from the IDE extension's workspace pin + * (see ./pin.ts), not from disk or the server. Ephemeral for the life of the + * process: it is never written to the cache, and `readCache` strips it back + * off anything that claims it on disk. Callers use it to tell an explicit, + * user-made selection apart from a binding the server volunteered. + * + * No production consumer yet, by design: the write-authorization guard it exists for + * (`memory-sync` currently has no `adopted` checks at all, so adopted bindings are writable + * despite the contract documented above) is a pre-existing gap being fixed separately. This + * carries the signal that guard will key on, so the two land independently. */ + pinned?: boolean } interface CacheFile { @@ -84,6 +99,11 @@ function isValidCacheFile(raw: unknown): raw is CacheFile { (typeof b.projectPath === "string" && b.projectPath.length > 0) if (!hasIdentity) return false if (typeof b.linkedAt !== "number") return false + // altimate_change — `pinned` is the IDE extension's in-process authorization marker and is + // never written here. Anything on disk claiming it is corrupt or hand-edited, and honouring it + // would let a file grant the write access a real selection is supposed to gate. Strip, don't + // reject: the rest of the row is still usable. + if ("pinned" in b) delete b.pinned // A corrupt marker must not read as "already seeded" and suppress the sweep. if (b.seededAt !== undefined && typeof b.seededAt !== "number") return false } @@ -369,7 +389,249 @@ export type BindingOutcome = | { status: "unbound" } | { status: "unknown" } +/** How long a validated pin is trusted before its datamate is re-checked against the account. + * Matches `REVALIDATE_MS` deliberately — this is the same question that rule answers, asked of a + * different source of truth. */ +export const PIN_VALIDATION_TTL_MS = REVALIDATE_MS + +/** Memoized `listDatamates` verdicts, keyed by `tenant|apiUrl|datamateId`. `resolveBindingOutcome` + * runs on every turn AND every memory write, so an unmemoized probe would put an HTTP round trip + * on both hot paths. Only SUCCESSFUL verdicts are stored: caching a failure would turn one network + * blip into five minutes of a dead workspace. */ +const pinValidation = new Map() + +/** How long a validated pin may keep being served once the account becomes UNREACHABLE. Bounded + * on purpose: without it, one successful validation plus an indefinitely failing endpoint would + * serve the pin forever. Beyond this the pin fails closed until the account answers again. */ +export const PIN_STALE_IF_ERROR_MS = 30 * 60 * 1000 + +/** Reads the clock. Seam so tests can advance past the TTLs above instead of sleeping — without it + * a second resolution is always a cache hit and the stale-on-error branches cannot be exercised. */ +let pinNow: () => number = () => Date.now() + +/** Exposed for tests; production never calls either. */ +export function __resetPinValidation(clock?: () => number): void { + pinValidation.clear() + pinNow = clock ?? (() => Date.now()) +} + +/** + * Whether a `listDatamates()` failure may be treated as "the account is unreachable" and so allow + * a previously-validated pin to keep being served. + * + * Only transport-shaped failures qualify. An authorization failure is a real answer — the account + * no longer has access — and collapsing it into the offline path lets a revoked or rotated + * credential keep its previous authorization until the process restarts. + * + * Classified by ERROR TYPE, not by a `status` property. `api-client.ts` throws `ForbiddenError`, + * `NotFoundError` and `NotConfiguredError` as plain named errors carrying NO status field, so a + * status-based test silently sorted a real 403 into the transient bucket — the exact opposite of + * the intent. Only `WorkspaceApiError` carries a status, and it is also what a genuine transport + * failure ("Cannot reach …") is reported as, with the status left undefined. + * + * The default is NOT transient: for a check that decides whether to keep serving authorization, + * an error we cannot classify must fail closed. + */ +async function isTransientApiFailure(err: unknown): Promise { + const { ForbiddenError, NotFoundError, NotConfiguredError, ConflictError, PreconditionFailedError, WorkspaceApiError } = + await import("./api-client") + // Definite answers about access or identity. Never transient. + if ( + err instanceof ForbiddenError || + err instanceof NotFoundError || + err instanceof NotConfiguredError || + err instanceof ConflictError || + err instanceof PreconditionFailedError + ) { + return false + } + if (err instanceof WorkspaceApiError) { + // No status = could not reach the host at all (DNS, refused, TLS, abort, timeout). + if (err.status === undefined) return true + // 408/429 are retry-shaped; 5xx is the server failing rather than the caller being refused. + return err.status === 408 || err.status === 429 || err.status >= 500 + } + return false +} + +/** Per-directory `resolveProjectIdentifier` cache. That call runs `spawnSync("git", …)` with a + * 3s timeout, and `resolveBindingOutcome` is reached per turn AND per memory write — so leaving it + * on the hot path put a synchronous subprocess (and up to 3s of blocked event loop) on both. The + * repo remote and project path do not change for the life of a `serve` process. */ +const projectIdentifierCache = new Map>() + +/** Bounded because the key derives from a caller-supplied directory. `resolveWithinRoot` proves + * containment, not existence, and `serve` is unsecured by default — so a local caller can ask about an unlimited + * number of distinct nonexistent subpaths under the pinned root and grow this map forever. In + * normal use a `serve` process sees one directory, so the ceiling is never approached. */ +const PROJECT_IDENTIFIER_CACHE_MAX = 256 + +function cachedProjectIdentifier(directory: string): ReturnType { + const hit = projectIdentifierCache.get(directory) + if (hit) { + // Refresh recency: re-inserting moves the key to the end of the Map's insertion order, so the + // eviction below drops the least recently USED entry rather than the oldest one, and a hot + // directory cannot be evicted by a flood of one-shot lookups. + projectIdentifierCache.delete(directory) + projectIdentifierCache.set(directory, hit) + return hit + } + const ident = resolveProjectIdentifier(directory) + projectIdentifierCache.set(directory, ident) + if (projectIdentifierCache.size > PROJECT_IDENTIFIER_CACHE_MAX) { + const oldest = projectIdentifierCache.keys().next() + if (!oldest.done) projectIdentifierCache.delete(oldest.value) + } + return ident +} + +/** In-flight `listDatamates()` calls, keyed like `pinValidation`. Concurrent turns that both find + * a cold or expired memo would otherwise each fire their own request before either writes back. + * Correctness was never at stake; this just stops the duplicate round trips. */ +const pinValidationInFlight = new Map>() + +function listDatamatesOnce( + cacheKey: string, + actAs: { url: string; instance: string; apiKey: string }, +): Promise<{ id: number; name: string }[]> { + const existing = pinValidationInFlight.get(cacheKey) + // Safe to share: `cacheKey` contains the credential digest, so two callers only ever join the + // same request when they are acting as the same credential. + if (existing) return existing + const started = (async () => { + const { WorkspaceApi } = await import("./api-client") + return WorkspaceApi.listDatamates(actAs) + })() + pinValidationInFlight.set(cacheKey, started) + // Only the caller that STARTED this request clears it, and only if the slot still holds its own + // promise. A waiter clearing on settle could delete a newer request some third caller had just + // registered, which would put the duplicate round trips straight back. + void started + .catch(() => undefined) + .finally(() => { + if (pinValidationInFlight.get(cacheKey) === started) pinValidationInFlight.delete(cacheKey) + }) + return started +} + +/** + * Resolve the extension's pin into a binding, or refuse. + * + * Refusals are `unknown`, never `unbound`: `unbound` is a positive statement that this project has + * no workspace, which callers act on destructively (`skill-sync` takes a snapshot out of service on + * it). "We could not confirm the pin" does not justify that. + */ +async function resolvePinnedBinding(directory: string, pin: ValidPin): Promise { + // The pin is scoped to the tree `serve` was launched for. `serve` resolves an instance per + // request from the `x-opencode-directory` header and runs unsecured by default, so without this + // any local caller could have an unrelated directory's memory attributed to the pinned workspace. + // Keep the path containment actually validated. Everything below awaits — credentials, a network + // round trip — and re-deriving the directory from the caller's string afterwards would authorise + // one path and then use whatever that string resolves to later. + const canonicalDirectory = resolveWithinRoot(directory, pin.root) + if (canonicalDirectory === null) { + log.warn("ignoring workspace pin for a directory outside the pinned root", { directory }) + return { status: "unknown" } + } + + // `tenantKey()` is deliberately not used here: it resolves the same credentials internally, so + // calling both duplicated the work and gave two failure paths an identical log line, leaving the + // message unable to say which had refused. (The validation path below reads credentials a second + // time on purpose — see the TOCTOU note there. That read only happens on a cache miss.) + // + // Scoped to the CREDENTIAL, not just the tenant. `tenantKey()` yields only `{tenant, apiUrl}`, so + // two accounts on one tenant shared a cache entry: switching credentials mid-process let the new + // principal inherit the previous one's successful authorization for the whole TTL, before it had + // demonstrated any visibility of its own. Only a short digest of the key is stored, never the key + // — same treatment as the memory index. + const creds = await AltimateApi.getCredentials().catch(() => null) + if (!creds?.altimateApiKey || !creds.altimateInstanceName || !creds.altimateUrl) { + log.warn("cannot honour the workspace pin: no Altimate credentials resolved") + return { status: "unknown" } + } + const account = createHash("sha256").update(creds.altimateApiKey).digest("hex").slice(0, 16) + const cacheKey = `${creds.altimateInstanceName}|${creds.altimateUrl}|${account}|${pin.datamateId}` + + const memo = pinValidation.get(cacheKey) + // Seeded from the memo rather than the environment: past the TTL an offline fallback would + // otherwise report the stale name the extension started with, discarding the server-confirmed + // one. Inside the TTL this is already `memo.name`, so no second assignment is needed. + let datamateName = memo?.name ?? pin.datamateName + if (!(memo && pinNow() - memo.at < PIN_VALIDATION_TTL_MS)) { + let accessible: { id: number; name: string }[] | null = null + let transient = false + try { + // The request acts as the credential this cache key was derived from, rather than resolving + // the ambient one again. An earlier version compared the credential before and after the + // call instead; that cannot distinguish "unchanged" from "changed and changed back", so an + // A->B->A switch passed the comparison while the answer had been served as B. There is + // nothing left to compare when the request and the key are the same credential by + // construction. + accessible = await listDatamatesOnce(cacheKey, { + url: creds.altimateUrl, + instance: creds.altimateInstanceName, + apiKey: creds.altimateApiKey, + }) + } catch (err) { + // Unreachable and unauthorized are different answers and must not collapse: only the former + // earns the stale grace below. + accessible = null + transient = await isTransientApiFailure(err) + log.warn("could not verify the pinned workspace", { err: String(err), transient }) + } + if (accessible) { + const hit = accessible.find((d) => d.id === pin.datamateId) + // The account genuinely cannot see this workspace. That is a real answer, so fail closed — + // and do not let a previously-cached verdict keep it alive. + if (!hit) { + pinValidation.delete(cacheKey) + log.warn("pinned workspace is not visible to this account", { datamateId: pin.datamateId }) + return { status: "unknown" } + } + // The server's name wins over the environment's, which can be stale if the workspace was + // renamed after the extension spawned this process. + datamateName = hit.name + pinValidation.set(cacheKey, { name: hit.name, at: pinNow() }) + } else if (!memo || !transient || pinNow() - memo.at >= PIN_STALE_IF_ERROR_MS) { + // Nothing was ever established, OR the failure was a refusal rather than a network problem, + // OR the grace window has run out. A revoked credential must stop working, and an endpoint + // that fails forever must not grant an unbounded licence. + pinValidation.delete(cacheKey) + return { status: "unknown" } + } + // Validated earlier and now genuinely unreachable, inside the grace window: keep serving it, + // which is what this module already does for a cached binding rather than tear a working setup + // down over a network blip. + } + + const ident = cachedProjectIdentifier(canonicalDirectory) + // Logged on success as well as on every refusal: a silently-working pin is indistinguishable + // from a pin that was never read, which is exactly the ambiguity that makes this hard to support. + log.info("resolved the workspace pinned by the IDE extension", { + datamateId: pin.datamateId, + datamateName, + }) + return { + status: "bound", + binding: { + datamateId: pin.datamateId, + datamateName, + repoRemote: ident.repoRemote ?? null, + projectPath: ident.projectPath, + linkedAt: Date.now(), + pinned: true, + }, + } +} + export async function resolveBindingOutcome(directory: string): Promise { + // altimate_change — the IDE extension's selection outranks whatever binding this project carries. + // Checked before the local cache and before any server lookup: the whole point is that the panel, + // not the project's history, decides which workspace this `serve` process works against. + const pin = readPinLogged() + if (pin.kind === "invalid") return { status: "unknown" } + if (pin.kind === "valid") return resolvePinnedBinding(directory, pin) + const local = await readLocalBinding(directory).catch(() => null) const key = await tenantKey() diff --git a/packages/opencode/src/tool/bash.ts b/packages/opencode/src/tool/bash.ts index 822348d787..5c3c0a1fda 100644 --- a/packages/opencode/src/tool/bash.ts +++ b/packages/opencode/src/tool/bash.ts @@ -209,6 +209,14 @@ export const BashTool = Tool.define("bash", async () => { // workspace mode is off. A terminal `altimate-code` started from here // under that host is not the host, and would otherwise settle disabled. delete mergedEnv["ALTIMATE_CODE_SERVE"] + // And the IDE extension's workspace pin, for the same reason. Inert while the serve marker + // above is stripped (``readPin`` checks it first), but a child that starts its own nested + // ``altimate-code serve`` would set that marker itself and then inherit a pin the session it + // came from was never given. Defence in depth — the pin should only ever come from the + // process the extension launched. + delete mergedEnv["ALTIMATE_PINNED_WORKSPACE_ID"] + delete mergedEnv["ALTIMATE_PINNED_WORKSPACE_NAME"] + delete mergedEnv["ALTIMATE_PINNED_WORKSPACE_ROOT"] // altimate_change end // altimate_change start — strip the run-mode markers for the same reason. stripRunModeMarkers(mergedEnv) diff --git a/packages/opencode/test/altimate/workspace/pin.test.ts b/packages/opencode/test/altimate/workspace/pin.test.ts new file mode 100644 index 0000000000..ee7e41a92b --- /dev/null +++ b/packages/opencode/test/altimate/workspace/pin.test.ts @@ -0,0 +1,143 @@ +// altimate_change - new file +// +// Unit coverage for the IDE extension's workspace pin parser. Pure: no cache, no network, no +// instance context — `readPin` reads an env bag it is handed, so every case is a plain assertion. +import { afterAll, describe, expect, test } from "bun:test" +import { mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync } from "node:fs" +import os from "node:os" +import path from "node:path" +import { readPin, resolveWithinRoot, withinRoot } from "../../../src/altimate/workspace/pin" + +const ROOT = path.resolve("/tmp/pin-root") + +function env(over: Record = {}): NodeJS.ProcessEnv { + return { + ALTIMATE_CODE_SERVE: "1", + ALTIMATE_PINNED_WORKSPACE_ID: "42", + ALTIMATE_PINNED_WORKSPACE_NAME: "Data Eng", + ALTIMATE_PINNED_WORKSPACE_ROOT: ROOT, + ...over, + } +} + +describe("readPin", () => { + test("a fully-specified pin under serve is valid", () => { + const pin = readPin(env()) + expect(pin.kind).toBe("valid") + if (pin.kind !== "valid") return + expect(pin.datamateId).toBe(42) + expect(pin.datamateName).toBe("Data Eng") + expect(pin.root).toBe(ROOT) + }) + + test("stands down outside serve, so the TUI's --workspace flow is untouched", () => { + // The regression this guards: `launch-resolve.ts` sets only an id for `--workspace`. If the + // pin ever read that shape it would classify it invalid and fail the TUI closed. + expect(readPin(env({ ALTIMATE_CODE_SERVE: undefined })).kind).toBe("absent") + }) + + test("no pin variables at all is absent, not invalid", () => { + const pin = readPin({ ALTIMATE_CODE_SERVE: "1" }) + expect(pin.kind).toBe("absent") + }) + + test.each([ + ["missing name", { ALTIMATE_PINNED_WORKSPACE_NAME: undefined }], + ["missing root", { ALTIMATE_PINNED_WORKSPACE_ROOT: undefined }], + ["missing id", { ALTIMATE_PINNED_WORKSPACE_ID: undefined }], + ])("a partial pin (%s) is invalid, never partially honoured", (_label, over) => { + expect(readPin(env(over as Record)).kind).toBe("invalid") + }) + + test.each([["zero", "0"], ["negative", "-3"], ["non-numeric", "abc"], ["float", "4.5"]])( + "a %s datamate id is invalid", + (_label, id) => { + expect(readPin(env({ ALTIMATE_PINNED_WORKSPACE_ID: id })).kind).toBe("invalid") + }, + ) + + test("a relative root is invalid", () => { + expect(readPin(env({ ALTIMATE_PINNED_WORKSPACE_ROOT: "relative/path" })).kind).toBe("invalid") + }) +}) + +describe("withinRoot — symlink containment", () => { + // The bypass this replaced a lexical fallback to fix. `realpathSync` fails on a path that does + // not exist yet, and the old code then compared the raw string, so a not-yet-created path under + // a symlinked ancestor passed the prefix test and let an outside tree be attributed to the + // pinned workspace. The directory arrives from the caller-supplied `x-opencode-directory`. + const sandbox = mkdtempSync(path.join(os.tmpdir(), "pin-symlink-")) + const root = path.join(sandbox, "root") + const outside = path.join(sandbox, "outside") + mkdirSync(root, { recursive: true }) + mkdirSync(outside, { recursive: true }) + symlinkSync(outside, path.join(root, "link"), "dir") + + afterAll(() => rmSync(sandbox, { recursive: true, force: true })) + + test("an EXISTING directory reached through a symlink out of the root is rejected", () => { + expect(withinRoot(path.join(root, "link"), root)).toBe(false) + }) + + test("a NOT-YET-EXISTING descendant under a symlinked ancestor is rejected", () => { + // The exact case the lexical fallback accepted. + expect(withinRoot(path.join(root, "link", "new"), root)).toBe(false) + }) + + test("a real descendant that does not exist yet is still accepted", () => { + // Fail-closed must not become fail-everything: `serve` legitimately resolves directories that + // have not been created yet. + expect(withinRoot(path.join(root, "pkg", "src"), root)).toBe(true) + }) + + test("a `..` escape is rejected", () => { + expect(withinRoot(path.join(root, "..", "outside"), root)).toBe(false) + }) +}) + +describe("resolveWithinRoot — the validated path is what callers carry forward", () => { + const sandbox2 = mkdtempSync(path.join(os.tmpdir(), "pin-canon-")) + const root2 = path.join(sandbox2, "root") + mkdirSync(path.join(root2, "pkg"), { recursive: true }) + + afterAll(() => rmSync(sandbox2, { recursive: true, force: true })) + + test("returns the resolved path, not the caller's spelling", () => { + // `resolveBindingOutcome` validates containment, then awaits credentials and a network call + // before it needs the directory again. Re-deriving it from the caller's string at that point + // would let a symlink swapped in the gap change which path is used. The canonical form is + // captured once, here. + const messy = path.join(root2, ".", "pkg", "..", "pkg") + const got = resolveWithinRoot(messy, root2) + expect(got).toBe(realpathSync(path.join(root2, "pkg"))) + }) + + test("a directory that does not exist yet still yields a stable path", () => { + const got = resolveWithinRoot(path.join(root2, "not-created-yet"), root2) + expect(got).toBe(path.resolve(root2, "not-created-yet")) + }) + + test("returns null for anything outside the root, matching withinRoot", () => { + const outside = path.resolve("/tmp/definitely-elsewhere") + expect(resolveWithinRoot(outside, root2)).toBeNull() + expect(withinRoot(outside, root2)).toBe(false) + }) +}) + +describe("withinRoot", () => { + test("the root itself and its descendants are in scope", () => { + expect(withinRoot(ROOT, ROOT)).toBe(true) + expect(withinRoot(path.join(ROOT, "pkg", "src"), ROOT)).toBe(true) + }) + + test("a sibling sharing a name prefix is NOT in scope", () => { + // `startsWith` without the separator would call `/tmp/pin-root-other` a child of + // `/tmp/pin-root`, which is how a directory outside the pin gets its memory attributed to the + // pinned workspace. + expect(withinRoot(`${ROOT}-other`, ROOT)).toBe(false) + }) + + test("an unrelated directory is not in scope", () => { + expect(withinRoot(path.resolve("/tmp/somewhere-else"), ROOT)).toBe(false) + }) +}) diff --git a/packages/opencode/test/altimate/workspace/state-pin.test.ts b/packages/opencode/test/altimate/workspace/state-pin.test.ts new file mode 100644 index 0000000000..810ee784d7 --- /dev/null +++ b/packages/opencode/test/altimate/workspace/state-pin.test.ts @@ -0,0 +1,308 @@ +// altimate_change - new file +// +// Runtime coverage for the IDE extension's pin inside `resolveBindingOutcome` — the one hook that +// makes the extension's selection govern skills and memory. `pin.test.ts` covers parsing; this +// covers precedence and the refusals, which is where the damage would be. +import { afterAll, afterEach, beforeEach, describe, expect, test } from "bun:test" +import { mkdirSync, rmSync } from "node:fs" +import path from "node:path" +import os from "node:os" + +const ORIGINAL_XDG_STATE_HOME = process.env.XDG_STATE_HOME +const SANDBOX = path.join(os.tmpdir(), `altimate-state-pin-test-${process.pid}-${Date.now()}`) +mkdirSync(path.join(SANDBOX, "state"), { recursive: true }) +process.env.XDG_STATE_HOME = path.join(SANDBOX, "state") + +const { resolveBindingOutcome, __resetPinValidation, PIN_VALIDATION_TTL_MS, PIN_STALE_IF_ERROR_MS } = + await import("../../../src/altimate/workspace/state") +const { AltimateApi } = await import("../../../src/altimate/api/client") +const { WorkspaceApi, ForbiddenError, WorkspaceApiError } = await import( + "../../../src/altimate/workspace/api-client" +) + +const ROOT = path.join(SANDBOX, "project") +mkdirSync(ROOT, { recursive: true }) + +const originalIsConfigured = AltimateApi.isConfigured +const originalGetCreds = AltimateApi.getCredentials +const originalList = WorkspaceApi.listDatamates +type Creds = Awaited> + +function stubCreds(apiKey = "k") { + ;(AltimateApi as unknown as { isConfigured: () => Promise }).isConfigured = async () => true + ;(AltimateApi as unknown as { getCredentials: () => Promise }).getCredentials = async () => + ({ altimateInstanceName: "acme", altimateUrl: "https://api.test", altimateApiKey: apiKey }) as Creds +} + +/** `listDatamates` rejecting with a specific error, for classifying refusals vs transport faults. */ +function stubListError(err: unknown) { + ;(WorkspaceApi as unknown as { listDatamates: () => Promise }).listDatamates = async () => { + throw err + } +} + +/** `listDatamates` verdict for a test: the rows it returns, or a thrown network failure. */ +function stubList(rows: { id: number; name: string }[] | "unreachable") { + ;(WorkspaceApi as unknown as { listDatamates: () => Promise }).listDatamates = async () => { + // What `api-client` actually throws when the host cannot be reached: a `WorkspaceApiError` + // with no status. A plain `Error` would be classified as unclassifiable and fail closed — + // correct behaviour, but it would not be simulating a transport failure. + if (rows === "unreachable") throw new WorkspaceApiError("Cannot reach https://api.test: fetch failed") + return rows + } +} + +function setPin(over: Record = {}) { + const base: Record = { + ALTIMATE_CODE_SERVE: "1", + ALTIMATE_PINNED_WORKSPACE_ID: "237", + ALTIMATE_PINNED_WORKSPACE_NAME: "activity_test", + ALTIMATE_PINNED_WORKSPACE_ROOT: ROOT, + ...over, + } + for (const [k, v] of Object.entries(base)) { + if (v === undefined) delete process.env[k] + else process.env[k] = v + } +} + +const PIN_VARS = [ + "ALTIMATE_CODE_SERVE", + "ALTIMATE_PINNED_WORKSPACE_ID", + "ALTIMATE_PINNED_WORKSPACE_NAME", + "ALTIMATE_PINNED_WORKSPACE_ROOT", +] as const + +/** Captured once, before anything here touches them: the test process may itself have been + * launched with a pin, and deleting unconditionally would strip it for every later suite. */ +const ORIGINAL_PIN_ENV = Object.fromEntries(PIN_VARS.map((k) => [k, process.env[k]])) + +function clearPin() { + for (const k of PIN_VARS) delete process.env[k] +} + +function restorePinEnv() { + for (const k of PIN_VARS) { + const v = ORIGINAL_PIN_ENV[k] + if (v === undefined) delete process.env[k] + else process.env[k] = v + } +} + +/** Drives the module's validation clock so the TTL can be crossed without sleeping. */ +let now = 1_000_000 + +beforeEach(() => { + clearPin() + now = 1_000_000 + __resetPinValidation(() => now) + stubCreds() + stubList([{ id: 237, name: "activity_test" }]) +}) + +afterEach(() => clearPin()) + +afterAll(() => { + restorePinEnv() + __resetPinValidation() + ;(AltimateApi as unknown as { isConfigured: typeof originalIsConfigured }).isConfigured = + originalIsConfigured + ;(AltimateApi as unknown as { getCredentials: typeof originalGetCreds }).getCredentials = + originalGetCreds + ;(WorkspaceApi as unknown as { listDatamates: typeof originalList }).listDatamates = originalList + if (ORIGINAL_XDG_STATE_HOME === undefined) delete process.env.XDG_STATE_HOME + else process.env.XDG_STATE_HOME = ORIGINAL_XDG_STATE_HOME + rmSync(SANDBOX, { recursive: true, force: true }) +}) + +describe("resolveBindingOutcome — extension pin", () => { + test("a valid, account-visible pin binds, and is marked pinned", async () => { + setPin() + const out = await resolveBindingOutcome(ROOT) + expect(out.status).toBe("bound") + if (out.status !== "bound") return + expect(out.binding.datamateId).toBe(237) + expect(out.binding.pinned).toBe(true) + // Never `adopted`: that is the marker for a binding the server volunteered, and it is what the + // memory write guard keys on. + expect(out.binding.adopted).toBeUndefined() + }) + + test("the server's name wins over a stale one in the environment", async () => { + // The workspace can be renamed after the extension spawned this process. + stubList([{ id: 237, name: "renamed_on_server" }]) + setPin({ ALTIMATE_PINNED_WORKSPACE_NAME: "old_name" }) + const out = await resolveBindingOutcome(ROOT) + expect(out.status === "bound" && out.binding.datamateName).toBe("renamed_on_server") + }) + + test("a workspace the account cannot see fails closed", async () => { + stubList([{ id: 999, name: "someone-elses" }]) + setPin() + expect((await resolveBindingOutcome(ROOT)).status).toBe("unknown") + }) + + test("a directory outside the pinned root resolves nothing", async () => { + // `serve` takes a per-request directory and runs unsecured; without this, another local caller + // could have an unrelated tree's memory attributed to the pinned workspace. + setPin() + expect((await resolveBindingOutcome(path.join(SANDBOX, "elsewhere"))).status).toBe("unknown") + }) + + test("a partial pin fails closed rather than falling through to normal resolution", async () => { + setPin({ ALTIMATE_PINNED_WORKSPACE_NAME: undefined }) + expect((await resolveBindingOutcome(ROOT)).status).toBe("unknown") + }) + + test("unreachable before ever validating is unknown — the env name is not authorization", async () => { + stubList("unreachable") + setPin() + expect((await resolveBindingOutcome(ROOT)).status).toBe("unknown") + }) + + test("unreachable AFTER a successful validation keeps serving the pin, past the TTL", async () => { + // The TTL must actually be crossed. Calling again immediately is a cache hit, so the earlier + // version of this test passed even with the stale-on-error branch deleted. + stubList([{ id: 237, name: "renamed_on_server" }]) + setPin({ ALTIMATE_PINNED_WORKSPACE_NAME: "old_env_name" }) + expect((await resolveBindingOutcome(ROOT)).status).toBe("bound") + + now += PIN_VALIDATION_TTL_MS + 1 + stubList("unreachable") + const out = await resolveBindingOutcome(ROOT) + expect(out.status).toBe("bound") + // The server-confirmed name must survive, not regress to the environment's stale one. + expect(out.status === "bound" && out.binding.datamateName).toBe("renamed_on_server") + }) + + test("an authorization failure fails closed even after a successful validation", async () => { + setPin() + expect((await resolveBindingOutcome(ROOT)).status).toBe("bound") + + now += PIN_VALIDATION_TTL_MS + 1 + // The REAL error the API throws for a 403. It carries no `status` field, which is exactly why + // a status-based classifier mis-sorted it as transient. + stubListError(new ForbiddenError()) + // A refusal is a real answer about access; only transport failures earn the offline grace. + expect((await resolveBindingOutcome(ROOT)).status).toBe("unknown") + }) + + test("a real transport failure IS transient and keeps serving inside the window", async () => { + setPin() + expect((await resolveBindingOutcome(ROOT)).status).toBe("bound") + + now += PIN_VALIDATION_TTL_MS + 1 + // What `api-client` throws when the host cannot be reached: a WorkspaceApiError with no status. + stubListError(new WorkspaceApiError("Cannot reach https://api.test: fetch failed")) + expect((await resolveBindingOutcome(ROOT)).status).toBe("bound") + }) + + test("a 5xx IS transient", async () => { + setPin() + expect((await resolveBindingOutcome(ROOT)).status).toBe("bound") + now += PIN_VALIDATION_TTL_MS + 1 + stubListError(new WorkspaceApiError("boom", 503)) + expect((await resolveBindingOutcome(ROOT)).status).toBe("bound") + }) + + test("the offline grace window is finite", async () => { + setPin() + expect((await resolveBindingOutcome(ROOT)).status).toBe("bound") + + now += PIN_STALE_IF_ERROR_MS + 1 + stubList("unreachable") + // An endpoint that fails indefinitely must not grant an unbounded licence. + expect((await resolveBindingOutcome(ROOT)).status).toBe("unknown") + }) + + test("the verification request acts as the credential the cache key was built from", async () => { + // `req()` resolves credentials per call, so reading them here and letting the request read + // them again cannot guarantee both saw the same principal. The request is handed the captured + // credential instead; this asserts it actually arrives. + setPin() + let sawActAs: { apiKey?: string } | undefined + ;(WorkspaceApi as unknown as { listDatamates: (a?: unknown) => Promise }).listDatamates = + async (actAs?: unknown) => { + sawActAs = actAs as { apiKey?: string } + return [{ id: 237, name: "activity_test" }] + } + expect((await resolveBindingOutcome(ROOT)).status).toBe("bound") + expect(sawActAs?.apiKey).toBe("k") + }) + + test("an A->B->A credential swap during verification cannot mis-attribute the answer", async () => { + // The ABA case a before/after comparison cannot see: the digest matches at both ends while the + // request was served as B. Threading the credential removes the question — the request carries + // A, so the answer is A's regardless of what the ambient credential did meanwhile. + setPin() + let sawActAs: { apiKey?: string } | undefined + ;(WorkspaceApi as unknown as { listDatamates: (a?: unknown) => Promise }).listDatamates = + async (actAs?: unknown) => { + sawActAs = actAs as { apiKey?: string } + stubCreds("rotated-key") // A -> B + stubCreds("k") // B -> A, which a before/after check would read as "unchanged" + return [{ id: 237, name: "activity_test" }] + } + expect((await resolveBindingOutcome(ROOT)).status).toBe("bound") + expect(sawActAs?.apiKey).toBe("k") + }) + + test("concurrent resolutions share one listDatamates request", async () => { + setPin() + let calls = 0 + ;(WorkspaceApi as unknown as { listDatamates: () => Promise }).listDatamates = + async () => { + calls++ + await new Promise((r) => setTimeout(r, 10)) + return [{ id: 237, name: "activity_test" }] + } + const [a, b, c] = await Promise.all([ + resolveBindingOutcome(ROOT), + resolveBindingOutcome(ROOT), + resolveBindingOutcome(ROOT), + ]) + expect([a.status, b.status, c.status]).toEqual(["bound", "bound", "bound"]) + expect(calls).toBe(1) + }) + + test("a failed shared request does not poison the next resolution", async () => { + // The in-flight slot must clear on rejection too, or one outage would wedge every later call. + setPin() + stubListError(new WorkspaceApiError("Cannot reach https://api.test: fetch failed")) + expect((await resolveBindingOutcome(ROOT)).status).toBe("unknown") + stubList([{ id: 237, name: "activity_test" }]) + expect((await resolveBindingOutcome(ROOT)).status).toBe("bound") + }) + + test("a different credential in the same tenant does not inherit the authorization", async () => { + setPin() + expect((await resolveBindingOutcome(ROOT)).status).toBe("bound") + + // Same tenant and API URL, different key: the memo must not apply, so the new principal has to + // demonstrate visibility itself — and here the server says it has none. + stubCreds("different-key") + stubList([{ id: 999, name: "not-yours" }]) + expect((await resolveBindingOutcome(ROOT)).status).toBe("unknown") + }) + + test("a validated pin is memoized — no probe per turn or per memory write", async () => { + let calls = 0 + ;(WorkspaceApi as unknown as { listDatamates: () => Promise }).listDatamates = + async () => { + calls++ + return [{ id: 237, name: "activity_test" }] + } + setPin() + await resolveBindingOutcome(ROOT) + await resolveBindingOutcome(ROOT) + await resolveBindingOutcome(ROOT) + expect(calls).toBe(1) + }) + + test("outside serve the pin is ignored entirely, so the TUI --workspace flow is untouched", async () => { + setPin({ ALTIMATE_CODE_SERVE: undefined }) + // Falls through to ordinary resolution, which in this sandbox has no binding and no server. + const out = await resolveBindingOutcome(ROOT) + expect(out.status).not.toBe("bound") + }) +})