Skip to content
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

fix: exit with an error when some config file was found but lacks on reading permissions #2446

Merged
merged 3 commits into from
Feb 5, 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
5 changes: 3 additions & 2 deletions lib/compiler/assets-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,9 @@ export class AssetsManager {

this.watchers.push(watcher);
} else {
const files = sync(item.glob, { ignore: item.exclude })
.filter((matched) => statSync(matched).isFile());
const files = sync(item.glob, { ignore: item.exclude }).filter(
(matched) => statSync(matched).isFile(),
);
for (const path of files) {
this.actionOnFile({ ...option, path, action: 'change' });
}
Expand Down
21 changes: 14 additions & 7 deletions lib/configuration/nest-configuration.loader.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Reader } from '../readers';
import { Reader, ReaderFileLackPersmissionsError } from '../readers';
import { Configuration } from './configuration';
import { ConfigurationLoader } from './configuration.loader';
import { defaultConfiguration } from './defaults';
Expand All @@ -14,7 +14,7 @@ const loadedConfigsCache = new Map<string, Required<Configuration>>();
export class NestConfigurationLoader implements ConfigurationLoader {
constructor(private readonly reader: Reader) {}

public async load(name?: string): Promise<Required<Configuration>> {
public load(name?: string): Required<Configuration> {
const cacheEntryKey = `${this.reader.constructor.name}:${name}`;
const cachedConfig = loadedConfigsCache.get(cacheEntryKey);
if (cachedConfig) {
Expand All @@ -23,17 +23,24 @@ export class NestConfigurationLoader implements ConfigurationLoader {

let loadedConfig: Required<Configuration> | undefined;

const content: string | undefined = name
? await this.reader.read(name)
: await this.reader.readAnyOf([
const contentOrError = name
? this.reader.read(name)
: this.reader.readAnyOf([
'nest-cli.json',
'.nestcli.json',
'.nest-cli.json',
'nest.json',
]);

if (content) {
const fileConfig = JSON.parse(content);
if (contentOrError) {
const isMissingPersmissionsError =
contentOrError instanceof ReaderFileLackPersmissionsError;
if (isMissingPersmissionsError) {
console.error(contentOrError.message);
process.exit(1);
}

const fileConfig = JSON.parse(contentOrError);
if (fileConfig.compilerOptions) {
loadedConfig = {
...defaultConfiguration,
Expand Down
53 changes: 40 additions & 13 deletions lib/readers/file-system.reader.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,54 @@
import * as fs from 'fs';
import * as path from 'path';
import { Reader } from './reader';
import { Reader, ReaderFileLackPersmissionsError } from './reader';

export class FileSystemReader implements Reader {
constructor(private readonly directory: string) {}

public list(): Promise<string[]> {
return fs.promises.readdir(this.directory);
public list(): string[] {
return fs.readdirSync(this.directory);
}

public read(name: string): Promise<string> {
return fs.promises.readFile(path.join(this.directory, name), 'utf8');
public read(name: string): string {
return fs.readFileSync(path.join(this.directory, name), 'utf8');
}

public async readAnyOf(filenames: string[]): Promise<string | undefined> {
try {
for (const file of filenames) {
return await this.read(file);
public readAnyOf(
filenames: string[],
): string | undefined | ReaderFileLackPersmissionsError {
let firstFilePathFoundButWithInsufficientPermissions: string | undefined;

for (let id = 0; id < filenames.length; id++) {
const file = filenames[id];

try {
return this.read(file);
} catch (readErr) {
if (
!firstFilePathFoundButWithInsufficientPermissions &&
typeof readErr?.code === 'string'
) {
const isInsufficientPermissionsError =
readErr.code === 'EACCES' || readErr.code === 'EPERM';
if (isInsufficientPermissionsError) {
firstFilePathFoundButWithInsufficientPermissions = readErr.path;
}
}

const isLastFileToLookFor = id === filenames.length - 1;
if (!isLastFileToLookFor) {
continue;
}

if (firstFilePathFoundButWithInsufficientPermissions) {
return new ReaderFileLackPersmissionsError(
firstFilePathFoundButWithInsufficientPermissions,
readErr.code,
);
} else {
return undefined;
}
}
} catch (err) {
return filenames.length > 0
? await this.readAnyOf(filenames.slice(1, filenames.length))
: undefined;
}
}
}
17 changes: 14 additions & 3 deletions lib/readers/reader.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
export class ReaderFileLackPersmissionsError extends Error {
constructor(
public readonly filePath: string,
public readonly fsErrorCode: string,
) {
super(`File ${filePath} lacks read permissions!`);
}
}

export interface Reader {
list(): string[] | Promise<string[]>;
read(name: string): string | Promise<string>;
readAnyOf(filenames: string[]): string | Promise<string | undefined>;
list(): string[];
read(name: string): string;
readAnyOf(
filenames: string[],
): string | undefined | ReaderFileLackPersmissionsError;
}
22 changes: 9 additions & 13 deletions test/lib/configuration/nest-configuration.loader.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,17 @@ describe('Nest Configuration Loader', () => {
mock.mockImplementation(() => {
return {
readAnyOf: jest.fn(() =>
Promise.resolve(
JSON.stringify({
language: 'ts',
collection: '@nestjs/schematics',
}),
),
JSON.stringify({
language: 'ts',
collection: '@nestjs/schematics',
}),
),
read: jest.fn(() =>
Promise.resolve(
JSON.stringify({
language: 'ts',
collection: '@nestjs/schematics',
entryFile: 'secondary',
}),
),
JSON.stringify({
language: 'ts',
collection: '@nestjs/schematics',
entryFile: 'secondary',
}),
),
};
});
Expand Down
26 changes: 12 additions & 14 deletions test/lib/readers/file-system.reader.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ import * as fs from 'fs';
import { FileSystemReader, Reader } from '../../../lib/readers';

jest.mock('fs', () => ({
promises: {
readdir: jest.fn().mockResolvedValue([]),
readFile: jest.fn().mockResolvedValue('content'),
},
readdirSync: jest.fn().mockResolvedValue([]),
readFileSync: jest.fn().mockResolvedValue('content'),
}));

const dir: string = process.cwd();
Expand All @@ -15,25 +13,25 @@ describe('File System Reader', () => {
afterAll(() => {
jest.clearAllMocks();
});
it('should use fs.promises.readdir when list', async () => {
await reader.list();
expect(fs.promises.readdir).toHaveBeenCalled();
it('should use fs.readdirSync when list (for performance reasons)', async () => {
reader.list();
expect(fs.readdirSync).toHaveBeenCalled();
});
it('should use fs.promises.readFile when read', async () => {
await reader.read('filename');
expect(fs.promises.readFile).toHaveBeenCalled();
it('should use fs.readFileSync when read (for performance reasons)', async () => {
reader.read('filename');
expect(fs.readFileSync).toHaveBeenCalled();
});

describe('readAnyOf tests', () => {
it('should call readFile when running readAnyOf fn', async () => {
it('should call readFileSync when running readAnyOf fn', async () => {
const filenames: string[] = ['file1', 'file2', 'file3'];
await reader.readAnyOf(filenames);
reader.readAnyOf(filenames);

expect(fs.promises.readFile).toHaveBeenCalled();
expect(fs.readFileSync).toHaveBeenCalled();
});

it('should return undefined when no file is passed', async () => {
const content = await reader.readAnyOf([]);
const content = reader.readAnyOf([]);
expect(content).toEqual(undefined);
});
});
Expand Down