Skip to content

Add source-agnostic template fetching for private template families#20

Closed
next-devin wants to merge 2 commits into
mainfrom
agnostic-template-source
Closed

Add source-agnostic template fetching for private template families#20
next-devin wants to merge 2 commits into
mainfrom
agnostic-template-source

Conversation

@next-devin

Copy link
Copy Markdown

What

campaign-init fetched starter-template source from one hardcoded public repo, so it couldn't scaffold template families hosted elsewhere (e.g. an outside, access-controlled repo). This adds a source-agnostic resolver + authenticated fetch — while keeping page-kit a generic build tool that holds no per-family knowledge.

Design

  • lib/template-source.js (new) — resolveTemplateSource(overrides) overlays caller-supplied {repo, ref, source_path, private} on the public starter-templates default. Names no families, no private repos. As the trust boundary it validates shape (INVALID_SOURCE on bad repo / ref / traversing or non-POSIX source_path).
  • lib/actions/init.js — new --source-repo / --source-ref / --source-path / --private flags.
    • Default (no flags): public starter-templates, anonymous — unchanged behavior.
    • --private: authenticated tarball (GITHUB_TOKEN/GH_TOKEN or gh auth token), skips the public picker, requires --template. Token dropped on the signed redirect (pinned to https + GitHub-owned hosts), never serialized.
    • Missing token → exit 5; malformed --source-* → exit 6.
    • extractTemplate honors source_path via path.posix.join (platform-independent), instead of hardcoding src/.

The opinionated part — which family lives in which private repo — stays in the orchestrating layer (campaigns-os / the operator's invocation), never in page-kit.

Verification

  • --template <slug> --source-repo <owner/repo> --private → authenticated extract; name refined from campaigns.json.
  • --private without a token → exit 5; bad --source-path → exit 6.
  • Public --template <slug> with no flags → anonymous (unchanged).
  • 208 tests green.

Supersedes #19 — identical net change, but presented as one clean commit instead of #19's build-then-refactor history. Pairs with campaigns-os#101 (the Arjuna recognition contracts; repo-agnostic).

campaign-init fetched starter-template source from a single hardcoded public
repo, so it could not scaffold template families hosted elsewhere (e.g. an
outside, access-controlled repo). This adds a source-agnostic resolver plus an
authenticated fetch path, while keeping page-kit a generic build tool with no
per-family knowledge.

lib/template-source.js (new)
  resolveTemplateSource(overrides) overlays caller-supplied {repo, ref,
  source_path, private} on the public starter-templates default. It names no
  families and no private repos. As the trust boundary, it validates shape and
  throws INVALID_SOURCE on a malformed value:
    - repo: strict "owner/repo" (rejects traversal, query strings, odd chars)
    - ref:  branch/tag/SHA; slashes allowed (feature/foo, refs/heads/x) but no
            "..", leading/trailing slash, or odd chars
    - source_path: a relative POSIX subtree only (forward slashes, no "..",
            no absolute/"~") so extraction can never path.join out of the tree

lib/actions/init.js
  - New flags: --source-repo / --source-ref / --source-path / --private.
    Default (no flags) = public starter-templates, anonymous — unchanged.
    --private fetches an authenticated tarball (GITHUB_TOKEN/GH_TOKEN or
    `gh auth token`), skips the public picker, and requires --template.
  - Missing token -> exit 5 (MISSING_INPUT, code MISSING_GITHUB_TOKEN); no fetch
    attempted. Malformed --source-* -> exit 6 (INVALID_INPUT, INVALID_SOURCE).
  - Authenticated fetch follows only https redirects to GitHub-owned hosts; the
    token is dropped on the signed redirect and never serialized into output.
  - extractTemplate honors source_path (built with path.posix.join, so the
    subtree contract is platform-independent) instead of hardcoding src/.
  - A --private source's display name is refined from the source repo's
    campaigns.json when --name is omitted.

The opinionated part -- which family lives in which private repo -- stays in the
orchestrating layer (campaigns-os / the operator's invocation), never here.

208 tests (resolver, validation, redirect host-pinning, existing init tests).
Verified: --template <slug> --source-repo <owner/repo> --private extracts an
authenticated tarball; --private without a token exits 5; a bad --source-path
exits 6; public --template <slug> with no flags resolves anonymously.

Supersedes #19 (same net change, clean history).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Jun 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@next-devin next-devin marked this pull request as ready for review June 15, 2026 15:16
Comment thread lib/template-source.js Outdated
Comment thread lib/template-source.js
Comment thread lib/template-source.js Outdated
Comment thread lib/actions/init.js Outdated
Comment thread lib/actions/init.js Outdated
Comment thread lib/actions/init.js Outdated
Comment thread lib/actions/init.js
Comment thread test/init.test.js
@kilo-code-bot

kilo-code-bot Bot commented Jun 15, 2026

Copy link
Copy Markdown

Code Review Summary

Status: 1 Issue Found | Recommendation: Merge (minor gap)

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1

All 8 issues from the previous review have been resolved by this commit:

  • lib/template-source.js: both tarball URLs now take a plain ref (no refs/heads/ prefix), so a tag/SHA ref works on both and api.github.com no longer 404s on a refs/heads/... value. (WARNING x2 — fixed)
  • lib/template-source.js: ai_context_doc_url is now always derived from the public default repo, so it can't 404 for a --private source. (SUGGESTION — fixed)
  • lib/actions/init.js: AI-context URL is resolved lazily via aiContextDocUrl() instead of a module-load constant, so a future validation tightening can't turn an import into a hard module-load failure. (WARNING — fixed)
  • lib/actions/init.js: gh auth token now runs with a 5s timeout; a wedged gh is treated as "no token" instead of hanging. (WARNING — fixed)
  • lib/actions/init.js: the untrusted-redirect error now reports only the host, not the full signed URL. (SUGGESTION — fixed)
  • test/init.test.js: new e2e test asserts fetchBufferAuthed drops the Authorization header when it follows the 302 to codeload. (SUGGESTION — fixed)
Issue Details (click to expand)

SUGGESTION

File Line Issue
test/init.test.js 52 The new e2e test covers the trusted-redirect / drop-Authorization path, but the untrusted-redirect / reject path of fetchBufferAuthed is still only covered by the isTrustedGithubRedirect unit test. Consider adding a negative case to lock in the integration.
Files Reviewed (3 files)
  • lib/actions/init.js - 0 issues (all 4 previous findings resolved: lazy AI-context URL, host-only redirect error, gh timeout)
  • lib/template-source.js - 0 issues (all 3 previous findings resolved: plain ref in both tarball URLs, AI-context from public default)
  • test/init.test.js - 1 issue (drop-Authorization e2e test added; untrusted-redirect negative case still missing)
  • test/template-source.test.js - 0 issues (assertions updated to match plain-ref tarball URLs; positive coverage of tag/SHA/plain-ref shapes)

Fix these issues in Kilo Cloud

Previous Review Summary (commit 329010e)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 329010e)

Status: 8 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 4
SUGGESTION 4

The PR cleanly adds a source-agnostic resolver + authenticated tarball fetch, with a strong trust boundary in lib/template-source.js and a host-pinned redirect check in isTrustedGithubRedirect. The main risks are around URL-shape inconsistencies between the public and authenticated tarball endpoints, a module-load-time side effect in init.js, and a couple of secondary hardening gaps.

Issue Details (click to expand)

WARNING

File Line Issue
lib/template-source.js 95 tarball_url_anonymous hardcodes refs/heads/${ref} but ref validation accepts tags/SHAs/refs/heads/... — URL is misleading and inconsistent with the validator.
lib/template-source.js 93 tarball_url_authenticated uses api.github.com/repos/{repo}/tarball/{ref} which does NOT accept the refs/heads/... form; passing one with --private will 404.
lib/actions/init.js 31 AI_CONTEXT_DOC_URL is evaluated at module-load time; a future tightening of resolveTemplateSource validation could hard-fail the whole module, and tests cannot influence it.
lib/actions/init.js 187 execSync('gh auth token', ...) has no timeout — a hung gh (locked keyring, slow credential helper) blocks the entire campaign-init invocation.

SUGGESTION

File Line Issue
lib/template-source.js 96 ai_context_doc_url is built from rawBase even for private sources, where the URL will 404. Resolve from defaults unconditionally.
lib/actions/init.js 156 Error message echoes the full redirect URL into thrown errors; consider logging only the host.
lib/actions/init.js 714 Synthetic pickedTemplate.description interpolates source.repo; low risk given validation, but a sentinel constant would harden the trust boundary.
test/init.test.js 26 isTrustedGithubRedirect is unit-tested in isolation, but the end-to-end redirect-strips-Authorization behavior of fetchBufferAuthed is not asserted.
Files Reviewed (3 files)
  • lib/actions/init.js - 5 issues
  • lib/template-source.js - 3 issues
  • test/init.test.js - 1 issue
  • test/template-source.test.js - 0 issues (no concerns; validation paths and override overlay are well covered)

Fix these issues in Kilo Cloud


Reviewed by minimax-m3 · 311,693 tokens

…t host

Kilo review on #20:
- template-source.js: both tarball URLs now take a PLAIN ref. The anonymous
  codeload URL dropped its "refs/heads/" prefix (tar.gz/<ref>), matching the
  authed api.github.com tarball/<ref> form — so a tag or SHA ref works on both
  and api.github.com no longer 404s on a refs/heads/ value. (WARNING x2)
- template-source.js: ai_context_doc_url is now always derived from the PUBLIC
  default repo, so it can't 404 for a --private source. (SUGGESTION)
- init.js: AI-context URL is resolved lazily via aiContextDocUrl() instead of a
  module-load constant, so a future validation tightening can't turn an import
  into a hard module-load failure. (WARNING)
- init.js: `gh auth token` runs with a 5s timeout; a wedged gh is treated as
  "no token" (actionable MISSING_GITHUB_TOKEN) instead of hanging. (WARNING)
- init.js: the untrusted-redirect error reports only the host, not the full
  signed URL. (SUGGESTION)
- test: e2e test that fetchBufferAuthed DROPS the Authorization header when it
  follows the 302 to codeload (mocks https.get) — the security guarantee the
  helper exists for. (SUGGESTION)

210 tests green. Verified public (anonymous tar.gz/main) and private (authed
tarball/main) both still extract.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread test/init.test.js
// End-to-end: the authed fetch must DROP the Authorization header when it follows
// the signed 302 to codeload — the whole reason fetchBufferAuthed exists. Mock the
// shared https.get and assert the redirect request carries no token.
test('fetchBufferAuthed drops the Authorization header on redirect', async () => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUGGESTION: The new e2e test covers the trusted-redirect / drop-Authorization path, but the untrusted-redirect / reject path of fetchBufferAuthed is still only covered by the isTrustedGithubRedirect unit test. Consider adding a negative case (e.g. a 302 to https://attacker.example/... asserting the promise rejects with the "untrusted host" error) to lock in the integration of the trust check with the authed fetch helper.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

@next-devin next-devin requested a review from alexphelps June 15, 2026 15:40
@next-devin next-devin marked this pull request as draft June 15, 2026 15:40
@alexphelps

Copy link
Copy Markdown
Member

This feature was done in #21

@alexphelps alexphelps closed this Jun 16, 2026
@next-devin

Copy link
Copy Markdown
Author

Closing — superseded by #21 (merged, v0.2.0), which solves this more completely and is the canonical mechanism now. Named sources in _data/template-sources.json (git + local types), selected via --source <name>, using ambient SSH keys. That's the cleaner layering this PR was reaching for — the source config lives in the consuming repo, page-kit stays agnostic — and it drops the token/tarball/redirect-pinning machinery entirely (SSH git clone instead of an authenticated api.github.com tarball). Adopting #21's mechanism for the Arjuna/Adsbranded use case.

@next-devin next-devin deleted the agnostic-template-source branch June 16, 2026 06:55
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