Repository navigation
Made advancements in cgc bundle (.cgc format) - #1060
Conversation
|
@tejinderpa is attempting to deploy a commit to the shashankss1205's projects Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds security and portability features to CGC bundle export/import by introducing integrity manifests, optional HMAC signing, and optional AES-GCM encryption, plus CLI commands to verify/inspect/diff bundles and improved parsing heuristics for large files.
Changes:
- Add manifest generation + optional HMAC signature and AES-256-GCM “envelope” encryption to bundle export/import.
- Extend CLI with bundle verify/inspect/diff commands and new flags for signing/encryption.
- Adjust web parser worker to skip large files with different thresholds for likely-minified vs normal sources.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| website/src/lib/parser.worker.ts | Adds size thresholds and heuristics to skip overly large / likely-minified files during parsing. |
| src/codegraphcontext/core/cgc_bundle.py | Implements manifest checksums, optional HMAC signing, optional AES-GCM encryption envelope, and verify/inspect/diff helpers. |
| src/codegraphcontext/cli/main.py | Exposes signing/encryption options and adds verify/inspect/diff CLI subcommands. |
| pyproject.toml | Adds cryptography dependency to support encrypted bundles. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _extract_zip_safely(self, zip_path: Path, target_dir: Path) -> Tuple[bool, str]: | ||
| target_root = target_dir.resolve() | ||
| with zipfile.ZipFile(zip_path, "r") as zip_ref: | ||
| for entry in zip_ref.namelist(): | ||
| resolved = (target_dir / entry).resolve() | ||
| try: | ||
| resolved.relative_to(target_root) | ||
| except ValueError: | ||
| return False, f"Zip Slip detected: entry '{entry}' escapes target directory" | ||
| zip_ref.extractall(target_dir) | ||
| return True, "Extracted" |
| salt = base64.b64decode(metadata["salt"]) | ||
| nonce = base64.b64decode(metadata["nonce"]) | ||
| key = self._derive_encryption_key(password, salt, int(metadata.get("iterations", 390000))) | ||
| try: | ||
| plaintext = AESGCM(key).decrypt(nonce, ciphertext, None) | ||
| except Exception: | ||
| return None, "Failed to decrypt bundle payload" |
| salt = secrets.token_bytes(16) | ||
| nonce = secrets.token_bytes(12) | ||
| iterations = 390000 | ||
| key = self._derive_encryption_key(password, salt, iterations) | ||
| plaintext = payload_zip.read_bytes() | ||
| ciphertext = AESGCM(key).encrypt(nonce, plaintext, None) |
| manifest = { | ||
| "manifest_version": "1.0.0", | ||
| "digest": "sha256", | ||
| "files": files, | ||
| "created_at": datetime.utcnow().strftime("%Y-%m-%dT%H:%M:%SZ"), | ||
| } |
| output: str = typer.Argument(..., help="Output path for the .cgc bundle file"), | ||
| repo: Optional[str] = typer.Option(None, "--repo", "-r", help="Specific repository path to export (default: export all)"), | ||
| no_stats: bool = typer.Option(False, "--no-stats", help="Skip statistics generation"), | ||
| sign_key: Optional[str] = typer.Option(None, "--sign-key", envvar="CGC_BUNDLE_SIGN_KEY", help="HMAC signing key (or set CGC_BUNDLE_SIGN_KEY)"), |
| password: Optional[str] = typer.Option(None, "--password", envvar="CGC_BUNDLE_PASSWORD", help="Password for encrypted bundles (or set CGC_BUNDLE_PASSWORD)", hide_input=True), | ||
| verify_key: Optional[str] = typer.Option(None, "--verify-key", envvar="CGC_BUNDLE_VERIFY_KEY", help="HMAC verification key for signed bundles (or set CGC_BUNDLE_VERIFY_KEY)"), |
| "uvicorn>=0.22.0", | ||
| "cryptography>=42.0.0" |
| const isLikelyMinified = /\.min\.[cm]?[jt]sx?$/.test(f.path) || /(^|[\/\\])(vendor|lib)[\/\\]/.test(f.path); | ||
| const maxFileChars = isLikelyMinified ? MAX_PARSEABLE_MINIFIED_FILE_CHARS : MAX_PARSEABLE_FILE_CHARS; | ||
|
|
||
| if (f.content.length > maxFileChars || isPathIgnored) { | ||
| if (f.content.length > maxFileChars) { | ||
| console.warn(`[Diagnostic] Skipping very large file (${(f.content.length / 1024).toFixed(1)} KB, limit ${(maxFileChars / 1024).toFixed(0)} KB): ${f.path}`); |
|
Hey it is asking for permissions, kindly grant it so that i see if any conflicts are there...than resolve them and if no conflicts occur kindly merge it @Shashankss1205 |
|
👋 Thanks for contributing to CodeGraphContext! Since this PR was opened, |
|
Triage update: bundle integrity manifests, signing, and native .cgc diffing (#955, and #1312 asks for exactly the diff piece) are all still wanted. cgc_bundle.py has been heavily reworked since this branch (multi-backend PK maps, Zip Slip protection, identifier validation), so this needs a careful rebase — security-sensitive code we don't want to merge mechanically. @tejinderpa still up for rebasing it? Happy to review promptly if so. 🙏 |
|
Reviewed this properly and I'm handing it back rather than resolving the conflicts myself — with one piece of good news and one thing that needs your decision. The good news: your Zip Slip protection is soundThe conflict in It does not. resolved = (target_dir / entry).resolve()
try:
resolved.relative_to(target_root)
except ValueError:
return False, f"Zip Slip detected: entry '{entry}' escapes target directory"That is arguably stronger than Why I stoppedTwo reasons. 1. The conflicts are not mechanical. 23 commits have landed on 2. This is a design decision, not a merge. The PR introduces HMAC-SHA256 signing and AES-256-GCM encryption to the bundle format. That is a format and threat-model commitment: once bundles carry an envelope, every consumer must understand it, and the choices below become the project's answers. Questions worth settling before it lands, all of which are yours rather than mine:
What I'd suggestRebase onto current Happy to review once it's rebased. |
Conflicts were additive on both sides: - CLI: combine --password/--verify-key with main's --yes/--context, and forward every Typer-defaulted option at each delegation site (the test_command_delegation_and_exit_codes meta-test enforces this). - cgc_bundle: keep the _extract_bundle_archive refactor; its _extract_zip_safely applies the same component-wise Zip Slip check main added inline, to both the outer archive and the decrypted payload. - _validate_bundle: run manifest and signature verification *and* main's identifier validation, rather than either alone. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Shashankss1205
left a comment
Shashankss1205
left a comment
There was a problem hiding this comment.
Verified the full round-trip end-to-end on a real bundle:
export --sign-key mysignkey --encrypt-password hunter2 → Signature: HMAC-SHA256, Encryption: AES-256-GCM
verify --password hunter2 --verify-key mysignkey → Valid bundle
verify --password wrongpw → Failed to decrypt bundle payload
verify --verify-key badkey → Signature verification failed
import --password hunter2 --verify-key mysignkey → imported, 33 nodes / 57 edges
import (no password) → Bundle is encrypted; provide a password
Both negative cases fail correctly rather than silently degrading, which is the important part.
I resolved the conflicts with main and pushed to your branch. Three notes on how I resolved them, since they weren't purely mechanical:
- CLI options — combined your
--password/--verify-keywithmain's newer--yes/--contextrather than picking a side, and forwarded all four at every delegation site.test_command_delegation_and_exit_codesenforces this and caught one I'd missed atmain.py:1254(registry_download→bundle_import). - Zip extraction —
mainhad added inline Zip Slip protection while you refactored extraction into_extract_bundle_archive. I kept your refactor after checking that_extract_zip_safelydoes the equivalent component-wise check viarelative_to(), and that it's applied to both the outer archive and the decrypted inner payload — so this is actually stronger than what it replaced, not weaker. _validate_bundle— both sides added a validation step (your manifest + signature checks,main's_validate_bundle_identifiersguarding against Cypher injection). Both now run in sequence.
Unit suite green (1166 passed). Merging — thanks, this is a solid addition to the .cgc format.
github-project-automation
Bot
moved this from Backlog tasks
to Done
in CGC Progress Board
Two regressions from #1060, both caught by the integration suite: 1. bundle_export dereferences sign_key/encrypt_password. Called as a plain function rather than through Typer -- which tests/integration/test_parser_ goldens.py:273 does -- an omitted option keeps its OptionInfo sentinel. That sentinel is truthy, so `if sign_key:` passed and sign_key.encode() raised AttributeError, failing all 20 golden tests. Normalized at the top of the command; these two are the only options here that get dereferenced. 2. The new bundle verify/inspect/diff subcommands were not reflected in the CLI inventory tests. verify/inspect/diff each require a .cgc positional argument, so they are excluded from the bare-invocation smoke matrix the same way bundle merge already is. Full suites green: 1178 unit, 44 integration. Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
Shashankss1205
added
gssoc:approved
fixes #955
This PR improves the
.cgcbundle format with security and comparison features that were previously listed as roadmap/future work.What Changed
.cgcarchives..cgcfiles can be compared without importing them.cgc bundle verifycgc bundle inspectcgc bundle diffcgc bundle export --sign-key ...cgc bundle export --encrypt-password ...cgc bundle import --password ... --verify-key ...cgc bundle load --password ... --verify-key ...cryptography>=42.0.0as a dependency for encrypted bundle support.Why
The current
.cgcformat worked well for basic full-snapshot sharing, but it lacked several production-grade features:This PR makes
.cgcbundles safer to distribute and easier to compare across project versions.Verification
src/codegraphcontext/core/cgc_bundle.pysrc/codegraphcontext/cli/main.py.cgcarchives only contain:encryption.jsonpayload.encNotes
This PR does not fully implement registry publishing, database-level conflict resolution, or delta import/application yet. It lays the groundwork for those features by adding manifest-based verification and bundle comparison.