Skip to content

Commit 7adfc6a

Browse files
zuharzclaude
andcommitted
Fix Gerrit connection bugs and improve validation
- Fix critical bug: empty projects array filtering out all repos - Add minLength validation for Gerrit usernames - Update Gerrit auth docs from "token" to "HTTP password" - Improve network error handling with better logging - Make XSSI prefix removal more resilient - Remove unused @testing-library/react-hooks dependency - Enhance test coverage with better assertions 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 80351fe commit 7adfc6a

File tree

5 files changed

+18
-10
lines changed

5 files changed

+18
-10
lines changed

packages/backend/src/gerrit.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ export const getGerritReposFromConfig = async (
8787
}
8888

8989
// include repos by glob if specified in config
90-
if (config.projects) {
90+
if (config.projects?.length) {
9191
projects = projects.filter((project) => {
9292
return micromatch.isMatch(project.name, config.projects!);
9393
});
@@ -155,10 +155,11 @@ const fetchAllProjects = async (url: string, auth?: GerritAuthConfig): Promise<G
155155
throw err;
156156
}
157157

158-
const status = (err as any).code;
159-
logger.error(`Failed to fetch projects from Gerrit at ${endpointWithParams} with status ${status}`);
158+
const status = (err as any).code ?? 0;
159+
const message = (err as Error)?.message ?? String(err);
160+
logger.error(`Failed to fetch projects from Gerrit at ${endpointWithParams} with status ${status}: ${message}`);
160161
throw new BackendException(BackendError.CONNECTION_SYNC_FAILED_TO_FETCH_GERRIT_PROJECTS, {
161-
status: status,
162+
status,
162163
url: endpointWithParams,
163164
authenticated: !!auth
164165
});
@@ -168,7 +169,7 @@ const fetchAllProjects = async (url: string, auth?: GerritAuthConfig): Promise<G
168169
// Remove XSSI protection prefix that Gerrit adds to JSON responses
169170
// The regex /^\)\]\}'\n/ matches the literal string ")]}'" at the start of the response
170171
// followed by a newline character, which Gerrit adds to prevent JSON hijacking
171-
const jsonText = text.startsWith(")]}'") ? text.replace(/^\)\]\}'\n/, '') : text;
172+
const jsonText = text.replace(/^\)\]\}'\s*/, '');
172173
const data: GerritProjects = JSON.parse(jsonText);
173174

174175
// Add fetched projects to allProjects

packages/crypto/src/tokenUtils.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ describe('tokenUtils', () => {
3535
const result = await getTokenFromConfig(config, testOrgId, mockPrisma);
3636

3737
expect(result).toBe('decrypted-secret-value');
38+
// Verify we invoked decrypt with the secret's IV and encrypted value
39+
const { decrypt } = await import('./index.js');
40+
expect(decrypt).toHaveBeenCalledWith('test-iv', 'encrypted-value');
3841
expect(mockPrisma.secret.findUnique).toHaveBeenCalledWith({
3942
where: {
4043
orgId_key: {
@@ -52,6 +55,7 @@ describe('tokenUtils', () => {
5255
const result = await getTokenFromConfig(config, testOrgId, mockPrisma);
5356

5457
expect(result).toBe('env-token-value');
58+
expect(mockPrisma.secret.findUnique).not.toHaveBeenCalled();
5559
});
5660

5761
test('throws error for string tokens (security)', async () => {
@@ -75,6 +79,8 @@ describe('tokenUtils', () => {
7579

7680
await expect(getTokenFromConfig(config, testOrgId, mockPrisma))
7781
.rejects.toThrow('Secret with key non-existent-secret not found for org 1');
82+
const { decrypt } = await import('./index.js');
83+
expect(decrypt).not.toHaveBeenCalled();
7884
});
7985

8086
test('throws error for missing environment variable', async () => {

packages/schemas/src/v3/connection.schema.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -613,6 +613,7 @@ const schema = {
613613
"properties": {
614614
"username": {
615615
"type": "string",
616+
"minLength": 1,
616617
"description": "Gerrit username for authentication",
617618
"examples": [
618619
"john.doe"
@@ -635,7 +636,7 @@ const schema = {
635636
"secret": {
636637
"type": "string",
637638
"minLength": 1,
638-
"description": "The name of the secret that contains the token."
639+
"description": "The name of the secret that contains the HTTP password."
639640
}
640641
},
641642
"required": [
@@ -649,7 +650,7 @@ const schema = {
649650
"env": {
650651
"type": "string",
651652
"minLength": 1,
652-
"description": "The name of the environment variable that contains the token. Only supported in declarative connection configs."
653+
"description": "The name of the environment variable that contains the HTTP password. Only supported in declarative connection configs."
653654
}
654655
},
655656
"required": [

packages/schemas/src/v3/gerrit.schema.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ const schema = {
2323
"properties": {
2424
"username": {
2525
"type": "string",
26+
"minLength": 1,
2627
"description": "Gerrit username for authentication",
2728
"examples": [
2829
"john.doe"
@@ -45,7 +46,7 @@ const schema = {
4546
"secret": {
4647
"type": "string",
4748
"minLength": 1,
48-
"description": "The name of the secret that contains the token."
49+
"description": "The name of the secret that contains the HTTP password."
4950
}
5051
},
5152
"required": [
@@ -59,7 +60,7 @@ const schema = {
5960
"env": {
6061
"type": "string",
6162
"minLength": 1,
62-
"description": "The name of the environment variable that contains the token. Only supported in declarative connection configs."
63+
"description": "The name of the environment variable that contains the HTTP password. Only supported in declarative connection configs."
6364
}
6465
},
6566
"required": [

packages/web/package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,6 @@
193193
"@testing-library/dom": "^10.4.1",
194194
"@testing-library/jest-dom": "^6.7.0",
195195
"@testing-library/react": "^16.3.0",
196-
"@testing-library/react-hooks": "^8.0.1",
197196
"@types/micromatch": "^4.0.9",
198197
"@types/node": "^20",
199198
"@types/nodemailer": "^6.4.17",

0 commit comments

Comments
 (0)