Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/lib/components/git/IconGitea.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<!--
Gitea has no icon in @appwrite.io/pink-icons-svelte yet, so this is a
hand-drawn stand-in (simplified teapot silhouette, matching Gitea's
mascot) kept local to the git-connect UI until a proper brand icon
lands in the shared icon package. Mirrors the monochrome,
currentcolor-fill convention of the other Icon* components (e.g.
IconGithub) so it drops into <Icon icon={IconGitea} /> unchanged.
-->
<svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" fill="none" viewBox="0 0 20 20">
<path
fill="currentcolor"
fill-rule="evenodd"
d="M6 3.75A1.25 1.25 0 0 1 7.25 2.5h5.5A1.25 1.25 0 0 1 14 3.75V5h1.25A2.75 2.75 0 0 1 18 7.75v1.5A2.75 2.75 0 0 1 15.25 12H14v.114a3.886 3.886 0 0 1-3.886 3.886H9.886A3.886 3.886 0 0 1 6 12.114V3.75Zm7 5.75V3.75a.25.25 0 0 0-.25-.25h-5.5a.25.25 0 0 0-.25.25v8.364A2.886 2.886 0 0 0 9.886 15h.228A2.886 2.886 0 0 0 13 12.114V9.5ZM14 6v5h1.25c.966 0 1.75-.784 1.75-1.75v-1.5c0-.966-.784-1.75-1.75-1.75H14Z"
clip-rule="evenodd" />
<path
fill="currentcolor"
d="M4 16.5a.75.75 0 0 1 .75-.75h10.5a.75.75 0 0 1 0 1.5H4.75a.75.75 0 0 1-.75-.75Z" />
</svg>
32 changes: 24 additions & 8 deletions src/lib/components/git/connectGit.svelte
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
<script lang="ts">
import { isSelfHosted } from '$lib/system';
import { connectGitHub } from '$lib/stores/git';
import { connectGitHub, connectGitea } from '$lib/stores/git';
import Button from '$lib/elements/forms/button.svelte';
import { IconGithub } from '@appwrite.io/pink-icons-svelte';
import { Alert, Card, Empty, Icon, Layout } from '@appwrite.io/pink-svelte';
import { regionalConsoleVariables } from '$routes/(console)/project-[region]-[project]/store';
import IconGitea from './IconGitea.svelte';

export let callbackState: Record<string, string> = null;

let isVcsEnabled = $regionalConsoleVariables?._APP_VCS_ENABLED === true;
// Not in the SDK's generated types yet -- server already returns it.
let vcsProviders = ($regionalConsoleVariables as { _APP_VCS_PROVIDERS?: string[] })
?._APP_VCS_PROVIDERS;
let isGiteaEnabled = vcsProviders?.includes('gitea') ?? false;
Comment on lines +14 to +16

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.

P2 _APP_VCS_PROVIDERS runtime type assumption — The cast to { _APP_VCS_PROVIDERS?: string[] } is erased at runtime, so vcsProviders.includes('gitea') dispatches to whatever prototype the server actually returns. If the server sends a comma-separated string ("github,gitea"), String.prototype.includes will do a substring check instead of an exact-element check — which happens to work for 'gitea' but would produce false positives for values like "notgiteahere" or future provider names that are substrings of one another. Could you confirm the API response shape is already string[], or add a defensive runtime check?

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/components/git/connectGit.svelte
Line: 14-16

Comment:
**`_APP_VCS_PROVIDERS` runtime type assumption** — The cast to `{ _APP_VCS_PROVIDERS?: string[] }` is erased at runtime, so `vcsProviders.includes('gitea')` dispatches to whatever prototype the server actually returns. If the server sends a comma-separated string (`"github,gitea"`), `String.prototype.includes` will do a substring check instead of an exact-element check — which happens to work for `'gitea'` but would produce false positives for values like `"notgiteahere"` or future provider names that are substrings of one another. Could you confirm the API response shape is already `string[]`, or add a defensive runtime check?

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

Fix in Claude Code Fix in Codex

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@greptile Confirmed against the actual server code _APP_VCS_PROVIDERS is declared with 'array' => true in ConsoleVariables.php, and Appwrite's response serialization (Response.php:562) explicitly throws if that field isn't a real PHP array before encoding. So it's guaranteed to be a JSON array on the wire, never a comma-joined string — .includes('gitea') correctly dispatches to Array.prototype.includes. No substring-match risk here.

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.

Thanks for digging into that — the 'array' => true declaration and the serialization guard in Response.php are exactly the kind of server-side guarantees that make the type cast safe. The concern is fully resolved; no changes needed here.

</script>

<Layout.Stack>
Expand All @@ -35,13 +40,24 @@
title="No installation was added to the project yet"
description="Add an installation to connect repositories">
<svelte:fragment slot="actions">
<Button
secondary
href={connectGitHub(callbackState).toString()}
disabled={!isVcsEnabled}>
<Icon slot="start" icon={IconGithub} />
Connect to GitHub
</Button>
<Layout.Stack direction="row">
<Button
secondary
href={connectGitHub(callbackState).toString()}
disabled={!isVcsEnabled}>
<Icon slot="start" icon={IconGithub} />
Connect to GitHub
</Button>
{#if isGiteaEnabled}
<Button
secondary
href={connectGitea(callbackState).toString()}
disabled={!isVcsEnabled}>
<Icon slot="start" icon={IconGitea} />
Connect to Gitea
</Button>
{/if}
</Layout.Stack>
</svelte:fragment>
</Empty>
</Card.Base>
Expand Down
12 changes: 10 additions & 2 deletions src/lib/stores/git.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,29 @@
import { page } from '$app/state';
import { getApiEndpoint } from './sdk';

export function connectGitHub(callbackState: Record<string, string> = null) {
function connectVcsProvider(provider: string, callbackState: Record<string, string> = null) {
const redirect = new URL(page.url);
if (callbackState) {
Object.keys(callbackState).forEach((key) => {
redirect.searchParams.append(key, callbackState[key]);
});
}
const target = new URL(`${getApiEndpoint(page.params.region)}/vcs/github/authorize`);
const target = new URL(`${getApiEndpoint(page.params.region)}/vcs/${provider}/authorize`);
target.searchParams.set('project', page.params.project);
target.searchParams.set('success', redirect.toString());
target.searchParams.set('failure', redirect.toString());
target.searchParams.set('mode', 'admin');
return target;
}

export function connectGitHub(callbackState: Record<string, string> = null) {
return connectVcsProvider('github', callbackState);
}

export function connectGitea(callbackState: Record<string, string> = null) {
return connectVcsProvider('gitea', callbackState);
}

export function deploymentStatusConverter(status: string) {
// Status component possible values - status: 'waiting' | 'ready' | 'processing' | 'pending' | 'failed' | 'complete';
switch (status) {
Expand Down
Loading