mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-09-06 07:29:09 +00:00
feat(git): classify Git host rate-limit responses as their own error state (#1884)
A Git host throttle response (a 429, or a sideband message naming a rate limit or abuse-detection mechanism) previously fell through to AUTH_FAILED or a generic GIT_ERROR depending on the host's exact wording, telling the operator to check a credential that was never the problem. Adds RATE_LIMITED to the transport-facing error model, mapped to HTTP 429, checked ahead of the auth-shaped branches in classifyGitFailure. Proven against a real git binary: git's smart-HTTP client never surfaces the HTTP response body in stderr, only the status line, so a host that throttles via a bare 403 stays indistinguishable from a rejected credential and correctly classifies as AUTH_FAILED; only an explicit 429, or a sideband remote: message naming the throttle, classifies as RATE_LIMITED.
This commit is contained in:
@@ -36,6 +36,10 @@ describe('gitSourceStatus', () => {
|
||||
expect(gitSourceStatus('NETWORK_TIMEOUT')).toBe(504);
|
||||
});
|
||||
|
||||
it('maps RATE_LIMITED to 429, not to the auth or generic status', () => {
|
||||
expect(gitSourceStatus('RATE_LIMITED')).toBe(429);
|
||||
});
|
||||
|
||||
it('maps PLAN_FINGERPRINT_REQUIRED to 400', () => {
|
||||
expect(gitSourceStatus('PLAN_FINGERPRINT_REQUIRED')).toBe(400);
|
||||
});
|
||||
@@ -58,7 +62,7 @@ describe('gitSourceStatus', () => {
|
||||
it('has exactly one explicit mapping for every GitSourceErrorCode', () => {
|
||||
const codes: GitSourceErrorCode[] = [
|
||||
'REPO_NOT_FOUND', 'AUTH_FAILED', 'REF_NOT_FOUND', 'REF_DELETED', 'UNSUPPORTED_REF',
|
||||
'SSH_HOST_KEY_FAILED', 'FILE_NOT_FOUND', 'NETWORK_TIMEOUT', 'GIT_ERROR', 'STALE_PLAN',
|
||||
'SSH_HOST_KEY_FAILED', 'FILE_NOT_FOUND', 'RATE_LIMITED', 'NETWORK_TIMEOUT', 'GIT_ERROR', 'STALE_PLAN',
|
||||
'PLAN_FINGERPRINT_REQUIRED', 'PLAN_BLOCKED', 'LEGACY_PENDING', 'PLAN_UNAVAILABLE',
|
||||
'OPERATION_IN_FLIGHT',
|
||||
];
|
||||
|
||||
@@ -391,8 +391,9 @@ describe('git transport support matrix', () => {
|
||||
});
|
||||
|
||||
describe('rate limiting', () => {
|
||||
it('is documented as a limitation, not a supported claim', () => {
|
||||
expect(limitationIds.has('no-rate-limit-classification')).toBe(true);
|
||||
it('is classified in the error model, not left as a documented limitation', () => {
|
||||
expect(limitationIds.has('no-rate-limit-classification')).toBe(false);
|
||||
expect(support.error_model.some((e) => e.code === 'RATE_LIMITED')).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,109 @@
|
||||
/**
|
||||
* Real end-to-end proof that a Git host throttle response classifies as
|
||||
* RATE_LIMITED, not AUTH_FAILED or GIT_ERROR.
|
||||
*
|
||||
* The other classifier fixtures (git-transport.test.ts) construct stderr
|
||||
* strings by hand; this file proves the string a real git binary actually
|
||||
* produces, since git does not surface an HTTP response body over the
|
||||
* smart-HTTP protocol; only the status line reaches stderr. A server that
|
||||
* intercepts every request before touching a real repository is enough:
|
||||
* resolveRef fails at ls-remote, before any fetch would need real
|
||||
* repository content.
|
||||
*
|
||||
* Soft-skips when the system git binary is unavailable, mirroring the other
|
||||
* native-git integration suites (see __helpers__/externalDeps.ts).
|
||||
*/
|
||||
import { promises as fs, readFileSync } from 'fs';
|
||||
import https from 'https';
|
||||
import os from 'os';
|
||||
import path from 'path';
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it } from 'vitest';
|
||||
import { classifyGitFailure, isTransportFailure } from '../services/git/errors';
|
||||
import { nativeGitTransport } from '../services/git/nativeGitTransport';
|
||||
import { requireGitBinary } from './__helpers__/externalDeps';
|
||||
|
||||
const FIXTURES_DIR = path.resolve(__dirname, '..', '..', '..', 'e2e', 'fixtures');
|
||||
|
||||
/** Throttle wording a real host would put in the response body, which git never shows. */
|
||||
const THROTTLE_BODY = 'You have exceeded a secondary rate limit. Please wait a few minutes before you try again.';
|
||||
|
||||
/** Serve a fixed HTTP status to every request, regardless of path or method. */
|
||||
function serveStatus(status: number): Promise<{ url: string; close: () => void }> {
|
||||
return new Promise((resolve, reject) => {
|
||||
const server = https.createServer(
|
||||
{
|
||||
cert: readFileSync(path.join(FIXTURES_DIR, 'git-server.pem')),
|
||||
key: readFileSync(path.join(FIXTURES_DIR, 'git-server.key')),
|
||||
},
|
||||
(req, res) => {
|
||||
res.statusCode = status;
|
||||
res.end(THROTTLE_BODY);
|
||||
},
|
||||
);
|
||||
server.on('error', reject);
|
||||
server.listen(0, '127.0.0.1', () => {
|
||||
const address = server.address();
|
||||
if (address === null || typeof address === 'string') {
|
||||
reject(new Error('server did not bind'));
|
||||
return;
|
||||
}
|
||||
resolve({ url: `https://127.0.0.1:${address.port}/repo.git`, close: () => server.close() });
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
describe.skipIf(!requireGitBinary())('native git transport rate-limit classification (real git, real TLS)', () => {
|
||||
let prevExtraCaCerts: string | undefined;
|
||||
const workspaces: string[] = [];
|
||||
|
||||
beforeAll(() => {
|
||||
prevExtraCaCerts = process.env.NODE_EXTRA_CA_CERTS;
|
||||
process.env.NODE_EXTRA_CA_CERTS = path.join(FIXTURES_DIR, 'git-ca.pem');
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
if (prevExtraCaCerts === undefined) delete process.env.NODE_EXTRA_CA_CERTS;
|
||||
else process.env.NODE_EXTRA_CA_CERTS = prevExtraCaCerts;
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await Promise.all(workspaces.splice(0).map((w) => fs.rm(w, { recursive: true, force: true })));
|
||||
});
|
||||
|
||||
async function makeWorkspace(): Promise<string> {
|
||||
const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'sencho-git-ratelimit-ws-'));
|
||||
workspaces.push(dir);
|
||||
return dir;
|
||||
}
|
||||
|
||||
// Both statuses are served with the same throttle body, which is the
|
||||
// point of the pair: git shows only the status line, so a host that
|
||||
// signals a throttle as a bare 403 is indistinguishable from a rejected
|
||||
// credential and must still classify as AUTH_FAILED.
|
||||
it.each([
|
||||
[429, 'RATE_LIMITED'],
|
||||
[403, 'AUTH_FAILED'],
|
||||
] as const)('classifies a real %d response from the host as %s', async (status, code) => {
|
||||
const served = await serveStatus(status);
|
||||
try {
|
||||
const workspaceRoot = await makeWorkspace();
|
||||
const failure = await nativeGitTransport
|
||||
.resolveRef({ repoUrl: served.url, ref: 'main', token: 'irrelevant-token', timeoutMs: 15_000, workspaceRoot })
|
||||
.then(() => null, (e: unknown) => e);
|
||||
|
||||
expect(isTransportFailure(failure)).toBe(true);
|
||||
if (!isTransportFailure(failure)) throw new Error('unreachable');
|
||||
expect(failure.reason).toBe('exit');
|
||||
if (failure.reason === 'exit') {
|
||||
// Pins the real stderr shape the classifier's hand-written
|
||||
// fixtures (git-transport.test.ts) assume: the status line
|
||||
// reaches stderr, the served body never does.
|
||||
expect(failure.stderr).toMatch(new RegExp(`requested url returned error:\\s*${status}\\b`, 'i'));
|
||||
expect(failure.stderr).not.toMatch(/secondary rate limit/i);
|
||||
}
|
||||
expect(classifyGitFailure(failure).code).toBe(code);
|
||||
} finally {
|
||||
served.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -219,6 +219,122 @@ describe('classifyGitFailure (native git stderr corpus)', () => {
|
||||
expect(c.code).toBe('UNSUPPORTED_REF');
|
||||
});
|
||||
|
||||
it.each([
|
||||
// The HTTP shapes are verified against a real git binary talking to a
|
||||
// fixture server (git-transport-ratelimit.integration.test.ts): git
|
||||
// reports the status line only, never the response body.
|
||||
['bare 429 from the host', "fatal: unable to access 'https://h/x.git/': The requested URL returned error: 429"],
|
||||
['429 with trailing text', "fatal: unable to access 'https://h/x.git/': The requested URL returned error: 429 Too Many Requests"],
|
||||
// Text a host sends through the pack stream does reach stderr as a
|
||||
// remote: line, unlike an HTTP response body.
|
||||
['remote sideband rate-limit message', "remote: You have exceeded a secondary rate limit. Please wait a few minutes before you try again.\nfatal: the remote end hung up unexpectedly"],
|
||||
['remote sideband abuse-detection message', "remote: You have triggered an abuse detection mechanism.\nfatal: the remote end hung up unexpectedly"],
|
||||
])('classifies %s as RATE_LIMITED', (_label, stderr) => {
|
||||
const c = classifyGitFailure({
|
||||
transportFailure: true as const,
|
||||
reason: 'exit',
|
||||
stderr,
|
||||
exitCode: 128,
|
||||
host: 'github.com',
|
||||
hasToken: true,
|
||||
});
|
||||
expect(c.code).toBe('RATE_LIMITED');
|
||||
expect(c.message).toMatch(/rate limited/i);
|
||||
});
|
||||
|
||||
it('classifies an unambiguous rate limit as RATE_LIMITED even without a token', () => {
|
||||
// Rule 3 (see the module header) takes precedence over rule 2's
|
||||
// no-token private-repo masking: a throttle leaks nothing about repo
|
||||
// existence, so it should not be reported as REPO_NOT_FOUND.
|
||||
const c = classifyGitFailure({
|
||||
transportFailure: true as const,
|
||||
reason: 'exit',
|
||||
stderr: "remote: You have exceeded a secondary rate limit.\nfatal: the remote end hung up unexpectedly",
|
||||
exitCode: 128,
|
||||
host: 'github.com',
|
||||
hasToken: false,
|
||||
});
|
||||
expect(c.code).toBe('RATE_LIMITED');
|
||||
});
|
||||
|
||||
it('does not send a rate-limited operator to rotate a working credential', () => {
|
||||
// A sideband throttle message can arrive alongside a 403 fatal line,
|
||||
// which the auth branch below would otherwise claim. Both the code
|
||||
// and the message are asserted: reporting RATE_LIMITED while still
|
||||
// saying "check your token" would leave the operator with the same
|
||||
// wrong action.
|
||||
const c = classifyGitFailure({
|
||||
transportFailure: true as const,
|
||||
reason: 'exit',
|
||||
stderr: "remote: You have exceeded a secondary rate limit.\nfatal: unable to access 'https://github.com/o/r.git/': The requested URL returned error: 403",
|
||||
exitCode: 128,
|
||||
host: 'github.com',
|
||||
hasToken: true,
|
||||
});
|
||||
expect(c.code).toBe('RATE_LIMITED');
|
||||
expect(c.message).not.toMatch(/check your token/i);
|
||||
});
|
||||
|
||||
it('leaves a bare 403 with no rate-limit wording as an auth failure', () => {
|
||||
// Guards the other direction, and pins a real constraint: git does
|
||||
// not surface an HTTP response body, so a host that signals a
|
||||
// throttle as a bare 403 is indistinguishable from a rejected
|
||||
// credential. Widening the rate-limit branch to cover every 403
|
||||
// would make a genuinely bad token read as a throttle to wait out.
|
||||
const c = classifyGitFailure({
|
||||
transportFailure: true as const,
|
||||
reason: 'exit',
|
||||
stderr: "fatal: unable to access 'https://github.com/o/r.git/': The requested URL returned error: 403",
|
||||
exitCode: 128,
|
||||
host: 'github.com',
|
||||
hasToken: true,
|
||||
});
|
||||
expect(c.code).toBe('AUTH_FAILED');
|
||||
});
|
||||
|
||||
it('does not mistake a repository named "rate-limiter" for a rate-limit signal', () => {
|
||||
// git's fatal line echoes the full repo URL verbatim, so an
|
||||
// unscoped rate-limit word match would fire on the path itself. A
|
||||
// genuinely bad token against a repo whose name happens to contain
|
||||
// rate-limit wording must still classify as an auth failure.
|
||||
const c = classifyGitFailure({
|
||||
transportFailure: true as const,
|
||||
reason: 'exit',
|
||||
stderr: "fatal: unable to access 'https://github.com/acme/rate-limiter.git/': The requested URL returned error: 403",
|
||||
exitCode: 128,
|
||||
host: 'github.com',
|
||||
hasToken: true,
|
||||
});
|
||||
expect(c.code).toBe('AUTH_FAILED');
|
||||
});
|
||||
|
||||
it('does not mistake an upload-pack progress counter for a 429 status', () => {
|
||||
// Progress lines like "Counting objects: 100% (429/429)" reach
|
||||
// stderr from the server sideband and can contain the literal digits
|
||||
// 429 with no connection to an HTTP status at all.
|
||||
const c = classifyGitFailure({
|
||||
transportFailure: true as const,
|
||||
reason: 'exit',
|
||||
stderr: "remote: Counting objects: 100% (429/429), done.\nfatal: the remote end hung up unexpectedly",
|
||||
exitCode: 128,
|
||||
host: 'github.com',
|
||||
hasToken: false,
|
||||
});
|
||||
expect(c.code).toBe('NETWORK_TIMEOUT');
|
||||
});
|
||||
|
||||
it('does not mistake an unrelated transient-error sideband for a rate limit', () => {
|
||||
const c = classifyGitFailure({
|
||||
transportFailure: true as const,
|
||||
reason: 'exit',
|
||||
stderr: "remote: Internal server error, please retry later\nfatal: the remote end hung up unexpectedly",
|
||||
exitCode: 128,
|
||||
host: 'github.com',
|
||||
hasToken: false,
|
||||
});
|
||||
expect(c.code).toBe('NETWORK_TIMEOUT');
|
||||
});
|
||||
|
||||
it('scrubs credentials from the generic fallback tail', () => {
|
||||
const c = classifyGitFailure({
|
||||
transportFailure: true as const,
|
||||
|
||||
@@ -68,6 +68,7 @@ export type GitSourceErrorCode =
|
||||
| 'UNSUPPORTED_REF'
|
||||
| 'SSH_HOST_KEY_FAILED'
|
||||
| 'FILE_NOT_FOUND'
|
||||
| 'RATE_LIMITED'
|
||||
| 'NETWORK_TIMEOUT'
|
||||
| 'GIT_ERROR'
|
||||
| 'STALE_PLAN'
|
||||
|
||||
@@ -7,12 +7,17 @@
|
||||
* (service -> git/*) and the classifier is unit-testable in isolation. The
|
||||
* service wraps the returned pair in its own GitSourceError.
|
||||
*
|
||||
* Two behaviors are contractual and pinned by tests; do not change them:
|
||||
* Three behaviors are contractual and pinned by tests; do not change them:
|
||||
* 1. Authentication failure WITH a supplied token reports AUTH_FAILED,
|
||||
* which the HTTP layer maps to 400, never 401, because the frontend's
|
||||
* global logout trips on any API-level 401.
|
||||
* 2. A 401/403-shaped refusal WITHOUT a token reports REPO_NOT_FOUND with
|
||||
* a private-repo hint, mirroring GitHub's masking of private repos.
|
||||
* a private-repo hint, mirroring GitHub's masking of private repos,
|
||||
* unless rule 3 claims it first.
|
||||
* 3. An unambiguous rate-limit signal (a 429 status, or a server sideband
|
||||
* line naming a throttle) reports RATE_LIMITED ahead of rules 1 and 2:
|
||||
* a throttle leaks nothing about repo existence, so the rule-2 masking
|
||||
* does not apply to it even when no token was supplied.
|
||||
*/
|
||||
|
||||
export type TransportFacingCode =
|
||||
@@ -21,6 +26,7 @@ export type TransportFacingCode =
|
||||
| 'SSH_HOST_KEY_FAILED'
|
||||
| 'REF_NOT_FOUND'
|
||||
| 'UNSUPPORTED_REF'
|
||||
| 'RATE_LIMITED'
|
||||
| 'NETWORK_TIMEOUT'
|
||||
| 'GIT_ERROR';
|
||||
|
||||
@@ -157,6 +163,26 @@ export function classifyGitFailure(
|
||||
};
|
||||
}
|
||||
|
||||
// Rate limiting is checked ahead of the auth-shaped branches below,
|
||||
// including the WITHOUT-a-token masking rule: a throttle is a throttle
|
||||
// regardless of credentials. Without this branch a throttled fetch
|
||||
// reaches the auth branch's /\b40[13]\b/ and tells the operator to rotate
|
||||
// a credential that is not the problem.
|
||||
//
|
||||
// Both patterns stay narrow, because git's fatal line echoes the repo URL
|
||||
// verbatim and sideband lines (always prefixed "remote:") carry arbitrary
|
||||
// server text: a bare \b429\b would also match an upload-pack progress
|
||||
// counter ("Counting objects: 100% (429/429)") or a number in the URL,
|
||||
// and an unanchored word match would fire on a repo named "rate-limiter".
|
||||
// Sideband wording is limited to throttle-specific phrases, since a
|
||||
// generic "retry later" also describes a transient 5xx.
|
||||
if (/returned error:\s*429\b/.test(raw) || /^remote:.*(too many requests|rate[ -]limit|abuse detection)/m.test(raw)) {
|
||||
return {
|
||||
code: 'RATE_LIMITED',
|
||||
message: `Rate limited by${dest}. Too many requests were sent in a short window. Wait a few minutes and retry.`,
|
||||
};
|
||||
}
|
||||
|
||||
// Auth-shaped refusals. Native git phrases these two ways: with a token
|
||||
// it gets "Authentication failed for '<url>'"; without one it cannot even
|
||||
// answer and reports the disabled terminal prompt.
|
||||
|
||||
@@ -34,6 +34,8 @@ export function gitSourceStatus(code: GitSourceErrorCode): number {
|
||||
case 'PLAN_UNAVAILABLE':
|
||||
case 'OPERATION_IN_FLIGHT':
|
||||
return 409;
|
||||
case 'RATE_LIMITED':
|
||||
return 429;
|
||||
case 'NETWORK_TIMEOUT':
|
||||
return 504;
|
||||
case 'GIT_ERROR':
|
||||
|
||||
@@ -57,7 +57,6 @@ Every "Supported" row here is backed by a test that runs on every change to Senc
|
||||
|
||||
## Not supported
|
||||
|
||||
- **No rate-limit classification.** A Git host rate-limit response (for example GitHub's secondary rate limits) is not classified as its own error state. It surfaces as an authentication failure or a generic transport error depending on the host's exact response. Wait and retry; there is no dedicated rate-limit message or backoff guidance yet.
|
||||
- **No Git LFS.** Compose and env files tracked via Git LFS are rejected rather than silently fetched as pointer stubs. Commit plain files instead.
|
||||
- **No submodules.** Submodule contents are not fetched. Inputs and build contexts that reference submodule contents are refused with an actionable message; a warning is shown when .gitmodules is present.
|
||||
- **No sparse or partial clone.** Every fetch materializes the complete repository at the resolved commit (shallow, single-branch); there is no sparse or partial clone for large monorepos.
|
||||
|
||||
@@ -549,10 +549,6 @@ claims:
|
||||
attestation: att-2026-09-01-github-pilot
|
||||
|
||||
limitations:
|
||||
- id: no-rate-limit-classification
|
||||
title: No rate-limit classification
|
||||
statement: A Git host rate-limit response (for example GitHub's secondary rate limits) is not classified as its own error state. It surfaces as an authentication failure or a generic transport error depending on the host's exact response. Wait and retry; there is no dedicated rate-limit message or backoff guidance yet.
|
||||
|
||||
- id: no-git-lfs
|
||||
title: No Git LFS
|
||||
statement: Compose and env files tracked via Git LFS are rejected rather than silently fetched as pointer stubs. Commit plain files instead.
|
||||
@@ -606,6 +602,10 @@ error_model:
|
||||
label: Commit not reachable on this host
|
||||
status: 400
|
||||
meaning: A pinned commit SHA that the Git host will not serve because it is not advertised by any branch or tag tip.
|
||||
- code: RATE_LIMITED
|
||||
label: Rate limited
|
||||
status: 429
|
||||
meaning: The Git host throttled the request (an HTTP 429, or a sideband message naming a rate limit or abuse-detection mechanism). Wait and retry; there is no automated backoff yet.
|
||||
- code: NETWORK_TIMEOUT
|
||||
label: Network timeout
|
||||
status: 504
|
||||
|
||||
Reference in New Issue
Block a user