Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 11 additions & 45 deletions apps/daemon/src/collab/created-project-workspace.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import type { ApiErrorResponse } from '@open-design/contracts';
import type { Response } from 'express';
import {
ambientWorkspaceResourceContext,
isWorkspaceResourceLocked,
withLastKnownMembership,
workspaceResourceContextFromRequest,
type AmbientWorkspaceSnapshot,
type GetAmbientWorkspace,
type GetLastKnownWorkspaceMembership,
type WorkspaceResourceContext,
} from './workspace-resource-mutation.js';
Expand All @@ -13,34 +16,12 @@ import {
} from './vela-workspace-context.js';
import { sendApiError } from '../http/api-errors.js';

/**
* The daemon's own last-verified workspace identity, narrowed to the fields a
* created project's binding needs.
*
* Structurally a subset of `WorkspaceCollabContext`, so a caller passes
* `() => workspaceContext.lastKnown?.() ?? null` directly. Deliberately NOT the
* `WorkspaceContextProvider` type — same reason `workspace-resource-mutation.ts`
* takes a plain `GetLastKnownWorkspaceMembership` closure: a create path must not
* drag the async B integration into a module every resource type depends on.
*/
export type AmbientWorkspaceSnapshot = {
workspaceId: string;
workspaceType: 'personal' | 'team';
workspaceMemberId: string;
role: WorkspaceResourceContext['role'];
memberStatus: WorkspaceResourceContext['memberStatus'];
lifecycleState: WorkspaceResourceContext['lifecycleState'];
permissions: { canShareProjects: boolean; canWriteSyncedFiles: boolean };
};

/**
* Read the daemon's ambient workspace with NO network I/O and no failure mode.
* Backed by `collab/workspace-context.ts`'s `lastKnown()`, which is populated as
* a side effect of the `.current()` traffic the daemon already serves (the web
* client's periodic `GET /api/workspace/context` poll, the collab-cloud poller's
* 5s tick, the dev/demo `PUT /api/workspace/context` seam).
*/
export type GetAmbientWorkspace = () => AmbientWorkspaceSnapshot | null | undefined;
// The ambient-identity types live in `workspace-resource-mutation.ts` (the lower
// module every resource type already depends on) so the mutation gate and the
// creation paths cannot drift into two different notions of "the daemon's own
// workspace". Re-exported here because the creation-path callers import them
// from this module.
export type { AmbientWorkspaceSnapshot, GetAmbientWorkspace };

export type CreatedProjectWorkspaceResolution =
| { ok: true; context: WorkspaceResourceContext | null }
Expand Down Expand Up @@ -206,23 +187,8 @@ export async function authorizeCreatedProjectWorkspace(
function ambientWorkspaceHome(
getAmbientWorkspace?: GetAmbientWorkspace,
): WorkspaceResourceContext | null {
const ambient = getAmbientWorkspace?.();
if (!ambient) return null;
const workspaceId = ambient.workspaceId?.trim();
const workspaceMemberId = ambient.workspaceMemberId?.trim();
if (!workspaceId || !workspaceMemberId) return null;
const context: WorkspaceResourceContext = {
workspaceId,
workspaceType: ambient.workspaceType === 'team' ? 'team' : 'personal',
workspaceTypeAsserted: null,
appUserId: '',
workspaceMemberId,
role: ambient.role,
memberStatus: ambient.memberStatus,
lifecycleState: ambient.lifecycleState,
canShareProjects: ambient.permissions.canShareProjects,
canWriteSyncedFiles: ambient.permissions.canWriteSyncedFiles,
};
const context = ambientWorkspaceResourceContext(getAmbientWorkspace);
if (!context) return null;
if (
context.memberStatus !== 'active'
|| !context.canWriteSyncedFiles
Expand Down
162 changes: 144 additions & 18 deletions apps/daemon/src/collab/workspace-resource-mutation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,60 @@ export type WorkspaceMembershipSnapshot = {

export type GetLastKnownWorkspaceMembership = () => WorkspaceMembershipSnapshot | null;

/**
* The daemon's OWN signed-in workspace identity, narrowed to the fields a
* resource decision needs. Structurally a subset of `WorkspaceCollabContext`, so
* a caller passes `() => workspaceContext.lastKnown?.() ?? null` directly.
*
* Deliberately a plain closure rather than the provider type, same rule as
* `GetLastKnownWorkspaceMembership` above: this module must stay free of the
* async B integration that every resource type depends on.
*
* Populated with NO network I/O and no UI involvement — `lastKnown()` caches
* whatever the provider last resolved from the vela session on disk, so a
* daemon-only process that has never served a browser still has it (verified
* end to end with `od daemon start --headless`).
*/
export type AmbientWorkspaceSnapshot = {
workspaceId: string;
workspaceType: 'personal' | 'team';
workspaceMemberId: string;
role: WorkspaceResourceContext['role'];
memberStatus: WorkspaceResourceContext['memberStatus'];
lifecycleState: WorkspaceResourceContext['lifecycleState'];
permissions: { canShareProjects: boolean; canWriteSyncedFiles: boolean };
};

export type GetAmbientWorkspace = () => AmbientWorkspaceSnapshot | null | undefined;

/**
* The daemon's ambient identity as a resource context, or null when it has none.
*
* `workspaceTypeAsserted` is null and `appUserId` empty on purpose: both record
* what a CALLER claimed, and nobody claimed anything here.
*/
export function ambientWorkspaceResourceContext(
getAmbientWorkspace: GetAmbientWorkspace | undefined,
): WorkspaceResourceContext | null {
const ambient = getAmbientWorkspace?.();
if (!ambient) return null;
const workspaceId = ambient.workspaceId?.trim();
const workspaceMemberId = ambient.workspaceMemberId?.trim();
if (!workspaceId || !workspaceMemberId) return null;
return {
workspaceId,
workspaceType: ambient.workspaceType === 'team' ? 'team' : 'personal',
workspaceTypeAsserted: null,
appUserId: '',
workspaceMemberId,
role: ambient.role,
memberStatus: ambient.memberStatus,
lifecycleState: ambient.lifecycleState,
canShareProjects: ambient.permissions.canShareProjects,
canWriteSyncedFiles: ambient.permissions.canWriteSyncedFiles,
};
}

/**
* Cross-check the client-asserted `memberStatus` against the daemon's own
* last-known-good workspace context.
Expand Down Expand Up @@ -326,6 +380,84 @@ export function requestCanMutateWorkspaceResource(
return workspaceResourceAccess(row, ctx).canMutate;
}

/**
* Decide a mutation whose request carries NO workspace identity at all.
*
* INVARIANT: every mutation resolves to a workspace identity. A request that
* ASSERTS one is judged on that assertion; a request that asserts NOTHING is the
* local daemon's own signed-in user, and is judged as that identity. Being
* unable to name a workspace is not the same as having no standing in one.
*
* Headerless is the `od` CLI's normal shape, not an anomaly: nothing in
* `apps/daemon/src/cli.ts` attaches `x-od-workspace-*` outside `od workspace …`,
* and `AGENTS.md` makes the CLI the embeddability contract that external agents
* drive Open Design through. This branch used to answer 401 for ANY bound
* resource, which was survivable only while headerless creates left projects
* unbound. Once every created project got a workspace home (#6201), the two
* rules combined into a project its own creator could not touch:
* `od project create` then `od project duplicate` -> 401.
*
* Resolving to the daemon's ambient identity — rather than to the request's
* claim, of which there is none — is the same fallback the create path already
* applies ("nothing asserted -> ambient"), so the gate and the creation paths
* now agree about what a headerless caller is. It does NOT weaken the two
* contracts that look adjacent: `authorizeCreatedProjectWorkspace` still refuses
* to let ambient stand in for a pair someone explicitly CLAIMED, and
* `resolveProjectWorkspaceScope` still resolves a PERSISTED binding without
* consulting ambient. Both govern cases where something was asserted; this is
* the third case.
*
* What stays refused, because the original branch protected something real
* (recvqbeDjAsejl / recvqbklNGDqYY, spec 04 §10):
*
* - a resource bound to a workspace the daemon is NOT currently in — a
* teammate's shared project, or one left behind by a previous identity. A
* headerless caller has no standing there and still gets 401.
* - a resource in the daemon's own workspace that the daemon's own identity
* may not mutate anyway. The SAME `workspaceResourceMutationAllowed`
* computation runs, so a plain member still cannot rename a teammate's
* project that happens to be shared into this workspace.
* - everything, when the daemon has no signed-in identity to resolve. Nothing
* can vouch for the caller, so the pre-existing answer stands.
*
* An unbound resource stays allowed, exactly as before.
*/
function headerlessMutationAllowed(
resourceType: string,
res: Response,
sendApiError: (res: Response, status: number, code: string, message: string) => unknown,
getWorkspaceResource: (db: unknown, workspaceId: string, resourceId: string) => WorkspaceResourceAccessInput | null | undefined,
getWorkspaceResourceByResourceId: (db: unknown, resourceId: string) => WorkspaceResourceAccessInput | null | undefined,
db: unknown,
resourceId: string,
capability: WorkspaceResourceMutationCapability,
getAmbientWorkspace: GetAmbientWorkspace | undefined,
): boolean {
const anyRow = getWorkspaceResourceByResourceId(db, resourceId);
// Never bound anywhere: nothing to have standing in.
if (!anyRow) return true;

const ambient = ambientWorkspaceResourceContext(getAmbientWorkspace);
if (!ambient) {
sendApiError(res, 401, 'WORKSPACE_CONTEXT_REQUIRED', 'workspace context is required');
return false;
}
const ownRow = getWorkspaceResource(db, ambient.workspaceId, resourceId);
if (!ownRow) {
// Bound, but to some other workspace. This is the case the 401 exists for.
sendApiError(res, 401, 'WORKSPACE_CONTEXT_REQUIRED', 'workspace context is required');
return false;
}
if (!workspaceResourceMutationAllowed(ownRow, ambient, capability)) {
const code = isWorkspaceResourceLocked(ambient)
? 'WORKSPACE_LOCKED'
: `WORKSPACE_${resourceType.toUpperCase()}_PERMISSION_DENIED`;
sendApiError(res, 403, code, `workspace ${resourceType} mutation is not allowed`);
return false;
}
return true;
Comment thread
lefarcen marked this conversation as resolved.
Comment thread
lefarcen marked this conversation as resolved.
}

export function enforceWorkspaceResourceMutation(
resourceType: string,
req: any,
Expand All @@ -337,27 +469,21 @@ export function enforceWorkspaceResourceMutation(
resourceId: string,
capability: WorkspaceResourceMutationCapability,
getLastKnownMembership?: GetLastKnownWorkspaceMembership,
getAmbientWorkspace?: GetAmbientWorkspace,
): boolean {
const requestCtx = workspaceResourceContextFromRequest(req);
if (requestCtx === null) {
// No workspace headers at all — a legacy pre-workspace caller, or a
// client that just logged out (the frontend only attaches these headers
// while `workspaceContext` is non-null). Either way there is no identity
// to check against a team. That's fine for a resource this daemon has
// never bound to a workspace at all (`row` is null/undefined) — but ANY
// bound resource, personal OR team, requires proof of membership the
// request doesn't carry. A `personal` binding is not "safe to trust a
// headerless caller with" — it just means the resource isn't SHARED, not
// that it isn't OWNED; a signed-out / different-workspace caller has no
// more standing over someone else's personal draft than over a team
// resource (recvqbeDjAsejl / recvqbklNGDqYY, spec 04 §10). Treat both the
// same as `'missing'` rather than granting the mutation.
const row = getWorkspaceResourceByResourceId(db, resourceId);
if (row) {
sendApiError(res, 401, 'WORKSPACE_CONTEXT_REQUIRED', 'workspace context is required');
return false;
}
return true;
return headerlessMutationAllowed(
resourceType,
res,
sendApiError,
getWorkspaceResource,
getWorkspaceResourceByResourceId,
db,
resourceId,
capability,
getAmbientWorkspace,
);
}
if (requestCtx === 'missing') {
sendApiError(res, 401, 'WORKSPACE_CONTEXT_REQUIRED', 'workspace context is required');
Expand Down
45 changes: 42 additions & 3 deletions apps/daemon/src/routes/project/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ import {
type WorkspaceResourceMutationCapability,
} from '../../collab/workspace-resource-mutation.js';
import type { WorkspaceContextProvider } from '../../collab/workspace-context.js';
import { ambientWorkspaceResourceContext } from '../../collab/workspace-resource-mutation.js';
import { resolveProjectWorkspaceScopeForCaller } from '../../collab/project-workspace-scope.js';
import {
authorizeCreatedProjectWorkspace,
Expand Down Expand Up @@ -292,6 +293,9 @@ export function createEnforceWorkspaceProjectMutation(
workspaceContext: Pick<WorkspaceContextProvider, 'lastKnown'> | undefined,
) {
const getLastKnownWorkspaceMembership = lastKnownWorkspaceMembership(workspaceContext);
/** The daemon's own signed-in identity, for a request that asserts none —
* see `headerlessMutationAllowed` in collab/workspace-resource-mutation.ts. */
const getAmbientWorkspace: GetAmbientWorkspace = () => workspaceContext?.lastKnown?.() ?? null;
return function enforceWorkspaceProjectMutation(
req: any,
res: Response,
Expand All @@ -313,6 +317,7 @@ export function createEnforceWorkspaceProjectMutation(
projectId,
capability,
getLastKnownWorkspaceMembership,
getAmbientWorkspace,
);
};
}
Expand Down Expand Up @@ -1615,6 +1620,27 @@ export function registerProjectRoutes(app: Express, ctx: RegisterProjectRoutesDe
* rather than becoming an orphan.
*/
const getAmbientWorkspace: GetAmbientWorkspace = () => ctx.workspaceContext?.lastKnown?.() ?? null;
/**
* The workspace identity a mutation on this request ACTUALLY acts under.
*
* INVARIANT: this is the same resolution `enforceWorkspaceProjectMutation` used
* to allow the request — the caller's asserted identity when it has one, the
* daemon's own signed-in identity when it does not. Any SIDE EFFECT a mutation
* performs must read the context from here rather than re-deriving it from
* headers, or the effect silently disagrees with the gate that permitted it.
*
* That disagreement was a real cross-client data-consistency bug: `DELETE
* /api/projects/:id` re-derived `workspaceProjectContextFromRequest(req)`, which
* is null for a headerless caller, so once headerless mutation was allowed the
* hub unpublish and catalog removal were skipped while the local delete still
* ran — teammates kept seeing a project whose owner had already destroyed it.
*/
function effectiveWorkspaceProjectContext(req: any): WorkspaceProjectContext | null {
const asserted = workspaceProjectContextFromRequest(req);
if (asserted === 'missing') return null;
if (asserted !== null) return asserted;
return ambientWorkspaceResourceContext(getAmbientWorkspace);
}
/**
* Where a created project belongs when the request has no authorization gate
* of its own — the duplicate / design-system-copy pair and the
Expand Down Expand Up @@ -3730,10 +3756,23 @@ export function registerProjectRoutes(app: Express, ctx: RegisterProjectRoutesDe
// whole project is about to stop existing regardless.
const workspaceRow = getWorkspaceProjectByProjectId(db, project.id);
if (workspaceRow?.visibility === 'team') {
const teamCtx = workspaceProjectContextFromRequest(req);
if (teamCtx && teamCtx !== 'missing') {
await requestTeamVisibility([project.id], teamCtx, 'personal');
// Same context the gate above allowed this delete under — NOT a fresh
// header read, which is null for a headerless caller and would skip the
// hub work while still deleting locally.
const teamCtx = effectiveWorkspaceProjectContext(req);
if (!teamCtx) {
// Unreachable while the gate is intact: it admits a team-bound row only
// for an asserted identity or a matching ambient one. Refuse rather than
// fall through, so a future gate change cannot quietly reintroduce a
// local-only delete of a still-shared project.
return sendApiError(
res,
401,
'WORKSPACE_CONTEXT_REQUIRED',
'workspace context is required to unshare this project before deleting it',
);
}
await requestTeamVisibility([project.id], teamCtx, 'personal');
}
// Stop any live agent run in this project before its row and directory
// are removed, otherwise the CLI subprocess is orphaned — it keeps
Expand Down
Loading
Loading