Skip to content

fix: do not fetch metadata for self-hosted Gitea/Forgejo #2788

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
serhalp wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: do not fetch metadata for self-hosted Gitea/Forgejo #2788

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3116170
fix: do not fetch metadata for self-hosted Gitea/Forgejo
serhalp May 24, 2026
8e09e1c
Merge branch 'main' into serhalp/fix/do-not-fetch-gitea-forgejo-selfh…
serhalp May 24, 2026
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
1 change: 1 addition & 0 deletions shared/utils/fetch-cache-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export const FETCH_CACHE_ALLOWED_DOMAINS = [
'gitlab.com', // GitLab API
'api.bitbucket.org', // Bitbucket API
'codeberg.org', // Codeberg (Gitea-based)
'gitea.com', // Gitea API
'gitee.com', // Gitee API
// microcosm endpoints for atproto data
CONSTELLATION_HOST,
Expand Down
47 changes: 16 additions & 31 deletions shared/utils/git-providers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,18 @@ export const GITLAB_HOSTS = [
'framagit.org',
]

/**
* Repo URLs come from npm package metadata, so package publishers can specify any hostname. As this
* is effectively user-controlled input that can point at a malicious user-controlled server, this
* would put us at risk of Server-Side Request Forgery (SSRF). Thus we only support allowlisted hosts.
*/
export const FORGEJO_HOSTS = ['next.forgejo.org', 'try.next.forgejo.org']
Copy link
Copy Markdown
Contributor

@ghostdevv ghostdevv May 24, 2026

Choose a reason for hiding this comment

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

codeberg? or is that done somewhere else?

Copy link
Copy Markdown
Member Author

@serhalp serhalp May 24, 2026

Choose a reason for hiding this comment

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

codeberg is handled as a separate provider


/**
* No open-ended Gitea host detection for the same reason as Forgejo above.
*/
export const GITEA_HOSTS = ['gitea.com']
Comment thread
gameroman marked this conversation as resolved.

interface ProviderConfig {
id: ProviderId
/** Check if hostname matches this provider */
Expand Down Expand Up @@ -215,14 +227,7 @@ const providers: ProviderConfig[] = [
},
{
id: 'forgejo',
matchHost: host => {
// Match explicit Forgejo instances
const forgejoPatterns = [/^forgejo\./i, /\.forgejo\./i]
// Known Forgejo instances
const knownInstances = ['next.forgejo.org', 'try.next.forgejo.org']
if (knownInstances.some(h => host === h)) return true
return forgejoPatterns.some(p => p.test(host))
},
matchHost: host => FORGEJO_HOSTS.includes(host),
parsePath: parts => {
if (parts.length < 2) return null
const owner = decodeURIComponent(parts[0] ?? '').trim()
Expand All @@ -244,29 +249,7 @@ const providers: ProviderConfig[] = [
},
{
id: 'gitea',
matchHost: host => {
// Match common Gitea hosting patterns (Forgejo has its own adapter)
const giteaPatterns = [/^git\./i, /^gitea\./i, /^code\./i, /^src\./i, /gitea\.io$/i]
// Skip known providers (including Forgejo patterns)
const skipHosts = [
'github.com',
'gitlab.com',
'codeberg.org',
'bitbucket.org',
'gitee.com',
'sr.ht',
'git.sr.ht',
'tangled.sh',
'tangled.org',
'next.forgejo.org',
'try.next.forgejo.org',
...GITLAB_HOSTS,
]
if (skipHosts.some(h => host === h || host.endsWith(`.${h}`))) return false
// Skip Forgejo patterns
if (/^forgejo\./i.test(host) || /\.forgejo\./i.test(host)) return false
return giteaPatterns.some(p => p.test(host))
},
matchHost: host => GITEA_HOSTS.includes(host),
parsePath: parts => {
if (parts.length < 2) return null
const owner = decodeURIComponent(parts[0] ?? '').trim()
Expand Down Expand Up @@ -407,4 +390,6 @@ export const GIT_PROVIDER_API_ORIGINS = {
export const ALL_KNOWN_GIT_API_ORIGINS: readonly string[] = [
...Object.values(GIT_PROVIDER_API_ORIGINS),
...GITLAB_HOSTS.map(host => `https://${host}`),
...FORGEJO_HOSTS.map(host => `https://${host}`),
...GITEA_HOSTS.map(host => `https://${host}`),
]
6 changes: 2 additions & 4 deletions shared/utils/repository-meta.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ const giteeAdapter: ProviderAdapter = {
}

/**
* Generic Gitea adapter for self-hosted instances.
* Adapter for exact allowlisted Gitea instances.
*/
const giteaAdapter: ProviderAdapter = {
links(ref) {
Expand All @@ -312,8 +312,6 @@ const giteaAdapter: ProviderAdapter = {
async fetchMeta(cachedFetch, ref, links, options = {}) {
if (!ref.host) return null

// Note: Generic Gitea instances may not be in the allowlist,
// so caching may not apply for self-hosted instances
let res: GiteaRepoResponse | null = null
try {
const { data } = await cachedFetch<GiteaRepoResponse>(
Expand Down Expand Up @@ -443,7 +441,7 @@ const radicleAdapter: ProviderAdapter = {
}

/**
* Adapter for explicit Forgejo instances.
* Adapter for exact allowlisted Forgejo instances.
*/
const forgejoAdapter: ProviderAdapter = {
links(ref) {
Expand Down
19 changes: 4 additions & 15 deletions test/nuxt/composables/use-repo-meta.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,26 +191,15 @@ describe('useRepoMeta - URL parsing via repoRef', () => {
})
})

describe('Generic Gitea URLs', () => {
it('should parse git.* subdomain as Gitea', () => {
const result = parseRepoUrl('https://git.example.com/user/repo')

expect(result).toEqual({
provider: 'gitea',
owner: 'user',
repo: 'repo',
host: 'git.example.com',
})
})

it('should parse gitea.* subdomain', () => {
const result = parseRepoUrl('https://gitea.example.org/org/project')
describe('Gitea URLs', () => {
it('should parse exact allowlisted Gitea hosts', () => {
const result = parseRepoUrl('https://gitea.com/org/project')

expect(result).toEqual({
provider: 'gitea',
owner: 'org',
repo: 'project',
host: 'gitea.example.org',
host: 'gitea.com',
})
})
})
Expand Down
27 changes: 9 additions & 18 deletions test/unit/shared/utils/git-providers.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,18 +325,6 @@ describe('parseRepositoryInfo', () => {
})

describe('Forgejo support', () => {
it('parses Forgejo URL from forgejo subdomain', () => {
const result = parseRepositoryInfo({
url: 'https://forgejo.example.com/owner/repo',
})
expect(result).toMatchObject({
provider: 'forgejo',
owner: 'owner',
repo: 'repo',
host: 'forgejo.example.com',
})
})

it('parses Forgejo URL from next.forgejo.org', () => {
const result = parseRepositoryInfo({
url: 'https://next.forgejo.org/forgejo/forgejo',
Expand All @@ -348,16 +336,19 @@ describe('parseRepositoryInfo', () => {
host: 'next.forgejo.org',
})
})
})

it('parses Forgejo URL with .git suffix', () => {
describe('Gitea support', () => {
it('parses exact allowlisted Gitea hosts', () => {
const result = parseRepositoryInfo({
url: 'git+ssh://git@forgejo.myserver.com/user/project.git',
url: 'https://gitea.com/owner/repo',
})
expect(result).toMatchObject({
provider: 'forgejo',
owner: 'user',
repo: 'project',
host: 'forgejo.myserver.com',
provider: 'gitea',
owner: 'owner',
repo: 'repo',
host: 'gitea.com',
rawBaseUrl: 'https://gitea.com/owner/repo/raw/branch/main',
})
Comment on lines +341 to 352
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add explicit deny-path tests for non-allowlisted Gitea/Forgejo-like hosts.

This block only proves the allowlisted success path. Please also assert that lookalike non-allowlisted hosts are rejected, so the SSRF boundary is locked in by tests.

Proposed test addition 
 describe('Gitea support', () => {
   it('parses exact allowlisted Gitea hosts', () => {
     const result = parseRepositoryInfo({
       url: 'https://gitea.com/owner/repo',
     })
     expect(result).toMatchObject({
       provider: 'gitea',
       owner: 'owner',
       repo: 'repo',
       host: 'gitea.com',
       rawBaseUrl: 'https://gitea.com/owner/repo/raw/branch/main',
     })
   })
+
+  it('rejects non-allowlisted Gitea/Forgejo-like hosts', () => {
+    expect(
+      parseRepositoryInfo({ url: 'https://gitea.example.com/owner/repo' }),
+    ).toBeUndefined()
+    expect(
+      parseRepositoryInfo({ url: 'https://forgejo.example.com/owner/repo' }),
+    ).toBeUndefined()
+  })
 })
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
describe('Gitea support', () => {
it('parses exact allowlisted Gitea hosts', () => {
const result = parseRepositoryInfo({
url: 'git+ssh://git@forgejo.myserver.com/user/project.git',
url: 'https://gitea.com/owner/repo',
})
expect(result).toMatchObject({
provider: 'forgejo',
owner: 'user',
repo: 'project',
host: 'forgejo.myserver.com',
provider: 'gitea',
owner: 'owner',
repo: 'repo',
host: 'gitea.com',
rawBaseUrl: 'https://gitea.com/owner/repo/raw/branch/main',
})
describe('Gitea support', () => {
it('parses exact allowlisted Gitea hosts', () => {
const result = parseRepositoryInfo({
url: 'https://gitea.com/owner/repo',
})
expect(result).toMatchObject({
provider: 'gitea',
owner: 'owner',
repo: 'repo',
host: 'gitea.com',
rawBaseUrl: 'https://gitea.com/owner/repo/raw/branch/main',
})
})
it('rejects non-allowlisted Gitea/Forgejo-like hosts', () => {
expect(
parseRepositoryInfo({ url: 'https://gitea.example.com/owner/repo' }),
).toBeUndefined()
expect(
parseRepositoryInfo({ url: 'https://forgejo.example.com/owner/repo' }),
).toBeUndefined()
})
})
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/unit/shared/utils/git-providers.spec.ts` around lines 341 - 352, Add
negative tests alongside the existing Gitea allowlist spec to assert that
lookalike/non-allowlisted hosts are rejected by parseRepositoryInfo: add test
cases (e.g., "rejects non-allowlisted Gitea-like hosts" and "rejects
Forgejo-like hosts") that call parseRepositoryInfo with URLs such as
'https://gitea.com.evil/owner/repo' or 'https://forgejo.example/owner/repo' and
assert it either returns undefined/null or throws (match existing failure
behavior), and verify no provider is set to 'gitea'/'forgejo' and no rawBaseUrl
is produced; locate tests near the existing Gitea block in
test/unit/shared/utils/git-providers.spec.ts and mirror the style/assertion
pattern used for allowlisted success cases.

Copy link
Copy Markdown
Member Author

@serhalp serhalp May 24, 2026

Choose a reason for hiding this comment

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

I thought about that but decided against it. It would be a very strange, porous heuristic test that speculates about what future patterns someone might implement. I think it would only provide false confidence.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 24, 2026

Choose a reason for hiding this comment

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

@serhalp, that's a fair point. Negative tests on a handful of example hostnames wouldn't actually prove the allowlist is exhaustive — they'd just show those specific strings don't match today, which could give a false sense of security if the underlying logic ever drifted. The positive test on the allowlisted host combined with the explicit allowlist constants in the source is the right place to enforce the boundary. Happy to drop this suggestion.

(ᓀ‸ᓂ)

🧠 Learnings used 
Learnt from: ghostdevv
Repo: npmx-dev/npmx.dev PR: 2195
File: test/nuxt/components/Header/MobileMenu.spec.ts:7-11
Timestamp: 2026-03-23T01:33:11.889Z
Learning: When using or mocking the `useConnector` composable in `npmx-dev/npmx.dev`, treat its public API as `isConnected` (not `connected`). Even though the internal `ConnectorState` uses a `connected` field, the composable’s returned object exposes `isConnected: computed(() => state.value.connected)`. Therefore, in tests/components, mocks and usages should reference `isConnected` and should not be flagged as incorrect for using `isConnected`.

Learnt from: serhalp
Repo: npmx-dev/npmx.dev PR: 2459
File: test/unit/server/utils/likes-leaderboard.spec.ts:137-165
Timestamp: 2026-04-26T00:02:20.876Z
Learning: In this Nuxt project (npmx-dev/npmx.dev), the `Packument` type is globally available via Nuxt auto-imports from `shared/types/` (exported from `shared/types/npm-registry.ts`). Therefore, do not raise or require missing `import type { Packument } from '`#shared/types`'` (or any equivalent) when `Packument` is referenced, including in unit test files.

})
})
Expand Down
Loading