Skip to content

Commit 8cb5caf

Browse files
kofkof
committed
wip
1 parent daede9c commit 8cb5caf

File tree

6 files changed

+59
-63
lines changed

6 files changed

+59
-63
lines changed

‎apps/builder/app/routes/cgi.asset.$.ts‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,14 @@ import { createReadStream, existsSync } from "node:fs";
22
import { join } from "node:path";
33
import { createReadableStreamFromReadable } from "@remix-run/node";
44
import type { LoaderFunctionArgs } from "@remix-run/server-runtime";
5-
import { getMimeTypeByFilename, isAllowedExtension } from "@webstudio-is/sdk";
5+
import {
6+
getMimeTypeByFilename,
7+
isAllowedExtension,
8+
decodePathFragment,
9+
} from "@webstudio-is/sdk";
610
import env from "~/env/env.server";
711
import { fileUploadPath } from "~/shared/asset-client";
812

9-
const decodePathFragment = (fragment: string) => {
10-
const decoded = decodeURIComponent(fragment);
11-
12-
// Prevent path traversal attacks
13-
if (decoded.includes("..") || decoded.startsWith("/")) {
14-
throw new Error("Invalid file path");
15-
}
16-
17-
return decoded;
18-
};
19-
2013
// This route serves generic assets without processing
2114
export const loader = async ({ request }: LoaderFunctionArgs) => {
2215
const basePath = `/cgi/asset/`;

‎apps/builder/app/routes/cgi.image.$.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { z } from "zod";
44
import { createReadableStreamFromReadable } from "@remix-run/node";
55
import type { LoaderFunctionArgs } from "@remix-run/server-runtime";
66
import { wsImageLoader } from "@webstudio-is/image";
7+
import { decodePathFragment } from "@webstudio-is/sdk";
78
import env from "~/env/env.server";
89
import { getImageNameAndType } from "~/builder/shared/assets/asset-utils";
910
import { fileUploadPath } from "~/shared/asset-client";
@@ -29,10 +30,6 @@ const ImageParams = z.object({
2930
]),
3031
});
3132

32-
const decodePathFragment = (fragment: string) => {
33-
return decodeURIComponent(fragment);
34-
};
35-
3633
// this route used as proxy for images to cloudflare endpoint
3734
// https://developers.cloudflare.com/fundamentals/get-started/reference/cdn-cgi-endpoint/
3835

‎apps/builder/app/routes/cgi.video.$.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,9 @@ import { join } from "node:path";
33
import { createReadableStreamFromReadable } from "@remix-run/node";
44
import type { LoaderFunctionArgs } from "@remix-run/server-runtime";
55
import env from "~/env/env.server";
6-
import { getMimeTypeByFilename } from "@webstudio-is/sdk";
6+
import { getMimeTypeByFilename, decodePathFragment } from "@webstudio-is/sdk";
77
import { fileUploadPath } from "~/shared/asset-client";
88

9-
const decodePathFragment = (fragment: string) => {
10-
return decodeURIComponent(fragment);
11-
};
12-
139
// this route used as proxy for videos to cloudflare endpoint or serve local files
1410
// https://developers.cloudflare.com/fundamentals/get-started/reference/cdn-cgi-endpoint/
1511

‎packages/sdk/src/assets.test.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
FONT_EXTENSIONS,
1414
FILE_EXTENSIONS_BY_CATEGORY,
1515
detectAssetType,
16+
decodePathFragment,
1617
} from "./assets";
1718

1819
describe("allowed-file-types", () => {
@@ -368,4 +369,39 @@ describe("allowed-file-types", () => {
368369
expect(detectAssetType("my.doc.file.pdf")).toBe("file");
369370
});
370371
});
372+
373+
describe("decodePathFragment", () => {
374+
test("decodes URI components correctly", () => {
375+
expect(decodePathFragment("hello%20world.jpg")).toBe("hello world.jpg");
376+
expect(decodePathFragment("image%2Btest.png")).toBe("image+test.png");
377+
expect(decodePathFragment("file%40name.pdf")).toBe("[email protected]");
378+
});
379+
380+
test("throws error on path traversal attempts with ..", () => {
381+
expect(() => decodePathFragment("../secret.txt")).toThrow(
382+
"Invalid file path"
383+
);
384+
expect(() => decodePathFragment("folder/../secret.txt")).toThrow(
385+
"Invalid file path"
386+
);
387+
expect(() => decodePathFragment("..%2Fsecret.txt")).toThrow(
388+
"Invalid file path"
389+
);
390+
});
391+
392+
test("throws error on absolute path attempts", () => {
393+
expect(() => decodePathFragment("/etc/passwd")).toThrow(
394+
"Invalid file path"
395+
);
396+
expect(() => decodePathFragment("%2Fetc%2Fpasswd")).toThrow(
397+
"Invalid file path"
398+
);
399+
});
400+
401+
test("allows normal filenames and paths", () => {
402+
expect(decodePathFragment("image.jpg")).toBe("image.jpg");
403+
expect(decodePathFragment("folder/image.jpg")).toBe("folder/image.jpg");
404+
expect(decodePathFragment("my-file_123.png")).toBe("my-file_123.png");
405+
});
406+
});
371407
});

‎packages/sdk/src/assets.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,21 @@ export const detectAssetType = (
289289
return "file";
290290
};
291291

292+
/**
293+
* Safely decode a URL path fragment, preventing path traversal attacks
294+
* @throws Error if the decoded path contains path traversal attempts
295+
*/
296+
export const decodePathFragment = (fragment: string): string => {
297+
const decoded = decodeURIComponent(fragment);
298+
299+
// Prevent path traversal attacks
300+
if (decoded.includes("..") || decoded.startsWith("/")) {
301+
throw new Error("Invalid file path");
302+
}
303+
304+
return decoded;
305+
};
306+
292307
/**
293308
* Generates the appropriate URL for an asset based on its type and format.
294309
* - Images use /cgi/image/ with format=raw

‎packages/sdk/src/expression.ts‎

Lines changed: 1 addition & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ export const allowedArrayMethods = new Set([
4949
"includes",
5050
"join",
5151
"slice",
52-
"find",
5352
"filter",
5453
]);
5554

@@ -96,43 +95,8 @@ export const lintExpression = ({
9695
sourceType: "module",
9796
});
9897

99-
// Track arrow functions that are used as callbacks for safe array methods
100-
type ArrowFunctionNode = Node & { type: "ArrowFunctionExpression" };
101-
const allowedArrowFunctions = new Set<ArrowFunctionNode>();
102-
// Track arrow function parameters to skip validation
103-
const arrowFunctionParams = new Set<string>();
104-
105-
// First pass: identify arrow functions used in safe contexts
106-
simple(root, {
107-
CallExpression(node) {
108-
if (node.callee.type === "MemberExpression") {
109-
if (node.callee.property.type === "Identifier") {
110-
const methodName = node.callee.property.name;
111-
if (allowedArrayMethods.has(methodName)) {
112-
// Mark arrow function arguments as allowed
113-
for (const arg of node.arguments) {
114-
if (arg.type === "ArrowFunctionExpression") {
115-
allowedArrowFunctions.add(arg);
116-
// Collect parameter names
117-
for (const param of arg.params) {
118-
if (param.type === "Identifier") {
119-
arrowFunctionParams.add(param.name);
120-
}
121-
}
122-
}
123-
}
124-
}
125-
}
126-
}
127-
},
128-
});
129-
13098
simple(root, {
13199
Identifier(node) {
132-
// Skip validation for arrow function parameters
133-
if (arrowFunctionParams.has(node.name)) {
134-
return;
135-
}
136100
if (availableVariables.has(node.name) === false) {
137101
addMessage(
138102
`"${node.name}" is not defined in the scope`,
@@ -196,12 +160,7 @@ export const lintExpression = ({
196160
},
197161
NewExpression: addMessage("Classes are not supported"),
198162
SequenceExpression: addMessage(`Only single expression is supported`),
199-
ArrowFunctionExpression(node) {
200-
// Allow arrow functions only when used as callbacks for safe array methods
201-
if (!allowedArrowFunctions.has(node)) {
202-
addMessage("Functions are not supported")(node);
203-
}
204-
},
163+
ArrowFunctionExpression: addMessage("Functions are not supported"),
205164
TaggedTemplateExpression: addMessage("Tagged template is not supported"),
206165
ClassExpression: addMessage("Classes are not supported"),
207166
MetaProperty: addMessage("Imports are not supported"),

0 commit comments

Comments
 (0)