Skip to content
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
3 changes: 2 additions & 1 deletion src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ export const saveSession = () => {
if (session !== savedSession) {
const current = session;
const data = JSON.stringify(current, null, 4);
fs.writeFileSync(credentialFile, data, 'utf8');
fs.writeFileSync(credentialFile, data, { encoding: 'utf8', mode: 0o600 });
fs.chmodSync(credentialFile, 0o600);
savedSession = current;
}
};
Expand Down
15 changes: 8 additions & 7 deletions src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { question } from './utils';

import { doDelete, get, post } from './api';
import type { Platform } from './types';
import { updateJson } from './utils/constants';
import { t } from './utils/i18n';

interface AppSummary {
Expand Down Expand Up @@ -125,9 +126,9 @@ export const appCommands = {
let updateInfo: Partial<
Record<Platform, { appId: number; appKey: string }>
> = {};
if (fs.existsSync('update.json')) {
if (fs.existsSync(updateJson)) {
try {
updateInfo = JSON.parse(fs.readFileSync('update.json', 'utf8'));
updateInfo = JSON.parse(fs.readFileSync(updateJson, 'utf8'));
} catch (e) {
console.error(t('failedToParseUpdateJson'));
throw e;
Expand All @@ -138,10 +139,10 @@ export const appCommands = {
appId: id,
appKey,
};
fs.writeFileSync(
'update.json',
JSON.stringify(updateInfo, null, 4),
'utf8',
);
fs.writeFileSync(updateJson, JSON.stringify(updateInfo, null, 4), {
encoding: 'utf8',
mode: 0o600,
});
fs.chmodSync(updateJson, 0o600);
},
};
139 changes: 139 additions & 0 deletions tests/security.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
import {
afterEach,
beforeEach,
describe,
expect,
mock,
spyOn,
test,
} from 'bun:test';

// Mock missing dependencies BEFORE any imports
mock.module('tty-table', () => ({
__esModule: true,
default: class Table {
render() { return 'mocked table'; }
},
}));
mock.module('i18next', () => ({
__esModule: true,
default: {
use: () => ({
init: () => {},
t: (key: string) => key,
}),
t: (key: string) => key,
},
t: (key: string) => key,
}));
mock.module('chalk', () => ({
__esModule: true,
default: {
green: (s: string) => s,
red: (s: string) => s,
yellow: (s: string) => s,
blue: (s: string) => s,
cyan: (s: string) => s,
magenta: (s: string) => s,
},
}));
mock.module('filesize-parser', () => ({
__esModule: true,
default: (s: string) => 1024,
}));
mock.module('form-data', () => ({
__esModule: true,
default: class FormData {
append() {}
},
}));
mock.module('node-fetch', () => ({
__esModule: true,
default: mock(async () => ({
ok: true,
json: async () => ({}),
})),
}));
mock.module('progress', () => ({
__esModule: true,
default: class ProgressBar {
tick() {}
},
}));
mock.module('tcp-ping', () => ({
__esModule: true,
ping: (opts: any, cb: any) => cb(null, { avg: 10 }),
}));
mock.module('fs-extra', () => ({
__esModule: true,
default: {
ensureDirSync: () => {},
readFileSync: (f: string, e: string) => '{}',
existsSync: () => true,
},
}));
mock.module('read', () => ({
__esModule: true,
read: async () => 'mocked input',
}));
mock.module('compare-versions', () => ({
__esModule: true,
satisfies: () => true,
}));

import fs from 'fs';
import { saveSession, replaceSession } from '../src/api';
import { appCommands } from '../src/app';
import { credentialFile } from '../src/utils/constants';

// Mock the get function in api.ts
mock.module('../src/api', () => {
const original = require('../src/api');
return {
...original,
get: mock(async () => ({ appKey: 'test-app-key' })),
};
});
Comment on lines +84 to +96
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟑 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if Bun's mock.module works when called after import
# Look for documentation or patterns in the codebase

# Search for other test files that use mock.module to see the pattern used
fd -e ts -e js . tests --exec grep -l "mock.module" {} 2>/dev/null | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && cat {}'

Repository: reactnativecn/react-native-update-cli

Length of output: 6757


Move the ../src/api mock registration before the imports.

The mock for the get function at lines 90-96 is registered after appCommands is imported at line 86. In Bun's module system, mock.module must be called before a module is imported to intercept it. Since appCommands is already bound to the original get function, the subsequent mock won't affect its behavior. Move the ../src/api mock to the beginning with the other mocks (before line 53).

πŸ€– Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/security.test.ts` around lines 84 - 96, The mock registration for the
get function (mock.module('../src/api', ...)) is placed after importing
appCommands, so appCommands has already bound the original get; move the
mock.module(...) block so it runs before any imports that pull in ../src/api
(specifically before the import of appCommands and any other imports that
transitively import get). Ensure mock.module is declared alongside the other
mocks at the top of the test file so that appCommands (and any functions that
depend on get) will see the mocked get implementation.


describe('Security: File Permissions', () => {
const testFiles = [credentialFile, 'update.json', 'cresc.config.json'];

beforeEach(() => {
// Clean up any existing files
for (const file of testFiles) {
if (fs.existsSync(file)) {
fs.unlinkSync(file);
}
}
});

afterEach(() => {
// Clean up
for (const file of testFiles) {
if (fs.existsSync(file)) {
fs.unlinkSync(file);
}
}
});

test('saveSession should create credentialFile with 0o600 permissions', () => {
replaceSession({ token: 'test-token' });
saveSession();

expect(fs.existsSync(credentialFile)).toBe(true);
const stats = fs.statSync(credentialFile);
expect(stats.mode & 0o777).toBe(0o600);
});

test('selectApp should create update.json with 0o600 permissions', async () => {
await appCommands.selectApp({
args: ['123'],
options: { platform: 'ios' },
});

const targetFile = 'update.json';
expect(fs.existsSync(targetFile)).toBe(true);
const stats = fs.statSync(targetFile);
expect(stats.mode & 0o777).toBe(0o600);
});
Comment on lines +128 to +138
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟑 Minor

Test hardcodes 'update.json' instead of using the updateJson constant.

If IS_CRESC is true, selectApp writes to 'cresc.config.json' (via the updateJson constant), but the test asserts on the hardcoded 'update.json', causing a false failure. Import and use updateJson for consistency.

πŸ’‘ Proposed fix

Update the import at line 87:

-import { credentialFile } from '../src/utils/constants';
+import { credentialFile, updateJson } from '../src/utils/constants';

Then update the test:

   test('selectApp should create update.json with 0o600 permissions', async () => {
     await appCommands.selectApp({
       args: ['123'],
       options: { platform: 'ios' },
     });

-    const targetFile = 'update.json';
+    const targetFile = updateJson;
     expect(fs.existsSync(targetFile)).toBe(true);
     const stats = fs.statSync(targetFile);
     expect(stats.mode & 0o777).toBe(0o600);
   });
πŸ“ Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test('selectApp should create update.json with 0o600 permissions', async () => {
await appCommands.selectApp({
args: ['123'],
options: { platform: 'ios' },
});
const targetFile = 'update.json';
expect(fs.existsSync(targetFile)).toBe(true);
const stats = fs.statSync(targetFile);
expect(stats.mode & 0o777).toBe(0o600);
});
test('selectApp should create update.json with 0o600 permissions', async () => {
await appCommands.selectApp({
args: ['123'],
options: { platform: 'ios' },
});
const targetFile = updateJson;
expect(fs.existsSync(targetFile)).toBe(true);
const stats = fs.statSync(targetFile);
expect(stats.mode & 0o777).toBe(0o600);
});
πŸ€– Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/security.test.ts` around lines 128 - 138, The test is asserting against
the hardcoded filename 'update.json' but selectApp writes to the filename
referenced by the updateJson constant (which can be 'cresc.config.json' when
IS_CRESC is true); import updateJson into tests/security.test.ts and replace the
hardcoded 'update.json' usages (e.g., the targetFile declaration and any fs
checks) with the updateJson constant so the test checks the actual file
selectApp writes to; ensure the import name matches the exported symbol and
update references to fs.existsSync(updateJson) and fs.statSync(updateJson).

});
Loading