fix: add defensive array check for GitLab sync variables response

[devin-ai-integration]

Context

Customers using GitLab secret syncs (created via Terraform) are hitting the error:

"syncMessage": "{\"error\":\"variables.filter is not a function\"}"

The gitbeaker library's get() helper checks Array.isArray(response.body) internally — when GitLab's API returns a non-array response (e.g., an error object with 200 status, a rate-limit response, or a self-hosted GitLab variation), gitbeaker's getSingle() path returns the raw object instead of an array. The code then calls .filter() on that object, producing the cryptic TypeError.

This adds an explicit Array.isArray() guard in getGitLabVariables before the .filter() call, converting the cryptic TypeError into a descriptive SecretSyncError.

Note: This is a defensive fix that improves the error message. The underlying cause of GitLab returning a non-array response for these specific customers still needs investigation (could be auth/token issues, self-hosted GitLab API differences, or rate limiting).

Important review considerations

• The fix throws rather than treating the non-array as empty — this is intentional to avoid silent data loss (an empty variable list would trigger deletion of all synced secrets if disableSecretDeletion is false).
• The catch block now re-throws SecretSyncError instances unchanged (matching the pattern in syncSecrets) to prevent double-wrapping the descriptive error message.
• Other call sites that use gitbeaker .all() + array methods (e.g., listGitLabProjects, listGitLabGroups in gitlab-connection-fns.ts) may have the same vulnerability but are not addressed here since they weren't reported as failing.

Human review checklist

• Verify that the Array.isArray check correctly surfaces a useful error message to the customer (trace through SecretSyncError → syncMessage serialization path)
• Confirm the catch-block ordering (SecretSyncError → GitbeakerRequestError → generic) doesn't inadvertently swallow other error types

Steps to verify the change

1. Confirm the Array.isArray check is placed after the .all() call and before the .filter() call
2. Verify the error message includes useful debugging info (typeof variables)
3. Confirm the SecretSyncError re-throw in the catch block prevents double-wrapping
4. Confirm the error handling is consistent with the pattern in syncSecrets

Type

• Fix
• Feature
• Improvement
• Breaking
• Docs
• Chore

Checklist

• Title follows the conventional commit format: type(scope): short description
• Tested locally
• Updated docs (if needed)
• Updated CLAUDE.md files (if needed)
• Read the contributing guide

Link to Devin session: https://app.devin.ai/sessions/e2cd8017aa62491fa2cfa065245d5637

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment and CI monitoring

[maidul98]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[greptile-apps]

Greptile Summary

This PR adds a defensive Array.isArray() guard in getGitLabVariables to prevent the cryptic "variables.filter is not a function" TypeError when GitLab's API returns a non-array response. The intent is correct and the decision to throw rather than silently treat the value as empty is a good one (avoids accidental mass-deletion).

However, there is one meaningful defect: the new Array.isArray() check lives inside the existing try block. When the guard throws a SecretSyncError, the catch block at line 103 catches it — and because SecretSyncError is not a GitbeakerRequestError, it falls through to the generic throw new SecretSyncError({ error }) at line 111. This double-wraps the error, burying the descriptive message inside a nested SecretSyncError.error property, and the useful context ("expected an array but received object") will likely not appear in the final syncMessage.

• Main issue: SecretSyncError thrown inside try block gets double-wrapped by the catch handler, defeating the purpose of the improved error message. Fix: add if (error instanceof SecretSyncError) { throw error; } at the top of the catch block in getGitLabVariables, mirroring the pattern already used in syncSecrets.
• Acknowledged but unaddressed: listGitLabProjects and listGitLabGroups in gitlab-connection-fns.ts have the same vulnerability; not a blocker for this PR but worth a follow-up.

Confidence Score: 4/5

Safe to merge after fixing the double-wrapping defect in the catch block — without the fix, the improved error message is silently swallowed.

The change is small and well-intentioned, but the single P1 finding means the primary goal of the PR (surfacing a clear error message) does not work as implemented. The double-wrapping needs to be resolved before merging.

backend/src/services/secret-sync/gitlab/gitlab-sync-fns.ts — specifically the catch block of getGitLabVariables

Important Files Changed

Filename	Overview
backend/src/services/secret-sync/gitlab/gitlab-sync-fns.ts	Adds Array.isArray() guard for GitLab variables response, but the thrown SecretSyncError is inside the try block and gets double-wrapped by the existing catch handler, negating the improved error message.

_{Reviews (1): Last reviewed commit: "fix: add defensive array check for GitLa..." | Re-trigger Greptile}

[greptile-apps]

New SecretSyncError gets silently double-wrapped

The Array.isArray() guard is placed inside the try block (lines 75–102). When the check fails and throws a SecretSyncError, it is immediately caught by the catch block at line 103. Since SecretSyncError is not a GitbeakerRequestError, execution falls through to the generic re-wrap on line 111:

throw new SecretSyncError({ error });  // error here is already a SecretSyncError

This means the outer SecretSyncError carries an inner SecretSyncError as its error property rather than a plain Error. The descriptive message "expected an array but received …" ends up nested and is unlikely to surface in the final syncMessage, defeating the primary purpose of this fix.

The simplest fix is to re-throw SecretSyncError instances unchanged at the top of the catch block, consistent with what syncSecrets already does:

Suggested change

	if (!Array.isArray(variables)) {
	throw new SecretSyncError({
	error: new Error(
	`Unexpected response from GitLab API when fetching variables: expected an array but received ${typeof variables}`
	)
	});
	}
	if (!Array.isArray(variables)) {
	throw new SecretSyncError({
	error: new Error(
	`Unexpected response from GitLab API when fetching variables: expected an array but received ${typeof variables}`
	)
	});
	}

And in the catch block (around line 103), add:

} catch (error) {
    if (error instanceof SecretSyncError) {
      throw error;
    }
    if (error instanceof GitbeakerRequestError) {
      // existing handling …
    }
    throw new SecretSyncError({ error });
  }