Skip to content

Commit a2d2ce6

Browse files
committed
fix: resolve CodeQL security warnings
- Replace SHA-256 with scrypt for API key hashing in hash.util.ts and the HashApiKeys migration to use a computationally expensive KDF - Add explicit max batch size (1000) in telemetry service to prevent loop bound injection from unbounded user-controlled arrays - Remove API key snippet from validation error messages in openclaw-plugin config to prevent sensitive data leakage in logs - Add explicit `permissions: contents: read` to CI workflow to follow principle of least privilege https://claude.ai/code/session_012Y448H3mNA8HJsDRJtXvEC
1 parent 8228a71 commit a2d2ce6

File tree

6 files changed

+21
-16
lines changed

6 files changed

+21
-16
lines changed

.github/workflows/ci.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ on:
99
branches: [main]
1010
workflow_dispatch:
1111

12+
permissions:
13+
contents: read
14+
1215
jobs:
1316
backend:
1417
runs-on: ubuntu-latest

packages/backend/src/common/utils/hash.util.spec.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { sha256, keyPrefix } from './hash.util';
22

33
describe('hash.util', () => {
4-
describe('sha256', () => {
4+
describe('sha256 (scrypt-based)', () => {
55
it('returns a 64-character hex string', () => {
66
const result = sha256('test-input');
77
expect(result).toHaveLength(64);
@@ -15,13 +15,6 @@ describe('hash.util', () => {
1515
it('produces different output for different inputs', () => {
1616
expect(sha256('input-a')).not.toBe(sha256('input-b'));
1717
});
18-
19-
it('matches known SHA-256 digest', () => {
20-
// echo -n "abc" | sha256sum
21-
expect(sha256('abc')).toBe(
22-
'ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad',
23-
);
24-
});
2518
});
2619

2720
describe('keyPrefix', () => {

packages/backend/src/common/utils/hash.util.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
1-
import { createHash } from 'crypto';
1+
import { scryptSync } from 'crypto';
2+
3+
const HASH_SALT = 'manifest-api-key-salt';
4+
const KEY_LENGTH = 32;
25

36
export function sha256(input: string): string {
4-
return createHash('sha256').update(input).digest('hex');
7+
return scryptSync(input, HASH_SALT, KEY_LENGTH).toString('hex');
58
}
69

710
export function keyPrefix(key: string): string {

packages/backend/src/database/migrations/1771500000000-HashApiKeys.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import { MigrationInterface, QueryRunner } from 'typeorm';
2-
import { createHash } from 'crypto';
2+
import { scryptSync } from 'crypto';
3+
4+
const HASH_SALT = 'manifest-api-key-salt';
5+
const KEY_LENGTH = 32;
36

47
export class HashApiKeys1771500000000 implements MigrationInterface {
58
name = 'HashApiKeys1771500000000';
@@ -53,7 +56,7 @@ export class HashApiKeys1771500000000 implements MigrationInterface {
5356
`SELECT id, key FROM "agent_api_keys" WHERE key IS NOT NULL AND key != ''`,
5457
);
5558
for (const row of agentKeys) {
56-
const hash = createHash('sha256').update(row.key).digest('hex');
59+
const hash = scryptSync(row.key, HASH_SALT, KEY_LENGTH).toString('hex');
5760
const prefix = row.key.substring(0, 12);
5861
await queryRunner.query(
5962
`UPDATE "agent_api_keys" SET key_hash = $1, key_prefix = $2 WHERE id = $3`,
@@ -65,7 +68,7 @@ export class HashApiKeys1771500000000 implements MigrationInterface {
6568
`SELECT id, key FROM "api_keys" WHERE key IS NOT NULL AND key != ''`,
6669
);
6770
for (const row of apiKeys) {
68-
const hash = createHash('sha256').update(row.key).digest('hex');
71+
const hash = scryptSync(row.key, HASH_SALT, KEY_LENGTH).toString('hex');
6972
const prefix = row.key.substring(0, 12);
7073
await queryRunner.query(
7174
`UPDATE "api_keys" SET key_hash = $1, key_prefix = $2 WHERE id = $3`,

packages/backend/src/telemetry/telemetry.service.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,17 @@ export class TelemetryService {
2727
private readonly pricingCache: ModelPricingCacheService,
2828
) {}
2929

30+
private static readonly MAX_EVENTS_PER_BATCH = 1000;
31+
3032
async ingest(events: TelemetryEventDto[], userId: string): Promise<IngestResult> {
33+
const safeEvents = events.slice(0, TelemetryService.MAX_EVENTS_PER_BATCH);
3134
let accepted = 0;
3235
let rejected = 0;
3336
const errors: Array<{ index: number; reason: string }> = [];
3437

35-
for (let i = 0; i < events.length; i++) {
38+
for (let i = 0; i < safeEvents.length; i++) {
3639
try {
37-
await this.insertEvent(events[i], userId);
40+
await this.insertEvent(safeEvents[i], userId);
3841
accepted++;
3942
} catch (err) {
4043
rejected++;

packages/openclaw-plugin/src/config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ export function validateConfig(config: ManifestConfig): string | null {
6565
}
6666
if (!config.apiKey.startsWith("mnfst_")) {
6767
return (
68-
`Invalid apiKey format '${config.apiKey.slice(0, 8)}…'. ` +
68+
"Invalid apiKey format. " +
6969
"Keys must start with 'mnfst_'. Fix it via:\n" +
7070
" openclaw config set manifest.apiKey mnfst_YOUR_KEY"
7171
);

0 commit comments

Comments
 (0)