Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
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
2 changes: 1 addition & 1 deletion packages/cli/src/commands/assets/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ export const fetchSharedAssetFolders = async ({
}
return { asset_folders: all.filter((folder) => keep.has(folder.id)) };
} catch (maybeError) {
handleAPIError("pull_shared_asset_folders", toError(maybeError));
handleAPIError("list_shared_asset_folders", toError(maybeError));
}
};

Expand Down
86 changes: 86 additions & 0 deletions packages/cli/src/commands/assets/pull/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,26 @@ const preconditions = {
),
);
},
forbidsLibraryDiscovery() {
server.use(
http.get("https://mapi.storyblok.com/v1/spaces/12345/shared_asset_folders", () =>
HttpResponse.json(
{ error: "This endpoint does not support this token type" },
{ status: 403 },
),
),
);
},
forbidsLibraryDiscoveryForAnotherReason() {
server.use(
http.get("https://mapi.storyblok.com/v1/spaces/12345/shared_asset_folders", () =>
HttpResponse.json(
{ error: "This token is restricted to specific spaces" },
{ status: 403 },
),
),
);
},
hasLocalStoriesReferencing(assetIds: number[]) {
vol.fromJSON({
".storyblok/stories/12345/home_uuid.json": JSON.stringify({
Expand Down Expand Up @@ -589,6 +609,72 @@ describe("assets pull command", () => {
expect(sharedSpy).not.toHaveBeenCalled();
});

it("should pull space assets and warn when referenced-library discovery is forbidden", async () => {
const spaceAsset = makeMockAsset({ id: 42 });
preconditions.hasLocalStoriesReferencing([42, 90]);
preconditions.canFetchRemoteFolders([]);
preconditions.canFetchRemoteAssetPages([[spaceAsset]]);
preconditions.canDownloadAssets([spaceAsset]);
preconditions.sharedAssetResolves({
id: 90,
filename: "https://a.storyblok.com/g/1/x.png",
asset_folder_id: 7,
});
preconditions.forbidsLibraryDiscovery();

await assetsCommand.parseAsync(["node", "test", "pull", "--space", "12345"]);

expect(assetFileExists(spaceAsset)).toBeTruthy();
expect(console.warn).toHaveBeenCalledWith(
expect.stringContaining("Shared libraries are unavailable"),
);
expect(process.exitCode).toBe(0);
});

it("should fail rather than degrade when referenced-library discovery fails for a non-token-type reason", async () => {
const spaceAsset = makeMockAsset({ id: 42 });
preconditions.hasLocalStoriesReferencing([42, 90]);
preconditions.canFetchRemoteFolders([]);
preconditions.canFetchRemoteAssetPages([[spaceAsset]]);
preconditions.canDownloadAssets([spaceAsset]);
preconditions.sharedAssetResolves({
id: 90,
filename: "https://a.storyblok.com/g/1/x.png",
asset_folder_id: 7,
});
preconditions.forbidsLibraryDiscoveryForAnotherReason();

await assetsCommand.parseAsync(["node", "test", "pull", "--space", "12345"]);

expect(console.warn).not.toHaveBeenCalledWith(
expect.stringContaining("Shared libraries are unavailable"),
);
expect(process.exitCode).toBe(1);
});

it("should fail when libraries were explicitly requested but are forbidden", async () => {
preconditions.forbidsLibraryDiscovery();

await assetsCommand.parseAsync([
"node",
"test",
"pull",
"--space",
"12345",
"--target",
"shared",
]);

expect(
(console.error as ReturnType<typeof vi.fn>).mock.calls
.flat()
.some(
(arg) => typeof arg === "string" && arg.includes("This command is not available with"),
),
).toBe(true);
expect(process.exitCode).toBe(1);
});

it("rejects an invalid --target value", async () => {
const pull = assetsCommand.commands.find((command) => command.name() === "pull")!;
pull.exitOverride();
Expand Down
41 changes: 32 additions & 9 deletions packages/cli/src/commands/assets/pull/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import { resolveCommandPath } from "../../../utils/filesystem";
import { getLogger } from "../../../lib/logger/logger";
import { getReporter } from "../../../lib/reporter/reporter";
import { requireAuthentication } from "../../../utils/auth";
import { APIError } from "../../../utils/error/api-error";
import { CommandError } from "../../../utils/error/command-error";
import { isUnsupportedTokenTypeServerError } from "../../../utils/error/credential-hint";
import { handleError, logOnlyError, toError } from "../../../utils/error/error";
import {
downloadAssetStream,
Expand Down Expand Up @@ -350,16 +352,37 @@ pullCmd.action(async (options, command) => {
const sharedAssets = resolved.filter((asset): asset is Asset => Boolean(asset?.id));

if (sharedAssets.length > 0) {
const resolveRoot = await buildLibraryRootResolver(space);
const byLibrary = new Map<number, Asset[]>();
for (const asset of sharedAssets) {
const libraryId = resolveRoot(asset.asset_folder_id ?? 0);
const bucket = byLibrary.get(libraryId) ?? [];
bucket.push(asset);
byLibrary.set(libraryId, bucket);
// `with-referenced` is implicit: the user asked to pull assets, not libraries. A
// credential that cannot reach library discovery degrades to the space-only pull
// with a warning rather than failing the whole command. An explicit `--target
// shared`/`--target all` still fails via `listReadableLibraries`.
let resolveRoot: ((assetFolderId: number) => number) | undefined;
try {
resolveRoot = await buildLibraryRootResolver(space);
} catch (error) {
if (
error instanceof APIError &&
error.code === 403 &&
isUnsupportedTokenTypeServerError(error.serverError)
) {
getUI().warn(
"Shared libraries are unavailable with this login; skipping referenced library assets.",
);
} else {
throw error;
}
}
for (const [libraryId, assets] of byLibrary) {
await pullReferencedAssets(libraryId, assets);
if (resolveRoot) {
const byLibrary = new Map<number, Asset[]>();
for (const asset of sharedAssets) {
const libraryId = resolveRoot(asset.asset_folder_id ?? 0);
const bucket = byLibrary.get(libraryId) ?? [];
bucket.push(asset);
byLibrary.set(libraryId, bucket);
}
for (const [libraryId, assets] of byLibrary) {
await pullReferencedAssets(libraryId, assets);
}
}
}
}
Expand Down
48 changes: 48 additions & 0 deletions packages/cli/src/commands/assets/push/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,16 @@ const preconditions = {
[filePath]: content,
});
},
forbidsLibraryDiscovery({ space = DEFAULT_SPACE }: { space?: string } = {}) {
server.use(
http.get(`https://mapi.storyblok.com/v1/spaces/${space}/shared_asset_folders`, () =>
HttpResponse.json(
{ error: "This endpoint does not support this token type" },
{ status: 403 },
),
),
);
},
hasLibraries(
libraries: { id: number; name: string; accessLevel: "read" | "write" }[],
{ space = DEFAULT_SPACE }: { space?: string } = {},
Expand Down Expand Up @@ -2570,5 +2580,43 @@ describe("assets push command", () => {
stderrCalls.some((msg: unknown) => typeof msg === "string" && msg.includes("ENOENT")),
).toBe(false);
});

it("should push space assets and warn when library discovery is forbidden", async () => {
preconditions.forbidsLibraryDiscovery();
const asset = makeMockAsset();
preconditions.canLoadFolders([]);
preconditions.canLoadAssets([asset]);
preconditions.canUpsertRemoteAssets([asset]);

await assetsCommand.parseAsync(["node", "test", "push", "--space", DEFAULT_SPACE]);

expect(console.warn).toHaveBeenCalledWith(
expect.stringContaining("Shared libraries are unavailable"),
);
expect(process.exitCode).toBe(0);
});

it("should fail when libraries were explicitly requested but are forbidden", async () => {
preconditions.forbidsLibraryDiscovery();

await assetsCommand.parseAsync([
"node",
"test",
"push",
"--space",
DEFAULT_SPACE,
"--target",
"shared",
]);

expect(
(console.error as ReturnType<typeof vi.fn>).mock.calls
.flat()
.some(
(arg) => typeof arg === "string" && arg.includes("This command is not available with"),
),
).toBe(true);
expect(process.exitCode).toBe(1);
});
});
});
29 changes: 21 additions & 8 deletions packages/cli/src/commands/assets/push/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ import { makeWriteStoryAPITransport } from "../../stories/streams";
import {
assertLibraryWritable,
listWritableLibraries,
listWritableLibrariesOrDegrade,
resolveScopeBaseDir,
type Scope,
} from "../scope";
Expand Down Expand Up @@ -211,14 +212,26 @@ pushCmd.action(async (assetInput, options, command) => {
scopes.push({ kind: "space", spaceId: fromSpace });
}
if (target === "shared" || target === "auto") {
// `listWritableLibraries` already filters to writable libraries, so
// these scopes need no further access check.
const libraries = await listWritableLibraries(targetSpace);
scopes.push(
...libraries.map(
(library) => ({ kind: "library", libraryId: library.id }) satisfies Scope,
),
);
// `auto` is implicit: the user asked to push assets, not libraries. A credential that
// cannot reach library discovery degrades to the space scope with a warning rather
// than failing the whole command. An explicit `--target shared` still fails.
// `listWritableLibraries`/`listWritableLibrariesOrDegrade` already filter to writable
// libraries, so these scopes need no further access check.
const libraries =
target === "auto"
? await listWritableLibrariesOrDegrade(targetSpace)
: await listWritableLibraries(targetSpace);
if (libraries === undefined) {
getUI().warn(
"Shared libraries are unavailable with this login; continuing with space assets only.",
);
} else {
scopes.push(
...libraries.map(
(library) => ({ kind: "library", libraryId: library.id }) satisfies Scope,
),
);
}
}
}
} catch (maybeError) {
Expand Down
65 changes: 63 additions & 2 deletions packages/cli/src/commands/assets/scope.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,66 @@
import { describe, expect, it } from "vitest";
import { resolveScopeBaseDir, type Scope } from "./scope";
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from "vitest";
import { http, HttpResponse } from "msw";
import { setupServer } from "msw/node";
import { getMapiClient } from "../../api";
import { listLibrariesOrDegrade, resolveScopeBaseDir, type Scope } from "./scope";

const server = setupServer();
beforeAll(() => server.listen({ onUnhandledRequest: "error" }));
beforeEach(() => {
getMapiClient({ personalAccessToken: "valid-token", region: "eu" });
});
afterEach(() => server.resetHandlers());
afterAll(() => server.close());

const preconditions = {
forbidsLibraryDiscovery() {
server.use(
http.get("https://mapi.storyblok.com/v1/spaces/12345/shared_asset_folders", () =>
HttpResponse.json(
{ error: "This endpoint does not support this token type" },
{ status: 403 },
),
),
);
},
hasNoLibraries() {
server.use(
http.get("https://mapi.storyblok.com/v1/spaces/12345/shared_asset_folders", () =>
HttpResponse.json({ shared_asset_folders: [] }),
),
);
},
forbidsLibraryDiscoveryForAnotherReason() {
server.use(
http.get("https://mapi.storyblok.com/v1/spaces/12345/shared_asset_folders", () =>
HttpResponse.json(
{ error: "This token is restricted to specific spaces" },
{ status: 403 },
),
),
);
},
};

describe("listLibrariesOrDegrade", () => {
it("should return undefined when the credential cannot reach library discovery", async () => {
preconditions.forbidsLibraryDiscovery();

await expect(listLibrariesOrDegrade("12345")).resolves.toBeUndefined();
});

it("should return an empty list when the space simply has no libraries", async () => {
preconditions.hasNoLibraries();

await expect(listLibrariesOrDegrade("12345")).resolves.toEqual([]);
});

it("should still fail loudly for a different 403 from the same endpoint", async () => {
preconditions.forbidsLibraryDiscoveryForAnotherReason();

await expect(listLibrariesOrDegrade("12345")).rejects.toThrow();
});
});

describe("resolveScopeBaseDir", () => {
it("returns the space subtree for a space scope", () => {
Expand Down
Loading
Loading