v1.3.0

* Add tracing around unfurl calls

* fix: Prevent thundering-herd unfurls

* tracer -> traceFunction

LICENSE

@@ -3,7 +3,7 @@ Business Source License 1.1
 Parameters
 
 Licensor:             General Outline, Inc.
-Licensed Work:        Outline 1.2.0
+Licensed Work:        Outline 1.3.0
                       The Licensed Work is (c) 2026 General Outline, Inc.
 Additional Use Grant: You may make use of the Licensed Work, provided that
                       you may not use the Licensed Work for a Document
@@ -15,7 +15,7 @@ Additional Use Grant: You may make use of the Licensed Work, provided that
                       Licensed Work by creating teams and documents
                       controlled by such third parties.
 
-Change Date:          2030-01-06
+Change Date:          2030-01-17
 
 Change License:       Apache License, Version 2.0
 

app/actions/definitions/documents.tsx

@@ -38,6 +38,7 @@ import Icon from "@shared/components/Icon";
 import type { NavigationNode } from "@shared/types";
 import { ExportContentType, TeamPreference } from "@shared/types";
 import { getEventFiles } from "@shared/utils/files";
+import { Week } from "@shared/utils/time";
 import type UserMembership from "~/models/UserMembership";
 import { client } from "~/utils/ApiClient";
 import DocumentDelete from "~/scenes/DocumentDelete";
@@ -630,7 +631,7 @@ export const copyDocumentAsMarkdown = createAction({
     if (document) {
       const res = await client.post("/documents.export", {
         id: document.id,
-        signedUrls: 3600 * 24 * 30, // 30 days
+        signedUrls: Week.seconds, // 7 days (AWS S3 max for presigned URLs)
       });
       copy(res.data);
       toast.success(t("Markdown copied to clipboard"));

package.json

@@ -390,6 +390,6 @@
     "prismjs": "1.30.0",
     "cheerio": "1.0.0-rc.12"
   },
-  "version": "1.2.0",
+  "version": "1.3.0",
   "packageManager": "yarn@4.11.0"
 }

server/routes/api/urls/urls.ts

@@ -1,5 +1,6 @@
 import dns from "dns";
 import Router from "koa-router";
+import { traceFunction } from "@server/logging/tracing";
 import { MentionType, UnfurlResourceType } from "@shared/types";
 import { getBaseDomain, parseDomain } from "@shared/utils/domains";
 import parseDocumentSlug from "@shared/utils/parseDocumentSlug";
@@ -13,7 +14,7 @@ import { Document, Share, Team, User, Group, GroupUser } from "@server/models";
 import { authorize, can } from "@server/policies";
 import presentUnfurl from "@server/presenters/unfurl";
 import type { APIContext, Unfurl } from "@server/types";
-import { CacheHelper } from "@server/utils/CacheHelper";
+import { CacheHelper, type CacheResult } from "@server/utils/CacheHelper";
 import { Hook, PluginManager } from "@server/utils/PluginManager";
 import { RateLimiterStrategy } from "@server/utils/RateLimiter";
 import * as T from "./schema";
@@ -121,35 +122,53 @@ router.post(
         });
         return;
       }
-      return (ctx.response.status = 204);
+      ctx.response.status = 204;
+      return;
     }
 
     // External resources
-    const cachedData = await CacheHelper.getData<Unfurl>(
-      CacheHelper.getUnfurlKey(actor.teamId, url)
-    );
-    if (cachedData) {
-      return (ctx.body = await presentUnfurl(cachedData));
-    }
+    // Use getDataOrSet which handles distributed locking to prevent thundering herd
+    // when multiple clients request the same URL simultaneously
+    const cacheKey = CacheHelper.getUnfurlKey(actor.teamId, url);
+    const defaultCacheExpiry = 3600;
 
-    for (const plugin of plugins) {
-      const unfurl = await plugin.value.unfurl(url, actor);
-      if (unfurl) {
-        if ("error" in unfurl) {
-          return (ctx.response.status = 204);
-        } else {
-          const data = unfurl as Unfurl;
-          await CacheHelper.setData(
-            CacheHelper.getUnfurlKey(actor.teamId, url),
-            data,
-            plugin.value.cacheExpiry
-          );
-          return (ctx.body = await presentUnfurl(data));
+    const unfurlResult = await CacheHelper.getDataOrSet<
+      Unfurl | { error: true }
+    >(
+      cacheKey,
+      async (): Promise<CacheResult<Unfurl | { error: true }> | undefined> => {
+        for (const plugin of plugins) {
+          const pluginName = plugin.name ?? "unknown";
+          const unfurl = await traceFunction({
+            spanName: "unfurl.plugin",
+            resourceName: pluginName,
+            tags: {
+              "unfurl.plugin": pluginName,
+              "unfurl.url_host": urlObj.hostname,
+            },
+          })(() => plugin.value.unfurl(url, actor))();
+          if (unfurl) {
+            if ("error" in unfurl) {
+              return { data: { error: true as const }, expiry: 60 };
+            }
+            return {
+              data: unfurl as Unfurl,
+              expiry: plugin.value.cacheExpiry,
+            };
+          }
         }
-      }
+        return undefined;
+      },
+      defaultCacheExpiry
+    );
+
+    if (!unfurlResult || "error" in unfurlResult) {
+      ctx.response.status = 204;
+      return;
     }
 
-    return (ctx.response.status = 204);
+    ctx.body = await presentUnfurl(unfurlResult);
+    return;
   }
 );
 

server/storage/files/BaseStorage.ts

@@ -4,6 +4,7 @@ import type { PresignedPost } from "@aws-sdk/s3-presigned-post";
 import omit from "lodash/omit";
 import FileHelper from "@shared/editor/lib/FileHelper";
 import { isBase64Url, isInternalUrl } from "@shared/utils/urls";
+import { Week } from "@shared/utils/time";
 import env from "@server/env";
 import Logger from "@server/logging/Logger";
 import type { RequestInit } from "@server/utils/fetch";
@@ -14,6 +15,12 @@ export default abstract class BaseStorage {
   /** The default number of seconds until a signed URL expires. */
   public static defaultSignedUrlExpires = 300;
 
+  /**
+   * The maximum number of seconds until a signed URL expires for S3 Signature V4.
+   * AWS S3 Signature V4 presigned URLs must have an expiration date less than one week in the future.
+   */
+  public static maxSignedUrlExpires = Week.seconds;
+
   /**
    * Returns a presigned post for uploading files to the storage provider.
    *

server/storage/files/S3Storage.test.ts

@@ -0,0 +1,40 @@
+import { Week, Day } from "@shared/utils/time";
+import BaseStorage from "./BaseStorage";
+
+describe("S3Storage", () => {
+  describe("getSignedUrl expiration limits", () => {
+    it("should define maximum expiration as 7 days for AWS S3 Signature V4", () => {
+      // AWS S3 Signature V4 presigned URLs have a maximum expiration of 7 days
+      const maxExpiration = Week.seconds;
+
+      // Verify our constant matches AWS limit
+      expect(BaseStorage.maxSignedUrlExpires).toBe(maxExpiration);
+      expect(BaseStorage.maxSignedUrlExpires).toBe(604800); // 7 days in seconds
+    });
+
+    it("should have Week.seconds equal to 7 days", () => {
+      expect(Week.seconds).toBe(7 * 24 * 60 * 60);
+      expect(Week.seconds).toBe(604800);
+    });
+
+    it("should ensure 30 days exceeds the limit", () => {
+      const thirtyDays = 30 * Day.seconds;
+      expect(thirtyDays).toBeGreaterThan(BaseStorage.maxSignedUrlExpires);
+      expect(thirtyDays).toBe(2592000); // 30 days in seconds
+    });
+
+    it("should ensure 4 days is within the limit", () => {
+      const fourDays = 4 * Day.seconds;
+      expect(fourDays).toBeLessThan(BaseStorage.maxSignedUrlExpires);
+      expect(fourDays).toBe(345600); // 4 days in seconds
+    });
+
+    it("should clamp values that exceed the limit", () => {
+      const thirtyDays = 30 * Day.seconds;
+      const clampedValue = Math.min(thirtyDays, BaseStorage.maxSignedUrlExpires);
+      
+      expect(clampedValue).toBe(BaseStorage.maxSignedUrlExpires);
+      expect(clampedValue).toBe(Week.seconds);
+    });
+  });
+});

server/storage/files/S3Storage.ts

@@ -152,8 +152,11 @@ export default class S3Storage extends BaseStorage {
     if (isDocker) {
       return `${this.getPublicEndpoint()}/${key}`;
     } else {
+      // Ensure expiration does not exceed AWS S3 Signature V4 limit of 7 days
+      const clampedExpiresIn = Math.min(expiresIn, S3Storage.maxSignedUrlExpires);
+
       const command = new GetObjectCommand(params);
-      const url = await getSignedUrl(this.client, command, { expiresIn });
+      const url = await getSignedUrl(this.client, command, { expiresIn: clampedExpiresIn });
 
       if (env.AWS_S3_ACCELERATE_URL) {
         return url.replace(

server/utils/CacheHelper.ts

@@ -3,6 +3,16 @@ import Logger from "@server/logging/Logger";
 import Redis from "@server/storage/redis";
 import { MutexLock } from "./MutexLock";
 
+/**
+ * Result type for cache callbacks that need to specify a dynamic expiry.
+ */
+export interface CacheResult<T> {
+  /** The data to cache. */
+  data: T;
+  /** Cache expiry in seconds. If not provided, uses the default expiry passed to getDataOrSet. */
+  expiry?: number;
+}
+
 /**
  * A Helper class for server-side cache management
  */
@@ -15,15 +25,19 @@ export class CacheHelper {
    * If data is not found, it will call the callback to get the data and save it in cache
    * using a distributed lock to prevent multiple writes.
    *
+   * The callback can return either:
+   * - A plain value of type T (uses the default expiry)
+   * - A CacheResult<T> object with { data, expiry } for dynamic expiry
+   *
    * @param key Cache key
    * @param callback Callback to get the data if not found in cache
-   * @param expiry Cache data expiry in seconds
+   * @param expiry Default cache data expiry in seconds
    * @param lockTimeout Lock timeout in milliseconds
    * @returns The data from cache or the result of the callback
    */
   public static async getDataOrSet<T>(
     key: string,
-    callback: () => Promise<T | undefined>,
+    callback: () => Promise<T | CacheResult<T> | undefined>,
     expiry: number,
     lockTimeout: number = MutexLock.defaultLockTimeout
   ): Promise<T | undefined> {
@@ -48,11 +62,25 @@ export class CacheHelper {
       }
 
       // Get the data from the callback and save it in cache
-      const value = await callback();
-      if (value) {
-        await this.setData<T>(key, value, expiry);
+      const result = await callback();
+      if (result) {
+        // Check if result is a CacheResult with dynamic expiry
+        const isCacheResult =
+          typeof result === "object" &&
+          result !== null &&
+          "data" in result &&
+          Object.keys(result).every((k) => k === "data" || k === "expiry");
+
+        if (isCacheResult) {
+          const { data, expiry: dynamicExpiry } = result as CacheResult<T>;
+          await this.setData<T>(key, data, dynamicExpiry ?? expiry);
+          return data;
+        }
+
+        await this.setData<T>(key, result as T, expiry);
+        return result as T;
       }
-      return value;
+      return undefined;
     } finally {
       if (lock) {
         await MutexLock.release(lock);

server/utils/__mocks__/CacheHelper.ts

@@ -1,4 +1,5 @@
 import { Day } from "@shared/utils/time";
+import type { CacheResult } from "../CacheHelper";
 
 /**
  * A Mock Helper class for server-side cache management
@@ -12,11 +13,15 @@ export class CacheHelper {
    */
   public static async getDataOrSet<T>(
     key: string,
-    callback: () => Promise<T | undefined>,
+    callback: () => Promise<T | CacheResult<T> | undefined>,
     _expiry: number,
-    _lockTimeout: number
+    _lockTimeout?: number
   ): Promise<T | undefined> {
-    return await callback();
+    const result = await callback();
+    if (result && typeof result === "object" && "data" in result) {
+      return (result as CacheResult<T>).data;
+    }
+    return result as T | undefined;
   }
 
   /**

shared/utils/time.ts

@@ -27,3 +27,14 @@ export class Day {
   /** Minutes in a day */
   public static minutes = 24 * Hour.minutes;
 }
+
+export class Week {
+  /** Milliseconds in a week */
+  public static ms = 7 * Day.ms;
+  /** Seconds in a week */
+  public static seconds = 7 * Day.seconds;
+  /** Minutes in a week */
+  public static minutes = 7 * Day.minutes;
+  /** Days in a week */
+  public static days = 7;
+}