fix: comprehensive security and robustness hardening for registry feature#407
Merged
fix: comprehensive security and robustness hardening for registry feature#407
Conversation
Bitbucket Server uses ssh://git@host:port/path format which was rejected by the URL validator. Only SCP-style (git@host:path) and https:// were accepted. This caused users to fall back to the SCP format with a port number, which git misinterprets as a path — leading to password prompts instead of SSH key auth. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Trim whitespace and trailing slashes from git URLs before validation - Reject URLs with embedded credentials (user:pass@host or token@host) and direct users to SSH keys / git credential helpers - Add sanitizeUrl() helper (exported) that masks credentials in log output - Enforce min/max length (2–64 chars) on registry names - Create ~/.libscope/ with mode 0o700 and chmod config.json to 0o600 after write - Fix deriveNameFromUrl() to handle ssh:// URLs via URL parser before falling back to SCP regex - Validate derived registry name before proceeding in `registry add`; give clear error with --name hint Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…k, and dedup to registry publish - validatePathSegment(): rejects empty, path separators, .., null bytes, and chars outside [a-zA-Z0-9._-]; called on pack.name and version in publishPack(), and on packName + version in unpublishPack() - validateSemver(): enforces ^\d+\.\d+\.\d+(-[a-zA-Z0-9._-]+)?$ pattern, called after final version is determined in publishPack() - 50 MB pack data size guard (JSON.stringify length) with clear error - index.json corruption now throws ValidationError instead of silently resetting to [], preserving data integrity - publish operations wrapped in try/catch; versionDir is removed on any failure to prevent orphaned directories blocking future publishes - duplicate index entries deduplicated by pack name (keep first, log warning) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add `verifyResolvedPackChecksum()` to `src/registry/resolve.ts` that reads the expected SHA-256 from the pack manifest and calls `verifyChecksum()` on the cached data file before any installation. The CLI now calls this function immediately before both `installPack()` sites that use a registry-resolved data path, ensuring tampered or corrupted packs are rejected with a clear error. Update `tests/integration/registry/registry-conflict.test.ts` to write real SHA-256 checksums in `createBareRepoWithPacks` instead of the "placeholder" string, so the integration tests remain valid under the new verification. Add 5 unit tests for `verifyResolvedPackChecksum` covering: matching checksum (passes), tampered file (throws), missing manifest (skips with warning), empty checksum in manifest (throws), and version not found in manifest (skips). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Runtime validation of index.json entries (type guard for PackSummary) - Symlink protection on git clone (core.symlinks=false) - Corrupted cache detection and auto-recovery in fetchRegistry - File-based sync locking to prevent concurrent operations - Configurable git timeout via LIBSCOPE_GIT_TIMEOUT_MS env var - origin/HEAD resolution fallback (tries main, then master) - Better diagnostic messages for git auth/network/timeout errors Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ening # Conflicts: # src/registry/config.ts
QA FAIL fixes: - resolve.ts: validate packName and version with validatePathSegment before path construction (prevents path traversal via malicious index.json) - publish.ts: clear index cache after fetchRegistry in publish/unpublish (prevents stale index reads within same process) - publish.ts: export validatePathSegment for use by resolve.ts QA WARN fixes: - git.ts: persist core.symlinks=false via git config in fetchRegistry (not just at clone time via -c flag) New tests (46 added, 201 total): - config.test.ts: name length limits, URL trimming, credential rejection, ssh:// URLs, sanitizeUrl, file permissions - git.test.ts: runtime type validation, symlink protection, cache corruption, HEAD fallback, timeout config - publish.test.ts: path traversal, semver, corrupt index, size limit, dedup, rollback, re-publish - sync.test.ts: locking, stale lock cleanup Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
…rdening # Conflicts: # src/registry/config.ts
- Replace || with ?? for nullish coalescing in GIT_TIMEOUT_MS (src/registry/git.ts:19) - Remove async keyword from isNeuralModelAvailable since it has no await expressions (tests/integration/retrieval-quality.test.ts:513) These changes fix the lint-and-typecheck job failures on PR #407. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
RobertLD
added a commit
that referenced
this pull request
Mar 18, 2026
* fix: accept ssh:// protocol URLs in registry URL validation Bitbucket Server uses ssh://git@host:port/path format which was rejected by the URL validator. Only SCP-style (git@host:path) and https:// were accepted. This caused users to fall back to the SCP format with a port number, which git misinterprets as a path — leading to password prompts instead of SSH key auth. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: harden registry config validation and file security - Trim whitespace and trailing slashes from git URLs before validation - Reject URLs with embedded credentials (user:pass@host or token@host) and direct users to SSH keys / git credential helpers - Add sanitizeUrl() helper (exported) that masks credentials in log output - Enforce min/max length (2–64 chars) on registry names - Create ~/.libscope/ with mode 0o700 and chmod config.json to 0o600 after write - Fix deriveNameFromUrl() to handle ssh:// URLs via URL parser before falling back to SCP regex - Validate derived registry name before proceeding in `registry add`; give clear error with --name hint Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add path traversal validation, semver check, size limit, rollback, and dedup to registry publish - validatePathSegment(): rejects empty, path separators, .., null bytes, and chars outside [a-zA-Z0-9._-]; called on pack.name and version in publishPack(), and on packName + version in unpublishPack() - validateSemver(): enforces ^\d+\.\d+\.\d+(-[a-zA-Z0-9._-]+)?$ pattern, called after final version is determined in publishPack() - 50 MB pack data size guard (JSON.stringify length) with clear error - index.json corruption now throws ValidationError instead of silently resetting to [], preserving data integrity - publish operations wrapped in try/catch; versionDir is removed on any failure to prevent orphaned directories blocking future publishes - duplicate index entries deduplicated by pack name (keep first, log warning) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: verify pack checksum before installing from registry cache Add `verifyResolvedPackChecksum()` to `src/registry/resolve.ts` that reads the expected SHA-256 from the pack manifest and calls `verifyChecksum()` on the cached data file before any installation. The CLI now calls this function immediately before both `installPack()` sites that use a registry-resolved data path, ensuring tampered or corrupted packs are rejected with a clear error. Update `tests/integration/registry/registry-conflict.test.ts` to write real SHA-256 checksums in `createBareRepoWithPacks` instead of the "placeholder" string, so the integration tests remain valid under the new verification. Add 5 unit tests for `verifyResolvedPackChecksum` covering: matching checksum (passes), tampered file (throws), missing manifest (skips with warning), empty checksum in manifest (throws), and version not found in manifest (skips). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: harden registry git operations and sync locking - Runtime validation of index.json entries (type guard for PackSummary) - Symlink protection on git clone (core.symlinks=false) - Corrupted cache detection and auto-recovery in fetchRegistry - File-based sync locking to prevent concurrent operations - Configurable git timeout via LIBSCOPE_GIT_TIMEOUT_MS env var - origin/HEAD resolution fallback (tries main, then master) - Better diagnostic messages for git auth/network/timeout errors Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address QA findings and add comprehensive audit tests QA FAIL fixes: - resolve.ts: validate packName and version with validatePathSegment before path construction (prevents path traversal via malicious index.json) - publish.ts: clear index cache after fetchRegistry in publish/unpublish (prevents stale index reads within same process) - publish.ts: export validatePathSegment for use by resolve.ts QA WARN fixes: - git.ts: persist core.symlinks=false via git config in fetchRegistry (not just at clone time via -c flag) New tests (46 added, 201 total): - config.test.ts: name length limits, URL trimming, credential rejection, ssh:// URLs, sanitizeUrl, file permissions - git.test.ts: runtime type validation, symlink protection, cache corruption, HEAD fallback, timeout config - publish.test.ts: path traversal, semver, corrupt index, size limit, dedup, rollback, re-publish - sync.test.ts: locking, stale lock cleanup Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address linting and typecheck CI failures - Replace || with ?? for nullish coalescing in GIT_TIMEOUT_MS (src/registry/git.ts:19) - Remove async keyword from isNeuralModelAvailable since it has no await expressions (tests/integration/retrieval-quality.test.ts:513) These changes fix the lint-and-typecheck job failures on PR #407. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * chore: fix code formatting via prettier * chore: fix formatting on all registry files via prettier * fix: handle gzip-compressed pack files in registry publish readPackJson() was reading files as UTF-8 text, which corrupts gzip binary data and produces "is not valid JSON" errors when publishing .json.gz pack files. Now reads as raw Buffer and auto-detects gzip via magic bytes (0x1f 0x8b), matching the existing pattern in src/core/packs.ts:readPackFile. Also adds project CLAUDE.md with agent guidelines including this pack file I/O pattern. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: comprehensive CLAUDE.md with full project guidelines Covers: build/test commands, CI expectations, code style (strict TS, no-any, prettier config), project structure, error hierarchy, database schema/migrations, testing patterns and fixtures, pack file gzip handling, registry system, logging, and configuration. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Full security and robustness audit of the git-based pack registry feature, addressing 20+ findings across critical, high, and medium severity.
Critical fixes
[a-zA-Z0-9._-]allowlist before use inpath.join()— in publish, unpublish, and resolve pathsverifyChecksum()now called before installing any registry-resolved pack (was defined but never invoked)readIndex()validates eachindex.jsonentry through a type guard instead of blindas PackSummary[]castHigh severity fixes
core.symlinks=falseon clone and persisted viagit configon fetchfetchRegistry()detects non-git cache dirs, removes them, and throws descriptive errorindex.jsonnow throws instead of silently resetting to[]0o600, directories with0o700user:pass@ortoken@rejected; URLs sanitized in logsMedium severity fixes
ssh://git@host:port/pathformat now accepted (was rejected, forcing SCP format which misinterprets port as path)^\d+\.\d+\.\d+(-[a-zA-Z0-9._-]+)?$LIBSCOPE_GIT_TIMEOUT_MSenv var, capped at 5 minutesorigin/HEAD→origin/main→origin/masterLow severity fixes
deriveNameFromUrl()handlesssh://URLs correctlyTests
tests/unit/registry/publish.test.tswith 19 testsTest plan
npx vitest run tests/unit/registry/ tests/integration/registry/)libscope registry add ssh://git@bitbucket:7999/mdog/libscope-registry.gitaccepts SSH URL🤖 Generated with Claude Code