Skip to content

fix: prevent overwriting template files via <FileTree> #336

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 23, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export default function ExampleFileTree() {
hiddenFiles={['package-lock.json']}
selectedFile={selectedFile}
onFileSelect={setSelectedFile}
onFileChange={(event) => {
onFileChange={async (event) => {
if (event.method === 'add') {
setFiles([...files, { path: event.value, type: event.type }]);
}
Expand Down
18 changes: 16 additions & 2 deletions docs/tutorialkit.dev/src/content/docs/reference/configuration.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,29 @@ type I18nText = {
*
* @default 'This action is not allowed'
*/
fileTreeActionNotAllowed?: string,
fileTreeActionNotAllowedText?: string,

/**
* Text shown on dialog describing allowed patterns when file or folder createion failed.
* Text shown on dialog when user attempts create file or folder that already exists on filesystem but is not visible on file tree, e.g. template files.
*
* @default 'File exists on filesystem already'
*/
fileTreeFileExistsAlreadyText?: string,

/**
* Text shown on dialog describing allowed patterns when file or folder creation failed.
*
* @default 'Created files and folders must match following patterns:'
*/
fileTreeAllowedPatternsText?: string,

/**
* Text shown on confirmation buttons on dialogs.
*
* @default 'OK'
*/
confirmationText?: string,

/**
* Text shown on top of the steps section.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,15 @@ A component to list files in a tree view.

* `onFileSelect: (file: string) => void` - A callback that will be called when a file is clicked. The path of the file that was clicked will be passed as an argument.

* `onFileChange: (event: FileChangeEvent) => void` - An optional callback that will be called when a new file or folder is created from the file tree's context menu. When callback is not passed, file tree does not allow adding new files.
* `onFileChange: (event: FileChangeEvent) => Promise<void>` - An optional callback that will be called when a new file or folder is created from the file tree's context menu. This callback should throw errors with `FilesystemError` messages.
```ts
interface FileChangeEvent {
type: 'file' | 'folder';
method: 'add' | 'remove' | 'rename';
value: string;
}

type FilesystemError = 'FILE_EXISTS' | 'FOLDER_EXISTS';
```

* `allowEditPatterns?: string[]` - Glob patterns for paths that allow editing files and folders. Disabled by default.
Expand Down
1 change: 1 addition & 0 deletions e2e/src/templates/default/file-on-template.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default 'This file is present on template';
Empty file.
2 changes: 1 addition & 1 deletion e2e/src/templates/file-server/index.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import http from 'node:http';
import fs from 'node:fs';
import http from 'node:http';

const server = http.createServer((req, res) => {
if (req.url === '/' || req.url === '/index.html') {
Expand Down
26 changes: 26 additions & 0 deletions e2e/test/file-tree.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,3 +222,29 @@ test('user cannot create files or folders in disallowed directories', async ({ p
await expect(dialog).not.toBeVisible();
}
});

test('user cannot create files or folders that exist on template', async ({ page }) => {
await page.goto(`${BASE_URL}/allow-edits-enabled`);
await expect(page.getByRole('heading', { level: 1, name: 'File Tree test - Allow Edits enabled' })).toBeVisible();

// wait for terminal to start
const terminalOutput = page.getByRole('tabpanel', { name: 'Terminal' });
await expect(terminalOutput).toContainText('~/tutorial', { useInnerText: true });

for (const [name, type] of [
['file-on-template.js', 'file'],
['folder-on-template', 'folder'],
] as const) {
await page.getByTestId('file-tree-root-context-menu').click({ button: 'right' });
await page.getByRole('menuitem', { name: `Create ${type}` }).click();

await page.locator('*:focus').fill(name);
await page.locator('*:focus').press('Enter');

const dialog = page.getByRole('dialog', { name: 'This action is not allowed' });
await expect(dialog.getByText('File exists on filesystem already')).toBeVisible();

await dialog.getByRole('button', { name: 'OK' }).click();
await expect(dialog).not.toBeVisible();
}
});
2 changes: 1 addition & 1 deletion packages/react/src/Panels/WorkspacePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ function EditorSection({ theme, tutorialStore, hasEditor }: PanelProps) {
}
}

function onFileTreeChange({ method, type, value }: FileTreeChangeEvent) {
async function onFileTreeChange({ method, type, value }: FileTreeChangeEvent) {
if (method === 'add' && type === 'file') {
return tutorialStore.addFile(value);
}
Expand Down
61 changes: 42 additions & 19 deletions packages/react/src/core/ContextMenu.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Root, Portal, Content, Item, Trigger } from '@radix-ui/react-context-menu';
import * as RadixDialog from '@radix-ui/react-dialog';
import { DEFAULT_LOCALIZATION, type FileDescriptor, type I18n } from '@tutorialkit/types';
import { DEFAULT_LOCALIZATION, type FileDescriptor, type I18n, type FilesystemError } from '@tutorialkit/types';
import picomatch from 'picomatch/posix';
import { useRef, useState, type ComponentProps, type ReactNode } from 'react';
import { Button } from '../Button.js';
Expand All @@ -18,8 +18,8 @@ interface FileRenameEvent extends FileChangeEvent {
}

interface Props extends ComponentProps<'div'> {
/** Callback invoked when file is changed. */
onFileChange?: (event: FileChangeEvent | FileRenameEvent) => void;
/** Callback invoked when file is changed. This callback should throw errors with {@link FilesystemError} messages. */
onFileChange?: (event: FileChangeEvent | FileRenameEvent) => Promise<void>;

/** Glob patterns for paths that allow editing files and folders. Disabled by default. */
allowEditPatterns?: string[];
Expand All @@ -37,6 +37,7 @@ interface Props extends ComponentProps<'div'> {
| 'fileTreeCreateFolderText'
| 'fileTreeActionNotAllowedText'
| 'fileTreeAllowedPatternsText'
| 'fileTreeFileExistsAlreadyText'
| 'confirmationText'
>;

Expand All @@ -54,14 +55,16 @@ export function ContextMenu({
triggerProps,
...props
}: Props) {
const [state, setState] = useState<'idle' | 'add_file' | 'add_folder' | 'add_failed'>('idle');
const [state, setState] = useState<
'idle' | 'add_file' | 'add_folder' | 'add_failed_not_allowed' | 'add_failed_exists'
>('idle');
const inputRef = useRef<HTMLInputElement>(null);

if (!allowEditPatterns?.length) {
return children;
}

function onFileNameEnd(event: React.KeyboardEvent<HTMLInputElement> | React.FocusEvent<HTMLInputElement>) {
async function onFileNameEnd(event: React.KeyboardEvent<HTMLInputElement> | React.FocusEvent<HTMLInputElement>) {
if (state !== 'add_file' && state !== 'add_folder') {
return;
}
Expand All @@ -72,14 +75,22 @@ export function ContextMenu({
const value = `${directory}/${name}`;
const isAllowed = picomatch.isMatch(value, allowEditPatterns!);

if (isAllowed) {
onFileChange?.({
if (!isAllowed) {
return setState('add_failed_not_allowed');
}

try {
await onFileChange?.({
value,
type: state === 'add_file' ? 'file' : 'folder',
method: 'add',
});
} else {
return setState('add_failed');
} catch (error: any) {
const message: FilesystemError | (string & {}) | undefined = error?.message;

if (message === 'FILE_EXISTS' || message === 'FOLDER_EXISTS') {
return setState('add_failed_exists');
}
}
}

Expand Down Expand Up @@ -140,20 +151,20 @@ export function ContextMenu({
</Content>
</Portal>

{state === 'add_failed' && (
{(state === 'add_failed_not_allowed' || state === 'add_failed_exists') && (
<Dialog
title={i18n?.fileTreeActionNotAllowedText || DEFAULT_LOCALIZATION.fileTreeActionNotAllowedText}
confirmText={i18n?.confirmationText || DEFAULT_LOCALIZATION.confirmationText}
onClose={() => setState('idle')}
>
{i18n?.fileTreeAllowedPatternsText || DEFAULT_LOCALIZATION.fileTreeAllowedPatternsText}
<ul className={classNames('mt-2', allowEditPatterns.length > 1 && 'list-disc ml-4')}>
{allowEditPatterns.map((pattern) => (
<li key={pattern} className="mb-1">
<code>{pattern}</code>
</li>
))}
</ul>
{state === 'add_failed_not_allowed' ? (
<>
{i18n?.fileTreeAllowedPatternsText || DEFAULT_LOCALIZATION.fileTreeAllowedPatternsText}
<AllowPatternsList allowEditPatterns={allowEditPatterns} />
</>
) : (
<>{i18n?.fileTreeFileExistsAlreadyText || DEFAULT_LOCALIZATION.fileTreeFileExistsAlreadyText}</>
)}
</Dialog>
)}
</Root>
Expand Down Expand Up @@ -189,7 +200,7 @@ function Dialog({
<RadixDialog.Overlay className="fixed inset-0 opacity-50 bg-black" />

<RadixDialog.Content className="fixed top-50% left-50% transform-translate--50% w-90vw max-w-450px max-h-85vh rounded-xl text-tk-text-primary bg-tk-background-primary">
<div className="relative py-4 px-10">
<div className="relative p-10">
<RadixDialog.Title className="text-6">{title}</RadixDialog.Title>

<div className="my-4">{children}</div>
Expand All @@ -203,3 +214,15 @@ function Dialog({
</RadixDialog.Root>
);
}

function AllowPatternsList({ allowEditPatterns }: Required<Pick<Props, 'allowEditPatterns'>>) {
return (
<ul className={classNames('mt-2', allowEditPatterns.length > 1 && 'list-disc ml-4')}>
{allowEditPatterns.map((pattern) => (
<li key={pattern} className="mb-1">
<code>{pattern}</code>
</li>
))}
</ul>
);
}
14 changes: 11 additions & 3 deletions packages/runtime/src/store/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { FileDescriptor, Files, Lesson } from '@tutorialkit/types';
import type { FileDescriptor, Files, FilesystemError, Lesson } from '@tutorialkit/types';
import type { WebContainer } from '@webcontainer/api';
import { atom, type ReadableAtom } from 'nanostores';
import { LessonFilesFetcher } from '../lesson-files.js';
Expand Down Expand Up @@ -312,7 +312,7 @@ export class TutorialStore {
this._editorStore.setSelectedFile(filePath);
}

addFile(filePath: string) {
async addFile(filePath: string): Promise<void> {
// always select the existing or newly created file
this.setSelectedFile(filePath);

Expand All @@ -321,16 +321,24 @@ export class TutorialStore {
return;
}

if (await this._runner.fileExists(filePath)) {
throw new Error('FILE_EXISTS' satisfies FilesystemError);
}

this._editorStore.addFileOrFolder({ path: filePath, type: 'file' });
this._runner.updateFile(filePath, '');
}

addFolder(folderPath: string) {
async addFolder(folderPath: string) {
// prevent creating duplicates
if (this._editorStore.files.get().some((file) => file.path.startsWith(folderPath))) {
return;
}

if (await this._runner.folderExists(folderPath)) {
throw new Error('FOLDER_EXISTS' satisfies FilesystemError);
}

this._editorStore.addFileOrFolder({ path: folderPath, type: 'folder' });
this._runner.createFolder(folderPath);
}
Expand Down
39 changes: 39 additions & 0 deletions packages/runtime/src/store/tutorial-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,45 @@ export class TutorialRunner {
);
}

async fileExists(filepath: string) {
return this._fsExists(filepath, 'file');
}

async folderExists(folderPath: string) {
return this._fsExists(folderPath, 'folder');
}

private async _fsExists(filepath: string, type: 'file' | 'folder') {
if (this._currentFiles?.[filepath] || this._currentTemplate?.[filepath]) {
return true;
}

const previousLoadPromise = this._currentLoadTask?.promise;

return new Promise<boolean>((resolve) => {
this._currentLoadTask = newTask(
async () => {
await previousLoadPromise;

const webcontainer = await this._webcontainer;

try {
if (type === 'file') {
await webcontainer.fs.readFile(filepath);
} else {
await webcontainer.fs.readdir(filepath);
}

resolve(true);
} catch {
resolve(false);
}
},
{ ignoreCancel: true },
);
});
}

/**
* Load the provided files into WebContainer and remove any other files that had been loaded previously.
*
Expand Down
1 change: 1 addition & 0 deletions packages/types/src/default-localization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export const DEFAULT_LOCALIZATION = {
fileTreeCreateFileText: 'Create file',
fileTreeCreateFolderText: 'Create folder',
fileTreeActionNotAllowedText: 'This action is not allowed',
fileTreeFileExistsAlreadyText: 'File exists on filesystem already',
fileTreeAllowedPatternsText: 'Created files and folders must match following patterns:',
confirmationText: 'OK',
prepareEnvironmentTitleText: 'Preparing Environment',
Expand Down
2 changes: 2 additions & 0 deletions packages/types/src/entities/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ export type * from './nav.js';
export type FileDescriptor = { path: string; type: 'file' | 'folder' };
export type Files = Record<string, string | Uint8Array>;

export type FilesystemError = 'FILE_EXISTS' | 'FOLDER_EXISTS';

/**
* This tuple contains a "ref" which points to a file to fetch with the `LessonFilesFetcher` and
* the list of file paths included by that ref.
Expand Down
12 changes: 12 additions & 0 deletions packages/types/src/schemas/i18n.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,18 @@ export const i18nSchema = z.object({
.optional()
.describe("Text shown on dialog when user attempts to edit files that don't match allowed patterns."),

/**
* Text shown on dialog when user attempts create file or folder that already exists on filesystem but is not visible on file tree, e.g. template files.
*
* @default 'File exists on filesystem already'
*/
fileTreeFileExistsAlreadyText: z
.string()
.optional()
.describe(
'Text shown on dialog when user attempts create file or folder that already exists on filesystem but is not visible on file tree, e.g. template files.',
),

/**
* Text shown on dialog describing allowed patterns when file or folder creation failed.
*
Expand Down