Skip to content

Commit 48dcf31

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 2a1eff4 commit 48dcf31

File tree

11 files changed

+422
-10
lines changed

11 files changed

+422
-10
lines changed

docs/snippets/schemas/v3/index.schema.mdx

Lines changed: 100 additions & 0 deletions
Large diffs are not rendered by default.

docs/snippets/schemas/v3/languageModel.schema.mdx

Lines changed: 100 additions & 0 deletions
Large diffs are not rendered by default.

docs/snippets/schemas/v3/shared.schema.mdx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@
9393
"properties": {
9494
"secret": {
9595
"type": "string",
96+
"minLength": 1,
9697
"description": "The name of the secret that contains the token."
9798
}
9899
},
@@ -106,6 +107,7 @@
106107
"properties": {
107108
"env": {
108109
"type": "string",
110+
"minLength": 1,
109111
"description": "The name of the environment variable that contains the token. Only supported in declarative connection configs."
110112
}
111113
},

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
@@ -608,6 +608,7 @@ const schema = {
608608
"properties": {
609609
"username": {
610610
"type": "string",
611+
"minLength": 1,
611612
"description": "Gerrit username for authentication",
612613
"examples": [
613614
"john.doe"
@@ -630,7 +631,7 @@ const schema = {
630631
"secret": {
631632
"type": "string",
632633
"minLength": 1,
633-
"description": "The name of the secret that contains the token."
634+
"description": "The name of the secret that contains the HTTP password."
634635
}
635636
},
636637
"required": [
@@ -644,7 +645,7 @@ const schema = {
644645
"env": {
645646
"type": "string",
646647
"minLength": 1,
647-
"description": "The name of the environment variable that contains the token. Only supported in declarative connection configs."
648+
"description": "The name of the environment variable that contains the HTTP password. Only supported in declarative connection configs."
648649
}
649650
},
650651
"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": [

0 commit comments

Comments
 (0)