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
20 changes: 17 additions & 3 deletions apps/api/src/feed-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,22 @@ export async function findLatestRepoScreenshots(
return page.items;
}

/**
* Whether a live link in this workspace can serve its files: the active
* storage lane (the record's own fields) has a public base URL, without which
* `publicUrl` in @uploads/storage is null. A live link is public, so creating
* one without it would leave a link whose page can only fail.
*/
export function liveLinksServable(workspace: WorkspaceRecord): boolean {
return Boolean(workspace.publicBaseUrl);
}

export function feedNotPublicError(): ServiceUnavailableError {
return new ServiceUnavailableError("Feed object is not publicly served.", {
code: "feed_object_not_public",
});
}

async function mapBounded<T, R>(
values: T[],
concurrency: number,
Expand Down Expand Up @@ -258,9 +274,7 @@ export async function hydrateFeedItems(
? objectPublicUrls(env, itemConfig, match.key)
: { url: null, embedUrl: null };
if (opts.requirePublicUrls !== false && meta && !withheld && urls.url === null)
throw new ServiceUnavailableError("Feed object is not publicly served.", {
code: "feed_object_not_public",
});
throw feedNotPublicError();
const dates = meta && !withheld ? publicObjectDateFields(meta) : {};
const id = await feedItemIdFor(match.key);
return {
Expand Down
14 changes: 14 additions & 0 deletions apps/api/src/github-comment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,20 @@ describe("gatherCommentBody live link (spec: PR comment links to the live link)"
);
});

it("creates no live link on a workspace with no public base URL", async () => {
const { env, ws, workspaceName, bucket } = makeTestEnv();
await putTagged(env, bucket, "gh/acme/web/pull/12/a.png", PR12);
const result = await gatherCommentBody(
env,
{ ...ws, name: workspaceName, publicBaseUrl: undefined },
workspaceName,
target,
);
expect(await findFeedByScope(env.DB, workspaceName, "acme/web", "", 12)).toBeNull();
expect(result.body).not.toContain("View all on uploads.sh");
expect(result.body).not.toContain("/c/");
});

it("omits the line when githubCommentLinkToFilePage is false", async () => {
const { env, ws, workspaceName, bucket } = makeTestEnv();
await putTagged(env, bucket, "gh/acme/web/pull/12/a.png", PR12);
Expand Down
17 changes: 14 additions & 3 deletions apps/api/src/github-comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

import { dbFor } from "./db-session";
import { feedItemIdFor, isInFeedScope, type FeedScope } from "@uploads/comment-render/scope";
import { feedItemUrl, feedUrl } from "./feed-service";
import { feedItemUrl, feedUrl, liveLinksServable } from "./feed-service";
import { createFeed } from "./feeds";
import { listObjects } from "./files-core";
import { getMetadataForKeys } from "./file-metadata";
Expand Down Expand Up @@ -232,7 +232,15 @@ async function gatherAttachments(

const liveLinkUrl =
items.length > 0
? await applyPrFeedPageUrls(env, workspaceName, target, items, scopeMetaByKey, linkToFilePage)
? await applyPrFeedPageUrls(
env,
ws,
workspaceName,
target,
items,
scopeMetaByKey,
linkToFilePage,
)
: null;

if (!showMetadata || items.length === 0) return { items, liveLinkUrl };
Expand Down Expand Up @@ -325,17 +333,20 @@ async function resolvePosterUrl(
* predicate as the scope query — and the id is hashed directly, with no
* 50-row page to fall off. Returns the live link URL when `linkToFilePage`
* is on, else null. Failures never fail the comment: `/f/` (or the raw URL)
* stays and the line is omitted.
* stays and the line is omitted. A workspace with no public base URL gets no
* live link at all, since its page could not serve the files.
*/
async function applyPrFeedPageUrls(
env: Env,
ws: WorkspaceRecord,
workspaceName: string,
target: GhTarget,
items: AttachmentItem[],
scopeMetaByKey: ReadonlyMap<string, Record<string, string>>,
linkToFilePage: boolean,
): Promise<string | null> {
try {
if (!liveLinksServable(ws)) return null;
// Comment-sync rows are uncapped and keep `comment` for life. GhTarget
// spells issues `issues`; createFeed takes `pr`/`issue`, and the slot
// lookup ignores kind, so a later web/CLI create for the same number
Expand Down
4 changes: 4 additions & 0 deletions apps/api/src/routes/feeds.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ import { createFeed, getFeed, listFeeds, softDeleteFeed } from "../feeds";
import {
decodeFeedCursor,
encodeFeedCursor,
feedNotPublicError,
feedSummary,
hydrateOwnerFeed,
liveLinksServable,
unwrapFeedMutation,
} from "../feed-service";
import { writeRateLimit } from "../guards";
Expand All @@ -23,6 +25,8 @@ async function ownerFeed(c: Context<WorkspaceVars>, id: string) {

export async function createFeedHandler(c: Context<WorkspaceVars>) {
const body = await jsonBody(c);
// Checked before the insert, so a refused create leaves no row behind.
if (!liveLinksServable(c.get("workspace"))) throw feedNotPublicError();
const result = unwrapFeedMutation(
await createFeed(dbFor(c.env), {
workspace: c.get("workspaceName"),
Expand Down
23 changes: 23 additions & 0 deletions apps/api/test/routes-feeds.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,14 @@ beforeEach(async () => {
publicBaseUrl: "https://storage.uploads.sh",
tokenHash: await sha256Hex(TOKEN),
},
// No public base URL: objects are reachable only through signed URLs.
gamma: {
provider: "r2",
bucket: "shared",
binding: "UPLOADS_DEFAULT",
prefix: "gamma/",
tokenHash: await sha256Hex(TOKEN),
},
};
env = {
DB: sqlite as unknown as D1Database,
Expand Down Expand Up @@ -427,6 +435,21 @@ describe("feed routes", () => {
expect(((await res.json()) as { source: string | null }).source).toBe("user");
});

it("refuses a live link on a workspace with no public base URL and saves no row", async () => {
// Both an empty scope and one with files: a live link could serve neither.
await putShot("gamma", "gh/acme/app/pull/1/a.png", { "gh.repo": "acme/app" });
for (const body of [{ repo: "acme/app", pr: 1 }, { repo: "acme/empty" }]) {
const created = await request("/v1/workspaces/gamma/feeds", {
method: "POST",
body: JSON.stringify(body),
});
expect(created.status).toBe(503);
expect(await created.json()).toMatchObject({ error: { code: "feed_object_not_public" } });
}
const list = await request("/v1/workspaces/gamma/feeds");
expect(((await list.json()) as { feeds: unknown[] }).feeds).toEqual([]);
});

it("creates 60 PR-scoped feeds over the API and caps only repo-scoped feeds at 50", async () => {
for (let n = 1; n <= 60; n++) {
const created = await request("/v1/workspaces/alpha/feeds", {
Expand Down
Loading