Skip to content

Commit 854f6b9

Browse files
loks0nclaude
andcommitted
fix: address review — encode refs, handle GitHub auth errors, test no-ref path
- encode ref in GitHub/Gitea archive URLs (preserve slashes for nested branches) - map GitHub 401/403 to a clear access-denied error - document the opt-in @throws on the base getRepositoryPresignedUrl - test the Gitea default-branch resolution (no-ref) path Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 109c424 commit 854f6b9

4 files changed

Lines changed: 21 additions & 2 deletions

File tree

src/VCS/Adapter.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,8 @@ abstract public function getLatestCommit(string $owner, string $repositoryName,
377377
* @param string $ref Branch, tag or commit to download (defaults to the default branch)
378378
* @param string $format Archive format, e.g. 'tarball' or 'zipball'
379379
* @return string Presigned download URL
380+
*
381+
* @throws Exception when the adapter does not implement it (opt-in, mirrors createCheckRun())
380382
*/
381383
public function getRepositoryPresignedUrl(string $owner, string $repositoryName, string $ref = '', string $format = 'tarball'): string
382384
{

src/VCS/Adapter/Git/GitHub.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -892,7 +892,8 @@ public function getRepositoryPresignedUrl(string $owner, string $repositoryName,
892892

893893
$url = "/repos/$owner/$repositoryName/$format";
894894
if (!empty($ref)) {
895-
$url .= "/$ref";
895+
// Encode the ref but keep slashes so nested branch names (e.g. feature/foo) still resolve
896+
$url .= '/' . \str_replace('%2F', '/', \rawurlencode($ref));
896897
}
897898

898899
$response = $this->call(self::METHOD_GET, $url, ['Authorization' => "Bearer $this->accessToken"], [], false, false);
@@ -902,6 +903,9 @@ public function getRepositoryPresignedUrl(string $owner, string $repositoryName,
902903
if ($responseHeadersStatusCode === 404) {
903904
throw new RepositoryNotFound("Repository or ref not found.");
904905
}
906+
if ($responseHeadersStatusCode === 401 || $responseHeadersStatusCode === 403) {
907+
throw new Exception("Access denied to repository archive; check the access token and its permissions.", $responseHeadersStatusCode);
908+
}
905909

906910
$presignedUrl = $responseHeaders['location'] ?? '';
907911
if (empty($presignedUrl)) {

src/VCS/Adapter/Git/Gitea.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,10 @@ public function getRepositoryPresignedUrl(string $owner, string $repositoryName,
290290
}
291291
}
292292

293-
return "{$this->endpoint}/repos/{$owner}/{$repositoryName}/archive/{$ref}.{$extension}?token=" . urlencode($this->accessToken);
293+
// Encode the ref but keep slashes so nested branch names (e.g. feature/foo) still resolve
294+
$encodedRef = \str_replace('%2F', '/', \rawurlencode($ref));
295+
296+
return "{$this->endpoint}/repos/{$owner}/{$repositoryName}/archive/{$encodedRef}.{$extension}?token=" . urlencode($this->accessToken);
294297
}
295298

296299
public function getRepositoryName(string $repositoryId): string

tests/VCS/Adapter/GiteaTest.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,16 @@ public function testGetRepositoryPresignedUrl(): void
9191
$zip = $adapter->getRepositoryPresignedUrl($owner, 'some-repo', static::$defaultBranch, 'zipball');
9292
$this->assertStringContainsString('.zip?token=', $zip);
9393

94+
// No ref: the default branch is resolved from the repository
95+
$repositoryName = 'test-presigned-url-' . \uniqid();
96+
$adapter->createRepository($owner, $repositoryName, false);
97+
try {
98+
$noRef = $adapter->getRepositoryPresignedUrl($owner, $repositoryName);
99+
$this->assertStringContainsString("/archive/" . static::$defaultBranch . '.tar.gz?token=', $noRef);
100+
} finally {
101+
$adapter->deleteRepository($owner, $repositoryName);
102+
}
103+
94104
$this->expectException(\Exception::class);
95105
$adapter->getRepositoryPresignedUrl($owner, 'some-repo', static::$defaultBranch, 'invalid');
96106
}

0 commit comments

Comments
 (0)