Skip to content

feat: enhance repository connection error handling and add public address resolution - #1114

Open
CasLubbers wants to merge 4 commits into
mainfrom
APL-2112
Open

CasLubbers wants to merge 4 commits into
mainfrom
APL-2112

Conversation

@CasLubbers

Copy link
Copy Markdown
Contributor

No description provided.

…ress resolution

Signed-off-by: Cas Lubbers <clubbers@akamai.com>
Copilot AI balanced review requested due to automatic review settings October 9, 2026 13:10

Copilot AI left a comment

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.

🟡 Changes recommended

DNS-rebinding and IPv6 bypasses undermine the protection, while raw Git errors may leak credentials.

4 open findings
What changed in this PR

Adds repository SSRF protections and standardizes connection failure responses.

Changes:

  • Rejects repository hosts resolving to restricted addresses.
  • Disables Git HTTP redirects.
  • Returns generic connection errors while logging details.
File Description
src/​utils/​codeRepoUtils.ts Adds DNS address validation and redirect protection.
src/​otomi-stack.ts Standardizes repository connection errors.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/otomi-stack.ts
Comment thread src/utils/codeRepoUtils.ts Outdated
const hostname = isSSH
? normalizedUrl.slice('git@'.length, normalizedUrl.indexOf(':'))
: new URL(normalizedUrl).hostname
await assertResolvesToPublicAddress(hostname)
Comment on lines +149 to +152
export async function assertResolvesToPublicAddress(hostname: string): Promise<void> {
let addresses: { address: string; family: number }[]
try {
addresses = await lookup(hostname, { all: true })
Signed-off-by: Cas Lubbers <clubbers@akamai.com>
Copilot AI balanced review requested due to automatic review settings October 9, 2026 13:32

Copilot AI left a comment

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.

🔵 Needs a closer look

The address validation remains vulnerable to DNS rebinding and incompletely handles IPv6 and internal SSH repositories.

4 open findings
Previously missed (1)

In code that hasn't changed since last review

Medium severity Recognize internal Gitea SSH URLs before private-IP rejection

src/​utils/​codeRepoUtils.ts:220

Internal Gitea SSH URLs regress here: isInternalGiteaUrl parses with new URL, so git@gitea.<domain>:org/repo.git is never recognized as internal and its private cluster address is rejected. getAuthenticatedGitClient explicitly supports SSH credentials, so derive the hostname first and apply the internal-host exemption independent of URL syntax.

🧠 Review effort: Balanced

…ress resolution

Signed-off-by: Cas Lubbers <clubbers@akamai.com>
Copilot AI balanced review requested due to automatic review settings October 9, 2026 14:20

Copilot AI left a comment

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.

🟡 Changes recommended

DNS rebinding can bypass the SSRF protection, and invalid CIDRs fail open.

4 open findings
1 resolved since last review

🧠 Review effort: Balanced

Comment on lines +116 to +130
// Fully admin-controlled via CODE_REPO_BLOCKED_CIDRS
function isBlockedAddress(address: string): boolean {
const addr = ipaddr.process(address) // unwraps IPv4-mapped IPv6 (::ffff:a.b.c.d) to plain IPv4
return env.CODE_REPO_BLOCKED_CIDRS.split(',')
.map((cidr) => cidr.trim())
.filter(Boolean)
.some((cidr) => {
try {
const [rangeAddr, bits] = ipaddr.parseCIDR(cidr)
return addr.kind() === rangeAddr.kind() && addr.match(rangeAddr, bits)
} catch {
return false
}
})
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants