Skip to content

Commit d0f7f32

Browse files
gavinbarronCopilot
andauthored
fix: prevent path traversal in playground manifest URL validation (#3487)
The Editor component validated manifest URLs using startsWith() which is bypassed by '../' path traversal segments. An attacker could load arbitrary manifests from other GitHub repos, enabling RCE and token exfiltration on mgt.dev. Changes: - Normalize URLs with URL constructor before prefix check (resolves ../) - Validate content URLs from manifest against trusted prefix - Extract isValidManifestUrl to shared utility with tests - Add workflow_dispatch workflow for manual storybook production deploys Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 6120d87 commit d0f7f32

4 files changed

Lines changed: 162 additions & 3 deletions

File tree

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
# Copyright (c) Microsoft Corporation.
2+
# Licensed under the MIT License.
3+
4+
name: Deploy storybook to production
5+
6+
on:
7+
workflow_dispatch:
8+
9+
permissions:
10+
contents: write
11+
12+
jobs:
13+
deploy:
14+
runs-on: ubuntu-latest
15+
if: github.repository == 'microsoftgraph/microsoft-graph-toolkit'
16+
17+
strategy:
18+
matrix:
19+
node-version: [20.x]
20+
21+
steps:
22+
- uses: actions/checkout@v4
23+
- name: Use Node.js ${{ matrix.node-version }}
24+
uses: actions/setup-node@v4
25+
with:
26+
node-version: ${{ matrix.node-version }}
27+
registry-url: 'https://registry.npmjs.org'
28+
29+
- name: Build 🛠
30+
run: |
31+
npm install -g yarn lerna
32+
yarn
33+
yarn build
34+
yarn storybook:build
35+
36+
- name: Deploy mgt.dev 🚀
37+
uses: JamesIves/github-pages-deploy-action@v4.4.1
38+
with:
39+
branch: gh-pages
40+
folder: storybook-static
41+
target-folder: .
42+
clean-exclude: next
43+
token: ${{ secrets.GITHUB_TOKEN }}

.storybook/addons/codeEditorAddon/codeAddon.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { ProviderState } from '../../../packages/mgt-element/dist/es6/providers/
44
import { EditorElement } from './editor';
55
import { CLIENTID, SETPROVIDER_EVENT, AUTH_PAGE } from '../../env';
66
import { beautifyContent } from '../../utils/beautifyContent';
7+
import { isValidManifestUrl } from '../../utils/isValidManifestUrl';
78

89
const mgtScriptName = './mgt.storybook.js';
910

@@ -140,9 +141,7 @@ export const withCodeEditor = makeDecorator({
140141
}
141142
};
142143

143-
const isValid = manifestUrl => {
144-
return manifestUrl && manifestUrl.startsWith('https://raw.githubusercontent.com/pnp/mgt-samples/main/');
145-
};
144+
const isValid = isValidManifestUrl;
146145

147146
if (context.name === 'Editor') {
148147
// If the editor is not iframed (Docs, GE, etc.)
@@ -152,6 +151,11 @@ export const withCodeEditor = makeDecorator({
152151

153152
if (isValid(manifestUrl)) {
154153
getContent(manifestUrl, true).then(manifest => {
154+
const contentUrls = [manifest[0].preview.html, manifest[0].preview.js, manifest[0].preview.css];
155+
if (contentUrls.some(u => u && !isValid(u))) {
156+
console.warn(`🦒: Manifest contains untrusted URLs`);
157+
return;
158+
}
155159
Promise.all([
156160
getContent(manifest[0].preview.html),
157161
getContent(manifest[0].preview.js),
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
const TRUSTED_PREFIX = 'https://raw.githubusercontent.com/pnp/mgt-samples/main/';
2+
3+
/**
4+
* Validates that a URL points to a trusted location within the pnp/mgt-samples repository.
5+
* Uses URL normalization to prevent path traversal attacks (e.g., '../' segments).
6+
*
7+
* @param {string} url - The URL to validate
8+
* @returns {boolean} Whether the URL is trusted
9+
*/
10+
export const isValidManifestUrl = url => {
11+
if (!url) return false;
12+
try {
13+
const normalized = new URL(url).href;
14+
return normalized.startsWith(TRUSTED_PREFIX) && !normalized.includes('..');
15+
} catch {
16+
return false;
17+
}
18+
};
19+
20+
export { TRUSTED_PREFIX };
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
import { describe, it } from 'node:test';
2+
import assert from 'node:assert/strict';
3+
import { isValidManifestUrl, TRUSTED_PREFIX } from './isValidManifestUrl.js';
4+
5+
describe('isValidManifestUrl', () => {
6+
describe('valid URLs', () => {
7+
it('should accept a URL directly under the trusted prefix', () => {
8+
const url = `${TRUSTED_PREFIX}samples/my-sample/manifest.json`;
9+
assert.equal(isValidManifestUrl(url), true);
10+
});
11+
12+
it('should accept a URL in a nested path under the trusted prefix', () => {
13+
const url = `${TRUSTED_PREFIX}samples/app/deep/path/manifest.json`;
14+
assert.equal(isValidManifestUrl(url), true);
15+
});
16+
17+
it('should accept the trusted prefix itself', () => {
18+
assert.equal(isValidManifestUrl(TRUSTED_PREFIX), true);
19+
});
20+
});
21+
22+
describe('path traversal attacks', () => {
23+
it('should reject a URL with ../ that traverses to another repository', () => {
24+
const url = 'https://raw.githubusercontent.com/pnp/mgt-samples/main/../../../attacker/repo/main/manifest.json';
25+
assert.equal(isValidManifestUrl(url), false);
26+
});
27+
28+
it('should reject a URL with encoded traversal segments (%2e%2e)', () => {
29+
const url = 'https://raw.githubusercontent.com/pnp/mgt-samples/main/%2e%2e/%2e%2e/%2e%2e/attacker/repo/main/evil.js';
30+
assert.equal(isValidManifestUrl(url), false);
31+
});
32+
33+
it('should reject a URL that uses ../ to escape to a sibling repo', () => {
34+
const url = 'https://raw.githubusercontent.com/pnp/mgt-samples/main/../other-repo/main/payload.json';
35+
assert.equal(isValidManifestUrl(url), false);
36+
});
37+
38+
it('should reject a URL with multiple ../ segments', () => {
39+
const url = 'https://raw.githubusercontent.com/pnp/mgt-samples/main/../../evil-org/evil-repo/main/manifest.json';
40+
assert.equal(isValidManifestUrl(url), false);
41+
});
42+
});
43+
44+
describe('different origins and invalid URLs', () => {
45+
it('should reject a URL from a completely different domain', () => {
46+
const url = 'https://evil.com/pnp/mgt-samples/main/manifest.json';
47+
assert.equal(isValidManifestUrl(url), false);
48+
});
49+
50+
it('should reject a URL from a different GitHub user/org', () => {
51+
const url = 'https://raw.githubusercontent.com/attacker/malicious-repo/main/manifest.json';
52+
assert.equal(isValidManifestUrl(url), false);
53+
});
54+
55+
it('should reject a URL that only partially matches the prefix', () => {
56+
const url = 'https://raw.githubusercontent.com/pnp/mgt-samples-evil/main/manifest.json';
57+
assert.equal(isValidManifestUrl(url), false);
58+
});
59+
60+
it('should reject a data: URI', () => {
61+
const url = 'data:application/json;base64,W3sicHJldmlldyI6eyJqcyI6Imh0dHA6Ly9ldmlsLmNvbS9ldmlsLmpzIn19XQ==';
62+
assert.equal(isValidManifestUrl(url), false);
63+
});
64+
65+
it('should reject a javascript: URI', () => {
66+
const url = 'javascript:alert(1)';
67+
assert.equal(isValidManifestUrl(url), false);
68+
});
69+
});
70+
71+
describe('null/empty/malformed inputs', () => {
72+
it('should reject null', () => {
73+
assert.equal(isValidManifestUrl(null), false);
74+
});
75+
76+
it('should reject undefined', () => {
77+
assert.equal(isValidManifestUrl(undefined), false);
78+
});
79+
80+
it('should reject empty string', () => {
81+
assert.equal(isValidManifestUrl(''), false);
82+
});
83+
84+
it('should reject a malformed URL', () => {
85+
assert.equal(isValidManifestUrl('not-a-url'), false);
86+
});
87+
88+
it('should reject a relative path', () => {
89+
assert.equal(isValidManifestUrl('../../../etc/passwd'), false);
90+
});
91+
});
92+
});

0 commit comments

Comments
 (0)