Skip to content

feat: add filename validation to newArticles command #289

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
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
15 changes: 15 additions & 0 deletions src/commands/newArticles.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,28 @@
import arg from "arg";
import { getFileSystemRepo } from "../lib/get-file-system-repo";
import { validateFilename } from "../lib/filename-validator";

export const newArticles = async (argv: string[]) => {
const args = arg({}, { argv });

const fileSystemRepo = await getFileSystemRepo();
let hasErrors = false;

if (args._.length > 0) {
for (const basename of args._) {
const validation = validateFilename(basename);
if (!validation.isValid) {
console.error(`Error: ${validation.error}`);
Copy link
Preview

Copilot AI Jul 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Currently the command skips invalid filenames but always exits with success. Consider setting process.exitCode = 1 when any validation fails so calling processes detect an overall failure.

Suggested change
console.error(`Error: ${validation.error}`);
console.error(`Error: ${validation.error}`);
hasErrors = true; // Mark that an error occurred

Copilot uses AI. Check for mistakes.

hasErrors = true;
continue;
}

const createdFileName = await fileSystemRepo.createItem(basename);
if (createdFileName) {
console.log(`created: ${createdFileName}.md`);
} else {
console.error(`Error: '${basename}.md' is already exist`);
hasErrors = true;
}
}
} else {
Expand All @@ -21,6 +31,11 @@ export const newArticles = async (argv: string[]) => {
console.log(`created: ${createdFileName}.md`);
} else {
console.error("Error: failed to create");
hasErrors = true;
}
}

if (hasErrors) {
process.exitCode = 1;
}
};
58 changes: 58 additions & 0 deletions src/lib/filename-validator.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import {
validateFilename,
ERROR_FILENAME_EMPTY,
ERROR_INVALID_CHARACTERS,
ERROR_INVALID_START_END,
} from "./filename-validator";

describe("validateFilename", () => {
it.each([
"test",
"test-article",
"test_article",
"test123",
"記事テスト",
"article.backup",
"my-awesome-article",
"test file",
])("should accept valid filename '%s'", (name) => {
const result = validateFilename(name);
expect(result.isValid).toBe(true);
expect(result.error).toBeUndefined();
});

it.each(["", " ", " ", "\t", "\n"])(
"should reject empty or whitespace-only filename '%s'",
(name) => {
const result = validateFilename(name);
expect(result.isValid).toBe(false);
expect(result.error).toBe(ERROR_FILENAME_EMPTY);
},
);

it.each([
"test<file",
"test>file",
"test:file",
'test"file',
"test/file",
"test\\file",
"test|file",
"test?file",
"test*file",
"test\x00file",
])("should reject filename with invalid characters '%s'", (name) => {
const result = validateFilename(name);
expect(result.isValid).toBe(false);
expect(result.error).toBe(ERROR_INVALID_CHARACTERS);
});

it.each([".test", "test.", " test", "test ", "..test", "test.."])(
"should reject filename starting or ending with dots or spaces '%s'",
(name) => {
const result = validateFilename(name);
expect(result.isValid).toBe(false);
expect(result.error).toBe(ERROR_INVALID_START_END);
},
);
});
40 changes: 40 additions & 0 deletions src/lib/filename-validator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// eslint-disable-next-line no-control-regex -- include control characters
const INVALID_FILENAME_CHARS = /[<>:"/\\|?*\x00-\x1f]/;

export const ERROR_FILENAME_EMPTY = "Filename is empty";
export const ERROR_INVALID_CHARACTERS =
'Filename contains invalid characters: < > : " / \\ | ? * and control characters';
export const ERROR_INVALID_START_END =
"Filename cannot start or end with a dot or space";

export interface FilenameValidationResult {
isValid: boolean;
error?: string;
}

export function validateFilename(filename: string): FilenameValidationResult {
if (!filename || filename.trim().length === 0) {
return {
isValid: false,
error: ERROR_FILENAME_EMPTY,
};
}

if (INVALID_FILENAME_CHARS.test(filename)) {
return {
isValid: false,
error: ERROR_INVALID_CHARACTERS,
};
}

if (/^[. ]|[. ]$/.test(filename)) {
return {
isValid: false,
error: ERROR_INVALID_START_END,
};
}

return {
isValid: true,
};
}