mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-09-01 21:27:58 +00:00
feat(git): swap isomorphic-git for native git transport behind clone seam (#1849)
* feat(git): swap isomorphic-git for native git transport behind clone seam Replace the isomorphic-git engine (HTTP-only, single importer) with the native git CLI behind the existing withClonedRepo seam, so SSH deploy keys, ref semantics, and private CAs become reachable in later PRs. - resolve-before-fetch: ls-remote pins the branch to an immutable SHA, then rev-parse verifies the checkout against it; tip races refuse - hardened spawns: argv arrays only, protocol allowlist (https only), neutralized hooks, isolated HOME and all config channels, no prompts - token reaches git only via a credential helper reading SENCHO_GIT_TOKEN from the child env; never argv or URL - size cap becomes a workspace watchdog (on-disk measure) keeping the same knob and breach message; deterministic final gate added - Windows: pin http.sslBackend=openssl (schannel ignores sslCAInfo) and anchor to Git's bundled CA; NODE_EXTRA_CA_CERTS combines with platform defaults instead of replacing them - error classification retargets to exit code + stderr while preserving the contractual mappings (AUTH_FAILED maps to 400, never 401; unauthenticated refusals mask as REPO_NOT_FOUND) - runtime image installs git; tests re-pointed at the transport boundary plus a new engine suite (classifier corpus, argv hardening, watchdog) Zero externally visible behavior change except two edge cases: an empty branch now surfaces BRANCH_NOT_FOUND, and a mid-fetch force push refuses instead of materializing the moved tip. * fix(git): unblock CI on linux kill-path test and codeql log warning Two CI-only findings from the first pipeline run: - The scripted spawn child in the transport tests lacked the kill method that killTree's POSIX fallback reaches when a fake process group does not exist; Linux runs crashed inside the timeout tests while Windows (taskkill branch) could not reproduce it. Give the fixture the method the real ChildProcess always has. - CodeQL flagged the workspace-removal warning that interpolated the NODE_EXTRA_CA_CERTS path (environment-sourced values are treated as sensitive at log sinks). Reword the warning to name the variable instead of its value; operators know their own environment. * fix(git): collapse remaining duplicated test setup so the shared helper is used * fix(git): close watchdog, size-gate, ref-validator, and kill-ordering gaps in native transport Resolves the release-blocking findings from an independent pre-merge audit of the native git transport swap: - A watchdog-triggered kill mid-clone was misclassified as a generic exit failure instead of a size breach, because runGit resolves (not rejects) when the child is killed via SIGKILL. - The final on-disk size measurement failed open when it could not be read (workspace removed mid-walk, permissions), letting an unmeasured clone through as a success. Now fails closed and logs the real cause. - The ref-name validator was an overly restrictive allow-list that rejected valid branch names (leading underscore, non-ASCII, '#'). Replaced with a deny-list matching real `git check-ref-format --branch` semantics, verified against the git binary, including a per-path-segment `.lock` check the first pass missed. - runGit's timeout handler settled as soon as a kill was issued rather than confirmed, racing workspace cleanup against a still-alive child tree. It now waits for the child's close event, with a bounded fallback if termination is never confirmed, and preserves the timeout classification if 'error' fires after the kill. - Windows killTree now also falls back to child.kill() when taskkill itself exits non-zero, not just when it fails to spawn. - Added a real, non-mocked integration test that drives the credential helper through the actual git binary against a local HTTPS server with Basic Auth checking. It caught a genuine bug the mocked suite could not see: the credential.helper config value was quoted in a way that broke git's own absolute-path helper detection, failing every authenticated clone. Fixed by removing the quotes. - Migrated a separately developed test file's mocks off the deleted isomorphic-git module onto the native transport seam, matching the pattern already used elsewhere, after merging with main pulled in that feature. Also updates two stale comments left over from the isomorphic-git era and adds a git version check to the Docker runtime image smoke tests. * fix(git): make credential-helper path safe, unify ref length, and fix Windows kill ordering Addresses three PR 1 correction items from pre-merge audit: - credential.helper is a shell string, not argv: interpolating the helper's workspace-relative path broke authenticated fetches whenever the workspace sat under a directory with a space in its name. The config value is now a fixed string that names an environment variable instead, so no workspace path character can affect how git's shell parses it. - The transport rejected branch names over 200 characters while the route accepted up to 256 and real git has no comparable limit. REF_MAX_LEN is now a single exported constant shared by the transport and both routes. - On Windows, taskkill runs as a separate process and could still be walking a killed process tree after the direct git child reported closed, letting the caller delete the workspace early. Kill operations are now awaited to completion (bounded by a timeout) before a timed-out or size-breached run settles, on both the close and error event paths. Verified against a real authenticated git server inside the built runtime image: public HTTPS, private HTTPS with a valid PAT, invalid PAT, a deleted branch, an oversized repository, and the awkward workspace-path case, including from a workspace path containing spaces and shell metacharacters. * fix(git): reap killed helpers and classify curl refusals
This commit is contained in:
Anso
@@ -0,0 +1,188 @@
|
||||
/**
|
||||
* Classification of native-git failures into the public GitSourceErrorCode
|
||||
* contract.
|
||||
*
|
||||
* Deliberately class-free: this module returns plain {code, message} data and
|
||||
* never imports GitSourceService, so the dependency graph stays one-way
|
||||
* (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:
|
||||
* 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.
|
||||
*/
|
||||
|
||||
export type TransportFacingCode =
|
||||
| 'REPO_NOT_FOUND'
|
||||
| 'AUTH_FAILED'
|
||||
| 'BRANCH_NOT_FOUND'
|
||||
| 'NETWORK_TIMEOUT'
|
||||
| 'GIT_ERROR';
|
||||
|
||||
/** Structured failure raised by the native transport; classified below. */
|
||||
export type TransportFailureReason =
|
||||
| 'invalid-url'
|
||||
| 'invalid-ref'
|
||||
| 'git-missing'
|
||||
| 'git-old'
|
||||
| 'ref-not-found'
|
||||
| 'tip-changed'
|
||||
| 'size'
|
||||
| 'timeout'
|
||||
| 'exit';
|
||||
|
||||
interface TransportFailureBase {
|
||||
/** Branded discriminant so isTransportFailure cannot false-positive on foreign errors. */
|
||||
readonly transportFailure: true;
|
||||
host: string;
|
||||
hasToken: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Discriminated on `reason`: each variant carries exactly the payload its
|
||||
* classifier branch needs (e.g. `size` must always know `maxBytes`, so the
|
||||
* operator-facing breach message can never render "0 B").
|
||||
*/
|
||||
export type TransportFailure = TransportFailureBase & (
|
||||
| { reason: 'invalid-url' }
|
||||
| { reason: 'invalid-ref' }
|
||||
| { reason: 'git-missing'; stderr?: string }
|
||||
| { reason: 'git-old'; stderr?: string }
|
||||
| { reason: 'ref-not-found' }
|
||||
| { reason: 'tip-changed' }
|
||||
| { reason: 'size'; maxBytes: number }
|
||||
| { reason: 'timeout' }
|
||||
| { reason: 'exit'; stderr?: string; exitCode?: number; /** Full child argv, attached for debug diagnostics only. */ argv?: string[] }
|
||||
);
|
||||
|
||||
/**
|
||||
* Defensive redaction mirroring GitSourceService.scrubCredentials. Kept local
|
||||
* instead of imported to preserve the one-way dependency direction; git never
|
||||
* receives credentials via URL, so this only guards against operators pasting
|
||||
* user:pass@ URLs that servers echo back in error text.
|
||||
*/
|
||||
function redactCredentials(text: string): string {
|
||||
return text
|
||||
.replace(/https?:\/\/[^/\s:@]+:[^/\s@]+@/gi, 'https://***:***@')
|
||||
.replace(/(authorization[:=]\s*)[^\s,;]+/gi, '$1***')
|
||||
.replace(/(token[:=]\s*)[^\s,;]+/gi, '$1***')
|
||||
.replace(/(password[:=]\s*)[^\s,;]+/gi, '$1***');
|
||||
}
|
||||
|
||||
function formatBytes(bytes: number): string {
|
||||
if (bytes >= 1024 * 1024) return `${Math.round(bytes / (1024 * 1024))} MB`;
|
||||
if (bytes >= 1024) return `${Math.round(bytes / 1024)} KB`;
|
||||
return `${bytes} B`;
|
||||
}
|
||||
|
||||
function hostQualifier(host: string): string {
|
||||
return host && host !== 'unknown' ? ` ${host}` : ' the repository host';
|
||||
}
|
||||
|
||||
/** Shared hint for refusals where a private repo and a missing one are indistinguishable. */
|
||||
const PRIVATE_REPO_HINT = 'Repository not found, or it is private. Add a Personal Access Token if the repo is private.';
|
||||
|
||||
/** Last few stderr lines, scrubbed, for the generic GIT_ERROR fallback. */
|
||||
function stderrTail(stderr: string | undefined): string {
|
||||
if (!stderr) return '';
|
||||
const lines = redactCredentials(stderr).trim().split(/\r?\n/).filter(Boolean);
|
||||
return lines.slice(-3).join(' ').slice(0, 400);
|
||||
}
|
||||
|
||||
export function classifyGitFailure(
|
||||
failure: TransportFailure,
|
||||
): { code: TransportFacingCode; message: string } {
|
||||
const dest = hostQualifier(failure.host);
|
||||
|
||||
// Structured outcomes decided by the transport itself, before any
|
||||
// stderr guessing.
|
||||
switch (failure.reason) {
|
||||
case 'invalid-url':
|
||||
return { code: 'GIT_ERROR', message: 'Unsupported repository URL. Use an https:// URL without embedded credentials.' };
|
||||
case 'invalid-ref':
|
||||
return { code: 'GIT_ERROR', message: 'Unsupported branch name. Use the branch name as the remote reports it.' };
|
||||
case 'git-missing':
|
||||
return { code: 'GIT_ERROR', message: failure.stderr || 'The git command was not found on PATH.' };
|
||||
case 'git-old':
|
||||
return { code: 'GIT_ERROR', message: failure.stderr || 'The installed git client is too old.' };
|
||||
case 'ref-not-found':
|
||||
return { code: 'BRANCH_NOT_FOUND', message: 'Branch not found in the repository.' };
|
||||
case 'tip-changed':
|
||||
return { code: 'GIT_ERROR', message: 'Repository tip changed during fetch; retry the pull.' };
|
||||
case 'size':
|
||||
return {
|
||||
code: 'GIT_ERROR',
|
||||
message: `Repository exceeds the maximum clone size of ${formatBytes(failure.maxBytes)}.`,
|
||||
};
|
||||
case 'timeout':
|
||||
return { code: 'NETWORK_TIMEOUT', message: `Timed out reaching${dest}.` };
|
||||
default:
|
||||
break;
|
||||
}
|
||||
|
||||
const raw = redactCredentials((failure.stderr ?? '').toLowerCase());
|
||||
|
||||
// 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.
|
||||
if (/could not read username|could not read password/.test(raw)) {
|
||||
// Prompting was suppressed, so the host refused the credentials it
|
||||
// was given (possibly none): mask like GitHub hides private repos.
|
||||
return {
|
||||
code: 'REPO_NOT_FOUND',
|
||||
message: PRIVATE_REPO_HINT,
|
||||
};
|
||||
}
|
||||
if (/authentication failed|\b40[13]\b/.test(raw)) {
|
||||
return failure.hasToken
|
||||
? { code: 'AUTH_FAILED', message: 'Repository authentication failed. Check your token.' }
|
||||
: {
|
||||
code: 'REPO_NOT_FOUND',
|
||||
message: PRIVATE_REPO_HINT,
|
||||
};
|
||||
}
|
||||
if (/remote branch .+ not found in upstream|branch not found/.test(raw)) {
|
||||
return { code: 'BRANCH_NOT_FOUND', message: 'Branch not found in the repository.' };
|
||||
}
|
||||
if (/repository[\s\S]*\bnot found\b|not found in upstream/.test(raw)) {
|
||||
return {
|
||||
code: 'REPO_NOT_FOUND',
|
||||
message: failure.hasToken
|
||||
? 'Repository not found. Verify the URL and that your token has read access to this repo.'
|
||||
: PRIVATE_REPO_HINT,
|
||||
};
|
||||
}
|
||||
|
||||
// TLS failures before generic network wording, so certificate problems do
|
||||
// not read as connectivity problems.
|
||||
if (/ssl certificate problem|server certificate verification failed|certificate subject name|unable to get local issuer certificate|self[- ]signed certificate/.test(raw)) {
|
||||
return { code: 'GIT_ERROR', message: `TLS certificate error reaching${dest}. The host certificate could not be verified.` };
|
||||
}
|
||||
|
||||
// Network family.
|
||||
if (/could not resolve host|name or service not known|temporary failure in name resolution/.test(raw)) {
|
||||
return { code: 'NETWORK_TIMEOUT', message: `Could not resolve${dest}. Check the repository URL and your network or DNS.` };
|
||||
}
|
||||
if (/connection refused|could not connect to server/.test(raw)) {
|
||||
return { code: 'NETWORK_TIMEOUT', message: `Connection refused by${dest}.` };
|
||||
}
|
||||
if (/connection timed out|operation timed out|connection was reset|remote end hung up|connection reset by peer/.test(raw)) {
|
||||
return { code: 'NETWORK_TIMEOUT', message: `Connection to${dest} failed. Retry; if it persists, check the host or your network.` };
|
||||
}
|
||||
|
||||
const tail = stderrTail(failure.stderr);
|
||||
return { code: 'GIT_ERROR', message: tail ? `Git fetch failed: ${tail}` : 'Git fetch failed.' };
|
||||
}
|
||||
|
||||
/**
|
||||
* Structural type guard. The branded `transportFailure` discriminant makes
|
||||
* false positives impossible: arbitrary errors that happen to carry
|
||||
* reason/host fields are not mistaken for transport failures at the service
|
||||
* boundary.
|
||||
*/
|
||||
export function isTransportFailure(e: unknown): e is TransportFailure {
|
||||
return typeof e === 'object' && e !== null && (e as { transportFailure?: unknown }).transportFailure === true;
|
||||
}
|
||||
Reference in New Issue
Block a user