Skip to content

fix: test git connection with stored token#22

Merged
TerrifiedBug merged 1 commit intomainfrom
fix/git-test-stored-token
Mar 6, 2026
Merged

fix: test git connection with stored token#22
TerrifiedBug merged 1 commit intomainfrom
fix/git-test-stored-token

Conversation

@TerrifiedBug
Copy link
Copy Markdown
Owner

Summary

  • Allow "Test Connection" button to work with the saved encrypted token
  • Server-side: make token optional in testGitConnection input, fall back to decrypting stored gitToken from environment
  • Client-side: omit token from request when using stored token instead of showing warning

Test Plan

  • Save Git integration settings with a valid token
  • Clear the token input field
  • Click "Test Connection" — should succeed using the stored token
  • Enter a new token and test — should use the new token
  • Test with no saved token and no input — should show "Enter an access token first"

The test connection button now works with the saved encrypted token
instead of requiring users to re-enter it. Server decrypts the stored
token when no explicit token is provided in the request.
@github-actions github-actions bot added the fix label Mar 6, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 6, 2026

Greptile Summary

This PR fixes the "Test Connection" button so it works with a previously saved (encrypted) Git token — previously, users with a saved token but a cleared input field were blocked by a warning. The server-side change makes token optional and falls back to decrypting the stored gitToken; the client-side change removes the now-redundant warning path and omits the token field from the request when no new token has been entered.

  • withTeamAccess("EDITOR") and withAudit middleware are both preserved — authorization and audit trail are unaffected.
  • The token-in-URL sanitization regex (https?:\/\/[^@\s]+@) correctly redacts the resolved token (whether new or stored) from any git error messages.
  • The decrypt() call is outside the try/catch that wraps the git clone. If the stored ciphertext is corrupted or was encrypted with a different NEXTAUTH_SECRET, the exception propagates as a raw tRPC INTERNAL_SERVER_ERROR instead of the graceful { success: false, error: "..." } pattern used everywhere else in this procedure — the client's onError handler fires with a generic message rather than a useful description.
  • isTesting is correctly reset in both onSuccess and onError callbacks on the client.

Confidence Score: 4/5

  • Safe to merge with minor fix — the decrypt error path should be caught to keep error handling consistent.
  • The feature logic is correct and all security-sensitive concerns (authorization, audit logging, token redaction in error messages) are properly handled. The only gap is that decrypt() is not wrapped in a try/catch, which can produce an unexpected raw error in a rare but realistic failure scenario (key rotation, data corruption). Everything else — guard conditions, isTesting state management, Zod schema, middleware — looks solid.
  • Pay attention to src/server/routers/environment.ts around the decrypt() call at line 163.

Important Files Changed

Filename Overview
src/server/routers/environment.ts Makes token optional in testGitConnection and falls back to decrypting the stored gitToken. Authorization, audit logging, and error sanitization are all intact. One issue: the decrypt() call is outside the try/catch block, so a corrupted or mis-keyed token throws a raw exception instead of returning { success: false, error } like the rest of the procedure.
src/components/environment/git-sync-section.tsx Removes the now-unnecessary "Enter a new token" warning path and correctly omits the token field from the mutation payload when the user hasn't typed a new token. Guard conditions and isTesting reset logic are unchanged and correct.

Sequence Diagram

sequenceDiagram
    participant UI as GitSyncSection
    participant tRPC as testGitConnection
    participant MW as Middleware
    participant DB as PostgreSQL
    participant Git as Git Remote

    UI->>tRPC: mutate({ environmentId, repoUrl, branch, token? })
    tRPC->>MW: verify EDITOR access via environmentId
    MW-->>tRPC: authorized

    alt new access credential provided
        tRPC->>tRPC: use input credential
    else no credential in input
        tRPC->>DB: findUnique(environmentId) select gitToken
        DB-->>tRPC: encrypted value
        tRPC->>tRPC: decrypt stored value
    end

    tRPC->>tRPC: embed credential in URL (percent-encoded)
    tRPC->>Git: git clone --depth 1 --branch
    alt clone succeeds
        Git-->>tRPC: OK
        tRPC-->>UI: { success: true }
    else clone fails
        Git-->>tRPC: error
        tRPC->>tRPC: sanitize URL in error message
        tRPC-->>UI: { success: false, error: sanitized }
    end
    MW->>DB: writeAuditLog (sensitive fields redacted)
Loading

Last reviewed commit: 4291c04

if (!env?.gitToken) {
return { success: false, error: "No access token configured" };
}
resolvedToken = decrypt(env.gitToken);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

decrypt() call is outside the try/catch block

If the stored gitToken value is corrupted, truncated, or was encrypted with a different NEXTAUTH_SECRET, decrypt() will throw an Error. Since this call sits outside the try/catch block that wraps the git clone, the exception propagates as a raw tRPC INTERNAL_SERVER_ERROR rather than the { success: false, error: "..." } shape that the rest of this procedure returns. The client's onSuccess handler is never reached, so the user sees a generic "Connection test failed" toast with no useful description.

Wrap the decryption in a try/catch to keep the error path consistent:

Suggested change
resolvedToken = decrypt(env.gitToken);
resolvedToken = (() => {
try {
return decrypt(env.gitToken);
} catch {
return null;
}
})();
if (!resolvedToken) {
return { success: false, error: "Stored token could not be decrypted. Please re-save your token." };
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/server/routers/environment.ts
Line: 163

Comment:
**`decrypt()` call is outside the try/catch block**

If the stored `gitToken` value is corrupted, truncated, or was encrypted with a different `NEXTAUTH_SECRET`, `decrypt()` will throw an `Error`. Since this call sits outside the `try/catch` block that wraps the git clone, the exception propagates as a raw tRPC `INTERNAL_SERVER_ERROR` rather than the `{ success: false, error: "..." }` shape that the rest of this procedure returns. The client's `onSuccess` handler is never reached, so the user sees a generic "Connection test failed" toast with no useful description.

Wrap the decryption in a try/catch to keep the error path consistent:

```suggestion
        resolvedToken = (() => {
          try {
            return decrypt(env.gitToken);
          } catch {
            return null;
          }
        })();
        if (!resolvedToken) {
          return { success: false, error: "Stored token could not be decrypted. Please re-save your token." };
        }
```

How can I resolve this? If you propose a fix, please make it concise.

@TerrifiedBug TerrifiedBug merged commit f18af4c into main Mar 6, 2026
12 checks passed
@TerrifiedBug TerrifiedBug deleted the fix/git-test-stored-token branch March 6, 2026 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant