Skip to content

feat: download cds get profiles from GitHub instead of local checkout - #493

Merged
RonaldHensbergen merged 5 commits into
mainfrom
feat/cds-get-github-download
Aug 24, 2026
Merged

feat: download cds get profiles from GitHub instead of local checkout#493
RonaldHensbergen merged 5 commits into
mainfrom
feat/cds-get-github-download

Conversation

@RonaldHensbergen

Copy link
Copy Markdown
Owner

Summary

cds get previously copied a profile and its dependent module/runtime assets from a local checkout (the current repository, or --remote <local-path>). By design, it now downloads from GitHub instead:

  • Default (no flags): fetches this project's upstream repository at the main branch via the GitHub tarball API (api.github.com/repos/{owner}/{repo}/tarball/{ref}) — no git binary required.
  • --remote <owner/repo> or a github.com/... URL, plus optional --ref <branch|tag|sha>: fetches a specific fork/revision from GitHub.
  • --local <dir>: explicit opt-in to use an existing local directory instead of downloading (mutually exclusive with --remote/--ref), for offline/dev workflows.

Changes

  • cli/getter.py: added GitHub remote parsing (owner/repo shorthand, github.com URLs, git@github.com: SSH form) and tarball download/extraction via urllib + stdlib tarfile (no new dependency). --local now explicitly selects a local directory; fetch_profile() gained ref and local parameters.
  • cli/main.py: cds get gained --ref and --local CLI flags; --remote help text updated to describe the new GitHub-only semantics.
  • README.md: documented the new default download-from-GitHub behavior and flags.
  • tests/test_getter.py / tests/test_main.py: updated existing local-repo tests to use --local/local=, and added coverage for GitHub remote parsing, default-remote download (mocked), explicit owner/repo + ref, download failure handling, unresolvable remote errors, and the --remote/--local mutual-exclusivity check.

Testing

  • python -m unittest discover -s tests -p "test_*.py" — 536 tests, OK
  • ruff check . — all checks passed
  • npx markdownlint-cli@0.49.0 README.md — clean

By design, `cds get` now fetches its source repository from GitHub
rather than copying files from a local checkout:

- Default (no flags): downloads this project's upstream repo at `main`
  via the GitHub tarball API (no `git` binary required).
- `--remote <owner/repo>` or a github.com URL, plus optional
  `--ref <branch|tag|sha>`, fetches a specific fork/revision.
- `--local <dir>` explicitly opts into using an existing local
  directory instead (mutually exclusive with --remote/--ref), for
  offline/dev workflows.

Updates cli/getter.py, cli/main.py, README.md, and tests accordingly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
RonaldHensbergen and others added 3 commits August 23, 2026 23:54
Bandit (B310) doesn't recognize ruff's `# noqa: S310` suppression syntax;
it needs its own `# nosec B310` marker, matching the existing convention
in cli/image_updates.py.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@RonaldHensbergen

Copy link
Copy Markdown
Owner Author

Review: LGTM ✅ (posted as a comment since GitHub doesn't allow self-approval)

Reviewed the diff and CI (all 11 checks passing). The core design is solid:

  • Uses the safe tarfile.extractall(..., filter="data") extraction mode, correctly mitigating path traversal/symlink attacks on the downloaded archive.
  • Download is scoped to a fixed api.github.com host with validated owner/repo values (no SSRF via arbitrary URL/scheme injection) and a 30s timeout.
  • Network/archive failures (HTTPError, URLError, tarfile.TarError) are caught and wrapped in clean GetErrors, no raw tracebacks.
  • Existing profile-path/module-traversal guards (_validate_source_repository, _resolve_source_profile_path, _collect_asset_roots) are unchanged and still apply correctly to the extracted directory.
  • Good test coverage for remote parsing, mocked downloads, ref selection, and --remote/--local mutual exclusivity.
  • Temp directory cleanup via context manager, including on error paths.

Two independent reviews turned up a few non-blocking follow-ups, now tracked as separate issues so they don't hold up this PR:

None of these are security-critical or regressions in existing guarantees. Good to merge.

@RonaldHensbergen
RonaldHensbergen merged commit db9acdd into main Aug 24, 2026
11 checks passed
@RonaldHensbergen
RonaldHensbergen deleted the feat/cds-get-github-download branch August 24, 2026 13:26
RonaldHensbergen pushed a commit that referenced this pull request Aug 24, 2026
Addresses the missing-CHANGELOG-entry follow-up from #493 (issue #508).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
RonaldHensbergen added a commit that referenced this pull request Aug 24, 2026
* chore(release): bump version to 0.5.1

* docs: finalize CHANGELOG [Unreleased] heading for 0.5.1

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* docs: note cds get GitHub-download breaking change in 0.5.1 changelog

Addresses the missing-CHANGELOG-entry follow-up from #493 (issue #508).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: RonaldHensbergen <22539616+RonaldHensbergen@users.noreply.github.com>
Co-authored-by: Ronald Hensbergen <r.hensbergen@regiogv.nl>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants