Skip to content
Open
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
6 changes: 6 additions & 0 deletions .server-changes/validate-packet-storage-paths.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
area: webapp
type: fix
---

Validate packet-relative storage paths before building object-store keys or presigned URLs.
6 changes: 3 additions & 3 deletions apps/webapp/app/routes/api.v1.packets.$.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { json } from "@remix-run/server-runtime";
import { z } from "zod";
import { authenticateApiRequest } from "~/services/apiAuth.server";
import { createLoaderApiRoute } from "~/services/routeBuilders/apiBuilder.server";
import { generatePresignedUrl } from "~/v3/objectStore.server";
import { generatePresignedUrl, jsonPacketPresignFailure } from "~/v3/objectStore.server";

const ParamsSchema = z.object({
"*": z.string(),
Expand Down Expand Up @@ -34,7 +34,7 @@ export async function action({ request, params }: ActionFunctionArgs) {
);

if (!signed.success) {
return json({ error: `Failed to generate presigned URL: ${signed.error}` }, { status: 500 });
return jsonPacketPresignFailure(signed);
}

// Caller can now use this URL to upload to that object.
Expand All @@ -59,7 +59,7 @@ export const loader = createLoaderApiRoute(
);

if (!signed.success) {
return json({ error: `Failed to generate presigned URL: ${signed.error}` }, { status: 500 });
return jsonPacketPresignFailure(signed);
}

// Caller can now use this URL to fetch that object.
Expand Down
4 changes: 2 additions & 2 deletions apps/webapp/app/routes/api.v2.packets.$.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { ActionFunctionArgs } from "@remix-run/server-runtime";
import { json } from "@remix-run/server-runtime";
import { z } from "zod";
import { authenticateApiRequest } from "~/services/apiAuth.server";
import { generatePresignedUrl } from "~/v3/objectStore.server";
import { generatePresignedUrl, jsonPacketPresignFailure } from "~/v3/objectStore.server";

const ParamsSchema = z.object({
"*": z.string(),
Expand Down Expand Up @@ -34,7 +34,7 @@ export async function action({ request, params }: ActionFunctionArgs) {
);

if (!signed.success) {
return json({ error: `Failed to generate presigned URL: ${signed.error}` }, { status: 500 });
return jsonPacketPresignFailure(signed);
}

if (signed.storagePath === undefined) {
Expand Down
178 changes: 152 additions & 26 deletions apps/webapp/app/v3/objectStore.server.ts
Comment thread
kathiekiwi marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
import { json } from "@remix-run/server-runtime";
import { type IOPacket } from "@trigger.dev/core/v3";
import { env } from "~/env.server";
import { type AuthenticatedEnvironment } from "~/services/apiAuth.server";
import { logger } from "~/services/logger.server";
import { ServiceValidationError } from "~/v3/services/common.server";
import { singleton } from "~/utils/singleton";
import { ObjectStoreClient, type ObjectStoreClientConfig } from "./objectStoreClient.server";
import {
normalizeObjectStoreLogicalKeyPathname,
ObjectStoreClient,
type ObjectStoreClientConfig,
} from "./objectStoreClient.server";

/**
* Parsed storage URI with optional protocol prefix
Expand Down Expand Up @@ -47,6 +53,115 @@ export function formatStorageUri(path: string, protocol?: string): string {
return path;
}

export const INVALID_PACKET_STORAGE_PATH = "Invalid packet storage path";

export type PacketPresignFailure = {
success: false;
error: string;
status?: number;
};

const PACKET_RELATIVE_PATH_BASE = "/__packet_base__";

function throwInvalidPacketStoragePath(): never {
throw new ServiceValidationError(INVALID_PACKET_STORAGE_PATH, 400);
}

function assertRawPacketRelativePathSegments(path: string): void {
if (!path || path.includes("\\") || path.startsWith("/")) {
throwInvalidPacketStoragePath();
}

for (const segment of path.split("/")) {
if (segment === "" || segment === "." || segment === "..") {
throwInvalidPacketStoragePath();
}

if (segment.includes("%")) {
let decoded: string;
try {
decoded = decodeURIComponent(segment);
} catch {
throwInvalidPacketStoragePath();
}

if (decoded === "." || decoded === ".." || decoded.includes("/")) {
throwInvalidPacketStoragePath();
}
}
}
}

/**
* Normalize a packet-relative path using the same URL pathname resolution as object-store clients.
*/
export function normalizePacketRelativePath(path: string): string {
const url = new URL("https://trigger.invalid");
url.pathname = `${PACKET_RELATIVE_PATH_BASE}/${path.replace(/^\/+/, "")}`;

const prefix = `${PACKET_RELATIVE_PATH_BASE}/`;
if (!url.pathname.startsWith(prefix)) {
throwInvalidPacketStoragePath();
}

return url.pathname.slice(prefix.length);
}

/**
* Ensure a full logical object-store key resolves under the packet prefix after URL normalization.
*/
export function assertPacketObjectStoreKeyUnderPrefix(key: string, packetPrefix: string): void {
const normalizedKeyPath = normalizeObjectStoreLogicalKeyPathname(key);
const normalizedPrefixPath = normalizeObjectStoreLogicalKeyPathname(packetPrefix);

if (
normalizedKeyPath !== normalizedPrefixPath &&
!normalizedKeyPath.startsWith(`${normalizedPrefixPath}/`)
) {
throwInvalidPacketStoragePath();
}
}

/**
* Validate a packet-relative path and return the canonical form used for object-store keys.
*/
export function resolveSafePacketRelativePath(path: string): string {
assertRawPacketRelativePathSegments(path);
const normalized = normalizePacketRelativePath(path);
assertRawPacketRelativePathSegments(normalized);
return normalized;
}

/**
* Reject path traversal and other unsafe packet-relative storage paths before
* building object-store keys or presigned URLs.
*/
export function assertSafePacketRelativePath(path: string): void {
resolveSafePacketRelativePath(path);
}

function buildPacketObjectStoreKey(
projectRef: string,
envSlug: string,
relativePath: string
): string {
const safeRelativePath = resolveSafePacketRelativePath(relativePath);
const prefix = `packets/${projectRef}/${envSlug}`;
const key = `${prefix}/${safeRelativePath}`;
assertPacketObjectStoreKeyUnderPrefix(key, prefix);
return key;
}

/** JSON response for packet presign failures (400 client error vs 500 internal). */
export function jsonPacketPresignFailure(failure: PacketPresignFailure) {
const status = failure.status ?? 500;
if (status === 400) {
return json({ error: failure.error }, { status: 400 });
}

return json({ error: `Failed to generate presigned URL: ${failure.error}` }, { status: 500 });
}

/**
* Get object storage configuration for a given protocol.
* Returns a config if baseUrl is set, even without explicit credentials —
Expand Down Expand Up @@ -134,14 +249,20 @@ export async function uploadPacketToObjectStore(
throw new Error(`Object store is not configured for protocol: ${protocol || "default"}`);
}

const key = `packets/${environment.project.externalRef}/${environment.slug}/${filename}`;
const { path } = parseStorageUri(filename);
const safePath = resolveSafePacketRelativePath(path);
const key = buildPacketObjectStoreKey(
environment.project.externalRef,
environment.slug,
safePath
);

Comment thread
kathiekiwi marked this conversation as resolved.
logger.debug("Uploading to object store", { key, protocol: protocol || "default" });

await client.putObject(key, data, contentType);

// Return filename with protocol prefix if specified
return formatStorageUri(filename, protocol);
// Return canonical storage URI (path only in the key; protocol prefix applied here)
return formatStorageUri(safePath, protocol);
}

export async function downloadPacketFromObjectStore(
Expand All @@ -162,14 +283,18 @@ export async function downloadPacketFromObjectStore(
}

const { protocol, path } = parseStorageUri(packet.data);
const key = buildPacketObjectStoreKey(
environment.project.externalRef,
environment.slug,
path
);

const client = getObjectStoreClient(protocol);

if (!client) {
throw new Error(`Object store is not configured for protocol: ${protocol || "default"}`);
}

const key = `packets/${environment.project.externalRef}/${environment.slug}/${path}`;

logger.debug("Downloading from object store", { key, protocol: protocol || "default" });

const data = await client.getObject(key);
Expand Down Expand Up @@ -220,10 +345,7 @@ export async function generatePresignedRequest(
method: "PUT" | "GET" = "PUT",
options?: GeneratePacketPresignOptions
): Promise<
| {
success: false;
error: string;
}
| PacketPresignFailure
| {
success: true;
request: Request;
Expand All @@ -237,6 +359,21 @@ export async function generatePresignedRequest(
options?.forceNoPrefix
);

let safePath: string;
try {
safePath = resolveSafePacketRelativePath(path);
} catch (error) {
if (error instanceof ServiceValidationError) {
return {
success: false,
error: error.message,
status: error.status ?? 400,
};
}

throw error;
}

const config = getObjectStoreConfig(storeProtocol);
if (!config?.baseUrl) {
return {
Expand All @@ -253,7 +390,7 @@ export async function generatePresignedRequest(
};
}

const key = `packets/${projectRef}/${envSlug}/${path}`;
const key = buildPacketObjectStoreKey(projectRef, envSlug, safePath);

try {
const url = await client.presign(key, method, 300); // 5 minutes
Expand All @@ -266,7 +403,7 @@ export async function generatePresignedRequest(
protocol: storeProtocol || "default",
});

const storagePath = method === "PUT" ? formatStorageUri(path, storeProtocol) : undefined;
const storagePath = method === "PUT" ? formatStorageUri(safePath, storeProtocol) : undefined;

return {
success: true,
Expand All @@ -276,9 +413,7 @@ export async function generatePresignedRequest(
} catch (error) {
return {
success: false,
error: `Failed to generate presigned URL: ${
error instanceof Error ? error.message : String(error)
}`,
error: error instanceof Error ? error.message : String(error),
};
}
}
Expand All @@ -289,23 +424,14 @@ export async function generatePresignedUrl(
filename: string,
method: "PUT" | "GET" = "PUT",
options?: GeneratePacketPresignOptions
): Promise<
| {
success: false;
error: string;
}
| {
success: true;
url: string;
storagePath?: string;
}
> {
): Promise<PacketPresignFailure | { success: true; url: string; storagePath?: string }> {
const signed = await generatePresignedRequest(projectRef, envSlug, filename, method, options);

if (!signed.success) {
return {
success: false,
error: signed.error,
status: signed.status,
};
}

Expand Down
16 changes: 13 additions & 3 deletions apps/webapp/app/v3/objectStoreClient.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@ import { AwsClient } from "aws4fetch";
import { GetObjectCommand, PutObjectCommand, S3Client } from "@aws-sdk/client-s3";
import { getSignedUrl } from "@aws-sdk/s3-request-presigner";

/**
* Normalize a logical object-store key the same way Aws4FetchClient assigns URL pathnames.
* Decodes percent-escapes and resolves `.` / `..` segments before the request is signed.
*/
export function normalizeObjectStoreLogicalKeyPathname(logicalKey: string): string {
const url = new URL("https://trigger.invalid");
url.pathname = `/${logicalKey.replace(/^\/+/, "")}`;
return url.pathname;
}

interface IObjectStoreClient {
putObject(key: string, body: ReadableStream | string, contentType: string): Promise<string>;
getObject(key: string): Promise<string>;
Expand Down Expand Up @@ -32,7 +42,7 @@ class Aws4FetchClient implements IObjectStoreClient {

private buildUrl(key: string): string {
const url = new URL(this.config.baseUrl);
url.pathname = `/${key}`;
url.pathname = normalizeObjectStoreLogicalKeyPathname(key);
return url.toString();
}

Expand All @@ -59,7 +69,7 @@ class Aws4FetchClient implements IObjectStoreClient {

async presign(key: string, method: "PUT" | "GET", expiresIn: number): Promise<string> {
const url = new URL(this.config.baseUrl);
url.pathname = `/${key}`;
url.pathname = normalizeObjectStoreLogicalKeyPathname(key);
url.searchParams.set("X-Amz-Expires", String(expiresIn));

const signed = await this.awsClient.sign(new Request(url, { method }), {
Expand Down Expand Up @@ -100,7 +110,7 @@ class AwsSdkClient implements IObjectStoreClient {

private logicalObjectUrl(logicalKey: string): string {
const url = new URL(this.config.baseUrl);
url.pathname = `/${logicalKey}`;
url.pathname = normalizeObjectStoreLogicalKeyPathname(logicalKey);
return url.href;
}

Expand Down
Loading