Feat/catalog ci gap closure#21
Conversation
- add explicit error-coded runtime checks for moduleName consistency, tarball download/integrity verification, depends link integrity, and official pre-1.0 cli range consistency - tighten peer dependency policy by removing global foo allowance and keeping it only as a scoped temporary exception for @choysum-dev/core@0.0.0-20260614200130 - expose tarball verification limits in validate workflow for deterministic CI behavior
- stop enforcing peerDependencies allowlist and scoped exceptions during catalog build - keep depends link validation while allowing any peerDependencies from module metadata - delete obsolete schemas/peer-dependencies-allowlist.json configuration file
Deploying choysum-modules-directory with
|
| Latest commit: |
c9c72b6
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://12205604.modules-directory.pages.dev |
| Branch Preview URL: | https://feat-catalog-ci-gap-closure.modules-directory.pages.dev |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe catalog build script gains tarball integrity verification, ChangesCatalog Validation and Integrity Enforcement
Sequence DiagramsequenceDiagram
participant ModuleProcessor as process_module
participant ValidateName as validate_module_name
participant VerifyIntegrity as verify_tarball_integrity
participant ParseIntegrity as parse_integrity_value
participant ValidateCLI as validate_official_pre1_cli_range
participant ValidateContracts as validate_runtime_contracts
ModuleProcessor->>ValidateName: choysum_meta, module_id
ValidateName-->>ModuleProcessor: ValueError if missing/mismatch
ModuleProcessor->>VerifyIntegrity: tarball_url, integrity string
VerifyIntegrity->>ParseIntegrity: parse algorithm and digest
ParseIntegrity-->>VerifyIntegrity: selected algorithm, expected_digest
VerifyIntegrity-->>ModuleProcessor: ValueError on scheme/download/size/hash failure
ModuleProcessor->>ValidateCLI: trust, normalized_cli_range
ValidateCLI-->>ModuleProcessor: ValueError if pre-1.0 constraint violated
Note over ValidateContracts: After all modules collected
ValidateContracts->>ValidateContracts: Validate depends ids, links, self-refs, duplicates
ValidateContracts-->>ModuleProcessor: aggregated ValueError on violations
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces robust validation mechanisms to the catalog builder in scripts/build_catalog.py, including module name verification, tarball integrity checks, official CLI range validation, and runtime contract dependency checks. The code review provides valuable suggestions to enhance these features: supporting multiple space-separated integrity hashes to select the strongest algorithm, optimizing tarball downloads by checking the Content-Length header to fail fast, and expanding dependency validation to detect self-dependencies and duplicate entries.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/build_catalog.py`:
- Around line 483-539: The verify_tarball_integrity function accepts a
tarball_url parameter directly from NPM registry metadata without validating its
URL scheme, creating a potential SSRF vulnerability where malicious URLs using
file://, ftp://, or other schemes could read local files or access internal
services. Add URL scheme validation at the start of the verify_tarball_integrity
function, before the urllib.request.urlopen call, to ensure the tarball_url only
uses https or http schemes. If the scheme is invalid, raise an appropriate error
to reject the request.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f55d5dc6-1bda-4499-b631-bc871045a53f
📒 Files selected for processing (2)
.github/workflows/validate-catalog.ymlscripts/build_catalog.py
- validate tarball URL scheme before download to prevent non-http(s) fetches - support multi-hash integrity values and choose the strongest valid supported algorithm - fail fast on oversized tarballs using Content-Length and enforce duplicate/self depends checks
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces robust validation mechanisms to the catalog build process in scripts/build_catalog.py. This includes verifying module names, checking tarball download integrity and size limits, validating CLI ranges for official pre-1.0 modules, and ensuring runtime contracts (dependency relationships) are valid and unbroken. Feedback suggests implementing a local caching mechanism for downloaded tarballs to improve build performance and reduce network bandwidth usage.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
- add optional CHOYSUM_CACHE_DIR support to reuse previously verified tarballs by integrity hash - keep strict integrity/size/scheme checks while reducing redundant downloads on repeated builds - ensure temporary cache files are atomically promoted on success and cleaned on failure
- add explicit comments in verify_cached_tarball for non-fatal OSError cleanup branches - keep existing behavior unchanged while satisfying github-code-quality empty-except guidance
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Module Submission / Update
Description:
Checklist
Please confirm the following before submitting your PR (refer to CONTRIBUTING.md / 中文指南 for guidance):
*.jsonfile structure strictly followsschemas/catalog-entry.schema.json(contains onlypackage,trust, andmaintainers; no redundant data like version numbers).official/verified/community).choysum.moduleNamespecified in the npm metadata.Summary by CodeRabbit