Skip to content

Conversation

@recepgunes1
Copy link
Member

@recepgunes1 recepgunes1 commented Dec 5, 2025

Summary by CodeRabbit

  • New Features

    • Added a startup banner and a template update/refresh capability.
  • Refactoring

    • Consolidated template management into a single CLI command (list/update/filter).
    • Reordered initialization and template loading to use a centralized user templates repository.
    • Removed the separate validate CLI command.
  • Chores

    • Removed numerous template manifests and associated compose configurations.
    • Added .vscode to ignore list, updated dependency listings, and removed the templates validation CI workflow.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 5, 2025

Walkthrough

Moves template storage to a user-local git repo (~/.vt-templates) with clone/pull support, consolidates list/update/filter into a single template CLI command, moves banner printing to program startup, and removes many local template files plus the template-validation CI workflow.

Changes

Cohort / File(s) Summary
Gitignore
/.gitignore
Adds .vscode to ignored paths.
Go module
go.mod
Adds multiple indirect dependencies (go-git v5 and related packages) to support git clone/pull behavior.
CLI startup & entry
cmd/vt/main.go, internal/cli/root.go
Moves banner.Print() into main, invokes CLI init at Run start, removes banner hooks from help/pre-run, and replaces prior subcommand registrations with the consolidated template command.
CLI commands & flags
internal/cli/list.go
Replaces previous list subcommand with a single template command exposing --list, --update, and --filter flags; adds mutual-exclusion and usage validation and routes to ListWithFilter or SyncTemplates.
CLI validate removal
internal/cli/validate.go (deleted)
Removes the validate CLI command and its per-template validation logic that enforced index.yaml presence and ID-directory consistency.
Template downloader & loader
pkg/templates/downloader.go, pkg/templates/template.go
Adds cloneTemplatesRepo(repoPath string, force bool) error (uses go-git), adds TemplateRemoteRepoistory constant, changes Init() to use ~/.vt-templates and category/name layout, implements recursive loading helpers, and adds SyncTemplates() to refresh templates.
Removed vulnerability templates
templates/...
Deletes many template directories and files (index.yaml, docker-compose.yaml, nuclei, semgrep) across multiple VTs, e.g. templates/vt-2024-53995/*, templates/vt-2025-29927/*, templates/vt-2025-64459/*, templates/vt-bwapp/*, templates/vt-dvwa/*, templates/vt-juice-shop/*, templates/vt-mutillidae-ii/*, templates/vt-webgoat/*.
CI workflow removed
.github/workflows/validate-templates.yaml
Removes the GitHub Actions workflow that built vt and ran vt validate to verify template index files.

Sequence Diagram(s)

sequenceDiagram
    participant Main as cmd/vt/main
    participant Banner as internal/banner
    participant CLI as internal/cli
    participant Templates as pkg/templates
    participant Git as RemoteGit

    Main->>Banner: banner.Print()
    Main->>CLI: InitCLI()
    CLI->>Templates: Init() — load ~/.vt-templates
    alt ~/.vt-templates missing
        Templates->>Git: shallow clone TemplateRemoteRepoistory (Depth=1)
        Git-->>Templates: cloned
    else repo exists
        Templates->>Git: open repo & check status
        alt status dirty and !force
            Templates-->>CLI: error "detected uncommitted changes"
        else
            Templates->>Git: pull origin (forced)
            Git-->>Templates: updated / already up-to-date
        end
    end
    CLI->>Templates: On `template --list/--update/--filter` invoke ListWithFilter or SyncTemplates
    Templates-->>CLI: results / errors
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • dogancanbakir
  • ahsentekd

Poem

🐰 I hopped to ~/.vt-templates by moonlit code,

I fetched new fields where template seeds were sowed.
Banner aloft at startup’s gentle call,
Commands now gather — list, update, filter all.
A happy little hop — clone, sync, and show it all!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 70.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title addresses the main changes: template structure refactoring, CLI reorganization, and removal of unused commands, which align with the significant changeset modifications.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/template-migration

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (4)
internal/cli/list.go (1)

60-64: Consider using Cobra's MarkFlagsMutuallyExclusive for cleaner flag validation.

Cobra provides built-in support for mutually exclusive flags which could simplify the validation logic in the Run function.

 func setupTemplateCommand() {
 	templateCmd.Flags().BoolP("list", "l", false, "List available templates")
 	templateCmd.Flags().BoolP("update", "u", false, "Fetch templates repository to local working directory")
 	templateCmd.Flags().StringP("filter", "f", "", "Filter templates by tag or keyword (only works with --list)")
+	templateCmd.MarkFlagsMutuallyExclusive("list", "update")
 }
pkg/templates/template.go (1)

103-116: Consider extracting duplicate path construction logic.

Both Init() and Update() duplicate the home directory and repo path construction (lines 54-59 and 105-110). Extract this to a helper function to reduce duplication.

+func getTemplatesRepoPath() (string, error) {
+	homeDir, err := os.UserHomeDir()
+	if err != nil {
+		return "", err
+	}
+	return filepath.Join(homeDir, "vt-templates"), nil
+}
+
 // Update loads all templates from remote repository (force).
 func Update() {
-	homeDir, err := os.UserHomeDir()
+	repoPath, err := getTemplatesRepoPath()
 	if err != nil {
 		log.Fatal().Msgf("%v", err)
 	}
-
-	repoPath := filepath.Join(homeDir, "vt-templates")
 	log.Info().Msgf("cloning github.com/HappyHackingSpace/vt-templates")
 	err = cloneTemplatesRepo(repoPath, true)
 	if err != nil {
 		log.Fatal().Msgf("%v", err)
 	}
 }
pkg/templates/downloader.go (1)

10-40: Consider handling git.ErrRepositoryNotExists explicitly for clearer control flow.

Currently, any error from git.PlainOpen triggers the clone path. Explicitly checking for git.ErrRepositoryNotExists would make the intent clearer and prevent masking other errors.

 func cloneTemplatesRepo(repoPath string, force bool) error {
 	repo, err := git.PlainOpen(repoPath)
-	if err == nil {
+	if err != nil && err != git.ErrRepositoryNotExists {
+		return err
+	}
+
+	if err == nil {
 		worktree, err := repo.Worktree()
internal/cli/root.go (1)

77-80: Remove unnecessary HelpFunc wrapper.

The custom help function wrapper simply calls the original without any modifications. This can be removed.

 func Run() {
 	// Initialize CLI before running
 	InitCLI()
 
-	originalHelp := rootCmd.HelpFunc()
-	rootCmd.SetHelpFunc(func(c *cobra.Command, s []string) {
-		originalHelp(c, s)
-	})
-
 	if err := rootCmd.Execute(); err != nil {
 		log.Fatal().Msg(err.Error())
 	}
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2cef0d5 and 7d8a847.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (28)
  • .gitignore (1 hunks)
  • cmd/vt/main.go (1 hunks)
  • go.mod (11 hunks)
  • internal/cli/list.go (1 hunks)
  • internal/cli/root.go (3 hunks)
  • internal/cli/validate.go (0 hunks)
  • pkg/templates/downloader.go (1 hunks)
  • pkg/templates/template.go (1 hunks)
  • templates/vt-2024-53995/docker-compose.yaml (0 hunks)
  • templates/vt-2024-53995/index.yaml (0 hunks)
  • templates/vt-2024-53995/vt-2024-53995.nuclei.yaml (0 hunks)
  • templates/vt-2024-53995/vt-2024-53995.semgrep (0 hunks)
  • templates/vt-2025-29927/docker-compose.yaml (0 hunks)
  • templates/vt-2025-29927/index.yaml (0 hunks)
  • templates/vt-2025-29927/vt-2025-29927.nuclei.yaml (0 hunks)
  • templates/vt-2025-64459/docker-compose.yaml (0 hunks)
  • templates/vt-2025-64459/index.yaml (0 hunks)
  • templates/vt-2025-64459/vt-2025-64459.nuclei.yaml (0 hunks)
  • templates/vt-bwapp/docker-compose.yaml (0 hunks)
  • templates/vt-bwapp/index.yaml (0 hunks)
  • templates/vt-dvwa/docker-compose.yaml (0 hunks)
  • templates/vt-dvwa/index.yaml (0 hunks)
  • templates/vt-juice-shop/docker-compose.yaml (0 hunks)
  • templates/vt-juice-shop/index.yaml (0 hunks)
  • templates/vt-mutillidae-ii/docker-compose.yaml (0 hunks)
  • templates/vt-mutillidae-ii/index.yaml (0 hunks)
  • templates/vt-webgoat/docker-compose.yaml (0 hunks)
  • templates/vt-webgoat/index.yaml (0 hunks)
💤 Files with no reviewable changes (21)
  • templates/vt-juice-shop/index.yaml
  • templates/vt-2025-29927/vt-2025-29927.nuclei.yaml
  • templates/vt-2025-64459/index.yaml
  • templates/vt-2025-29927/index.yaml
  • templates/vt-bwapp/index.yaml
  • templates/vt-2024-53995/index.yaml
  • templates/vt-juice-shop/docker-compose.yaml
  • templates/vt-mutillidae-ii/docker-compose.yaml
  • internal/cli/validate.go
  • templates/vt-2024-53995/docker-compose.yaml
  • templates/vt-webgoat/index.yaml
  • templates/vt-2025-64459/docker-compose.yaml
  • templates/vt-2025-64459/vt-2025-64459.nuclei.yaml
  • templates/vt-dvwa/docker-compose.yaml
  • templates/vt-dvwa/index.yaml
  • templates/vt-2024-53995/vt-2024-53995.semgrep
  • templates/vt-mutillidae-ii/index.yaml
  • templates/vt-2024-53995/vt-2024-53995.nuclei.yaml
  • templates/vt-2025-29927/docker-compose.yaml
  • templates/vt-webgoat/docker-compose.yaml
  • templates/vt-bwapp/docker-compose.yaml
🧰 Additional context used
🧬 Code graph analysis (3)
cmd/vt/main.go (1)
internal/banner/banner.go (1)
  • Print (86-88)
pkg/templates/template.go (1)
pkg/templates/parser.go (1)
  • LoadTemplate (12-24)
internal/cli/list.go (1)
pkg/templates/template.go (2)
  • ListWithFilter (124-174)
  • Update (104-116)
🪛 OSV Scanner (2.3.0)
go.mod

[CRITICAL] 209-209: gopkg.in/src-d/go-git.v4 4.13.1: Path traversal and RCE in github.com/go-git/go-git/v5 and gopkg.in/src-d/go-git.v4

(GO-2024-2456)


[CRITICAL] 209-209: gopkg.in/src-d/go-git.v4 4.13.1: Denial of service in github.com/go-git/go-git/v5 and gopkg.in/src-d/go-git.v4

(GO-2024-2466)


[CRITICAL] 209-209: gopkg.in/src-d/go-git.v4 4.13.1: Clients vulnerable to DoS via maliciously crafted Git server replies in github.com/go-git/go-git

(GO-2025-3367)


[CRITICAL] 209-209: gopkg.in/src-d/go-git.v4 4.13.1: Argument Injection via the URL field in github.com/go-git/go-git

(GO-2025-3368)


[CRITICAL] 209-209: gopkg.in/src-d/go-git.v4 4.13.1: Maliciously crafted Git server replies can lead to path traversal and RCE on go-git clients

(GHSA-449p-3h89-pw88)


[CRITICAL] 209-209: gopkg.in/src-d/go-git.v4 4.13.1: Maliciously crafted Git server replies can cause DoS on go-git clients

(GHSA-mw99-9chc-xw7r)


[CRITICAL] 209-209: gopkg.in/src-d/go-git.v4 4.13.1: go-git clients vulnerable to DoS via maliciously crafted Git server replies

(GHSA-r9px-m959-cxf4)


[CRITICAL] 209-209: gopkg.in/src-d/go-git.v4 4.13.1: go-git has an Argument Injection via the URL field

(GHSA-v725-9546-7q7m)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Validate Template Files
  • GitHub Check: Lint
  • GitHub Check: Analyze (go)
🔇 Additional comments (6)
.gitignore (1)

34-34: LGTM!

Adding .vscode to .gitignore is a sensible housekeeping addition that aligns with the existing pattern of excluding IDE configuration directories (e.g., .idea).

cmd/vt/main.go (1)

11-20: LGTM!

Clean startup sequence: banner → logger → templates → CLI. The initialization order is correct.

internal/cli/list.go (1)

9-58: LGTM!

The command structure is well-organized with proper flag validation, mutual exclusion checks, and clear error handling.

pkg/templates/template.go (1)

54-66: LGTM on initialization flow.

The logic to check for existing templates and clone if missing is sound. The two-level directory structure iteration is correctly implemented.

pkg/templates/downloader.go (1)

46-54: LGTM on clone implementation.

Shallow clone with Depth: 1 is appropriate for distributing templates, reducing bandwidth and disk usage.

internal/cli/root.go (1)

59-70: LGTM!

Clean command registration with the new template command replacing list and validate commands.

Comment on lines +208 to +210
gopkg.in/src-d/go-billy.v4 v4.3.2 // indirect
gopkg.in/src-d/go-git.v4 v4.13.1 // indirect
gopkg.in/warnings.v0 v0.1.2 // indirect
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 5, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

cd . && go mod graph 2>/dev/null | grep -E "src-d/go-git|go-git" | head -20

Repository: HappyHackingSpace/vulnerable-target

Length of output: 1617


🏁 Script executed:

cat -n go.mod | sed -n '1,20p'

Repository: HappyHackingSpace/vulnerable-target

Length of output: 790


🌐 Web query:

gopkg.in/src-d/go-git.v4 v4.13.1 CVE vulnerabilities

💡 Result:

Yes — gopkg.in/src-d/go-git.v4 v4.13.1 is affected by multiple disclosed CVEs. Summary:

  • CVE-2025-21613 — Argument injection via the URL (allows setting arbitrary git-upload-pack flags when using the file:// transport). Affects >=4.0.0 and <=4.13.1; fixed in go-git v5.13.0 (recommend upgrading). [1][2]

  • CVE-2025-21614 — Denial‑of‑Service via maliciously crafted Git server replies (resource exhaustion). Affects >=4.0.0 and <=4.13.1; mitigation is to upgrade to v5.13.0+. [1][3]

  • CVE-2023-49568 — DoS from specially crafted server replies (affects 4.7.1–4.13.1); upgrade recommended. [4]

  • CVE-2023-49569 — Path‑traversal (can lead to RCE when using ChrootOS/plain clone modes); affects 4.0.0–4.13.1; upgrade recommended or avoid vulnerable filesystem modes. [5]

References:

  1. GHSA advisory (CVE-2025-21613). [GitHub Advisory]. [1]
  2. GHSA advisory (CVE-2025-21614). [GitHub Advisory]. [2]
  3. GitLab advisory page for CVE-2023-49568. [3]
  4. GitLab advisory page for CVE-2023-49569. [4]

Would you like exact links and remediation commands (go get / replace) for upgrading to a safe version?


Remove direct dependency on vulnerable gopkg.in/src-d/go-git.v4.

gopkg.in/src-d/go-git.v4 v4.13.1 is a direct dependency with 4 confirmed critical CVEs affecting this exact version:

This is not a transitive dependency—it's explicitly required. Remove this dependency entirely, or replace it with github.com/go-git/go-git/v5 (already in your dependencies at v5.16.4) if you need git functionality.

🧰 Tools
🪛 OSV Scanner (2.3.0)

[CRITICAL] 209-209: gopkg.in/src-d/go-git.v4 4.13.1: Path traversal and RCE in github.com/go-git/go-git/v5 and gopkg.in/src-d/go-git.v4

(GO-2024-2456)


[CRITICAL] 209-209: gopkg.in/src-d/go-git.v4 4.13.1: Denial of service in github.com/go-git/go-git/v5 and gopkg.in/src-d/go-git.v4

(GO-2024-2466)


[CRITICAL] 209-209: gopkg.in/src-d/go-git.v4 4.13.1: Clients vulnerable to DoS via maliciously crafted Git server replies in github.com/go-git/go-git

(GO-2025-3367)


[CRITICAL] 209-209: gopkg.in/src-d/go-git.v4 4.13.1: Argument Injection via the URL field in github.com/go-git/go-git

(GO-2025-3368)


[CRITICAL] 209-209: gopkg.in/src-d/go-git.v4 4.13.1: Maliciously crafted Git server replies can lead to path traversal and RCE on go-git clients

(GHSA-449p-3h89-pw88)


[CRITICAL] 209-209: gopkg.in/src-d/go-git.v4 4.13.1: Maliciously crafted Git server replies can cause DoS on go-git clients

(GHSA-mw99-9chc-xw7r)


[CRITICAL] 209-209: gopkg.in/src-d/go-git.v4 4.13.1: go-git clients vulnerable to DoS via maliciously crafted Git server replies

(GHSA-r9px-m959-cxf4)


[CRITICAL] 209-209: gopkg.in/src-d/go-git.v4 4.13.1: go-git has an Argument Injection via the URL field

(GHSA-v725-9546-7q7m)

Copy link
Member

Choose a reason for hiding this comment

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

Têlim tirşim, Têlim tirşim, Têlim tirşim. :>

Copy link
Contributor

Choose a reason for hiding this comment

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

@omarkurt, I understand! 😊 Security warnings can be overwhelming, especially when dealing with dependency chains. The vulnerability report will remain here for future reference, but feel free to address it when you have the bandwidth. If you need help with the migration to v5 later, just let me know!

┏(-_-)┓ ♪

Copy link
Member

Choose a reason for hiding this comment

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

btw, I opened issue on vt-templates HappyHackingSpace/vt-templates#3

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (1)
pkg/templates/downloader.go (1)

23-27: The typo has been fixed.

The past review comment about the typo "deteceted" → "detected" has been addressed. The error message now correctly reads "detected uncommitted changes".

🧹 Nitpick comments (4)
pkg/templates/downloader.go (4)

29-33: Consider making progress output configurable.

Writing git progress directly to os.Stdout may not be appropriate in all contexts (e.g., when used as a library, in automated pipelines, or with structured logging). Consider accepting an io.Writer parameter or using a logger.

Example refactor:

-func cloneTemplatesRepo(repoPath string, force bool) error {
+func cloneTemplatesRepo(repoPath string, force bool, progress io.Writer) error {
+	if progress == nil {
+		progress = io.Discard
+	}
 	// ...
 		err = worktree.Pull(&git.PullOptions{
 			RemoteName: "origin",
 			Force:      true,
-			Progress:   os.Stdout,
+			Progress:   progress,
 		})

47-47: Make the repository URL configurable.

The repository URL is hardcoded, which reduces flexibility. Consider making it a parameter or loading it from configuration to support different template repositories or testing with alternative sources.

Apply this diff to parameterize the URL:

-func cloneTemplatesRepo(repoPath string, force bool) error {
+func cloneTemplatesRepo(repoPath string, repoURL string, force bool) error {
 	// ...
 	_, err = git.PlainClone(repoPath, false, &git.CloneOptions{
-		URL:      "https://github.com/HappyHackingSpace/vt-templates",
+		URL:      repoURL,
 		Progress: os.Stdout,
 		Depth:    1,
 	})

Alternatively, define a constant at the package level:

const DefaultTemplatesRepoURL = "https://github.com/HappyHackingSpace/vt-templates"

46-50: Note the implications of shallow cloning.

Using Depth: 1 creates a shallow clone with only the latest commit. While this reduces bandwidth and disk usage, it limits the ability to work with git history, branches, tags, or perform operations like git log or git blame. Verify this is acceptable for the template management use case.


29-33: Add resilience patterns for network operations.

The git operations (Pull and Clone) lack timeout configuration and error retry logic, which could lead to hanging operations on network issues. Additionally, there's no authentication handling for rate limits or private repositories.

Consider:

  1. Adding context with timeout to git operations (go-git supports context in newer versions)
  2. Implementing retry logic with exponential backoff for transient failures
  3. Supporting authentication via SSH keys or personal access tokens for private repos or to avoid rate limiting

Example with context (verify go-git version supports this):

ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
defer cancel()

_, err = git.PlainCloneContext(ctx, repoPath, false, &git.CloneOptions{
	URL:      "https://github.com/HappyHackingSpace/vt-templates",
	Progress: os.Stdout,
	Depth:    1,
})

Also applies to: 46-50

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between daa435d and 7682cf4.

📒 Files selected for processing (1)
  • pkg/templates/downloader.go (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (go)

@omarkurt omarkurt added help wanted Extra attention is needed test labels Dec 18, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a2ca6ec and e04f1b9.

📒 Files selected for processing (3)
  • cmd/vt/main.go (2 hunks)
  • pkg/templates/downloader.go (1 hunks)
  • pkg/templates/template.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • pkg/templates/downloader.go
  • pkg/templates/template.go
🧰 Additional context used
🧬 Code graph analysis (1)
cmd/vt/main.go (4)
internal/banner/banner.go (1)
  • Print (86-88)
internal/cli/root.go (1)
  • Run (73-85)
pkg/templates/template.go (1)
  • Init (53-101)
internal/logger/logger.go (1)
  • Init (34-36)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Lint
  • GitHub Check: Analyze (go)
🔇 Additional comments (1)
cmd/vt/main.go (1)

5-5: Banner printing successfully moved to startup.

The banner import and print call are correctly placed after logger initialization and before CLI execution, aligning with the PR objectives.

Also applies to: 15-16

@recepgunes1 recepgunes1 requested a review from omarkurt December 18, 2025 14:34
@recepgunes1 recepgunes1 linked an issue Dec 18, 2025 that may be closed by this pull request
}

_, err = git.PlainClone(repoPath, false, &git.CloneOptions{
URL: "https://github.com/HappyHackingSpace/vt-templates",
Copy link
Contributor

Choose a reason for hiding this comment

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

url sabit const olmalı

Copy link
Member

Choose a reason for hiding this comment

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

+1

Copy link
Member

@dogancanbakir dogancanbakir left a comment

Choose a reason for hiding this comment

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

left some comment

}

_, err = git.PlainClone(repoPath, false, &git.CloneOptions{
URL: "https://github.com/HappyHackingSpace/vt-templates",
Copy link
Member

Choose a reason for hiding this comment

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

+1

}

if !force {
if !status.IsClean() {
Copy link
Member

Choose a reason for hiding this comment

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

nested IFs can be merged

if template.ID != entry.Name() {
log.Fatal().Msgf("id and directory name should match")

// Second level: iterate through templates within each category
Copy link
Member

Choose a reason for hiding this comment

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

if we think we need the comment maybe we should move the code into a func

}
}

// Update loads all templates from remote repository (force).
Copy link
Member

Choose a reason for hiding this comment

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

a better, descriptive name maybe?

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
pkg/templates/downloader.go (1)

24-35: Clarify the force parameter semantics or align the Pull behavior.

The force parameter only controls whether uncommitted changes are checked (line 24), but the Pull operation on line 30 always uses Force: true. This is semantically misleading—consider either:

  1. Using Force: force to align the parameter with the actual behavior, or
  2. Renaming the parameter to skipDirtyCheck to reflect its actual purpose.
🧹 Nitpick comments (2)
pkg/templates/template.go (1)

56-69: Consider extracting common path resolution logic.

Both Init() and SyncTemplates() duplicate the home directory retrieval and repoPath construction. A small helper would reduce repetition and ensure consistency.

🔎 Proposed helper
func getTemplatesRepoPath() (string, error) {
	homeDir, err := os.UserHomeDir()
	if err != nil {
		return "", err
	}
	return filepath.Join(homeDir, "vt-templates"), nil
}

Then both functions can call:

repoPath, err := getTemplatesRepoPath()
if err != nil {
    log.Fatal().Msgf("%v", err)
}

Also applies to: 107-118

pkg/templates/downloader.go (1)

33-35: Consider using errors.Is() for error comparison.

While direct comparison works for go-git's sentinel error, errors.Is() is more idiomatic and handles wrapped errors gracefully.

🔎 Proposed fix
-		if err != nil && err != git.NoErrAlreadyUpToDate {
+		if err != nil && !errors.Is(err, git.NoErrAlreadyUpToDate) {
 			return err
 		}
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1ca52a2 and db57ded.

📒 Files selected for processing (4)
  • cmd/vt/main.go
  • internal/cli/list.go
  • pkg/templates/downloader.go
  • pkg/templates/template.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • cmd/vt/main.go
🧰 Additional context used
🧬 Code graph analysis (2)
internal/cli/list.go (2)
internal/cli/root.go (1)
  • Run (73-85)
pkg/templates/template.go (2)
  • ListWithFilter (127-177)
  • SyncTemplates (107-119)
pkg/templates/downloader.go (1)
pkg/templates/template.go (1)
  • TemplateRemoteRepoistory (14-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (go)
🔇 Additional comments (4)
internal/cli/list.go (2)

9-58: LGTM! Clean command structure with proper flag validation.

The consolidated templateCmd cleanly handles mutual exclusion between --list and --update, validates that --filter requires --list, and displays help when no actionable flag is provided. The error handling for flag retrieval is appropriate.


60-64: setupTemplateCommand is correctly invoked during CLI initialization.

The function is properly called in registerCommands() at line 69, which is invoked from InitCLI() at line 55. The initialization flow (Run → InitCLI → registerCommands → setupTemplateCommand) ensures the template flags are registered before command execution.

pkg/templates/template.go (1)

76-103: LGTM! Template loading logic is well-structured.

The two-level iteration correctly skips hidden files (including .git), validates that template IDs match directory names, and provides clear error messages on failure.

pkg/templates/downloader.go (1)

40-62: LGTM! Clone flow handles edge cases correctly.

The implementation properly cleans up any existing state before cloning, uses shallow clone for efficiency, and correctly handles cleanup on failure with errors.Join. Update the constant reference on line 50 once the typo in TemplateRemoteRepoistory is fixed.

@recepgunes1 recepgunes1 requested a review from ahsentekd January 1, 2026 11:14
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (1)
pkg/templates/template.go (1)

13-14: Fix typo in constant name (previously flagged).

This issue was already identified in a previous review. The constant name contains a typo: "Repoistory" should be "Repository". Since this is an exported constant, fixing it now is important to avoid a breaking change later.

🧹 Nitpick comments (2)
pkg/templates/template.go (2)

55-72: LGTM with optional refinement.

The initialization logic is sound: it checks for the local templates directory, clones on first use, and loads templates. The error handling with log.Fatal() is appropriate for initialization failures.

Consider extracting the hardcoded "vt-templates" directory name (lines 62, 71) into a package-level constant for better maintainability and reusability (also used in SyncTemplates at line 122).

🔎 Proposed refactor

Add a constant near the top of the file:

const TemplatesDirectoryName = "vt-templates"

Then use it in the code:

-	repoPath := filepath.Join(homeDir, "vt-templates")
+	repoPath := filepath.Join(homeDir, TemplatesDirectoryName)

115-128: Improve log message accuracy and reduce code duplication.

Two issues to address:

  1. The log message "cloning" (line 123) is misleading since force=true means the function may update an existing repository rather than always cloning fresh.

  2. Code duplication: Both Init and SyncTemplates contain identical logic for obtaining the home directory and constructing the repo path (lines 117-122 here, 57-62 in Init).

🔎 Proposed refactor

Extract a helper function:

func getTemplatesRepoPath() (string, error) {
	homeDir, err := os.UserHomeDir()
	if err != nil {
		return "", err
	}
	return filepath.Join(homeDir, "vt-templates"), nil
}

Then update both functions to use it:

 func SyncTemplates() {
-	homeDir, err := os.UserHomeDir()
+	repoPath, err := getTemplatesRepoPath()
 	if err != nil {
 		log.Fatal().Msgf("%v", err)
 	}
-
-	repoPath := filepath.Join(homeDir, "vt-templates")
-	log.Info().Msgf("cloning %s", TemplateRemoteRepoistory)
+	log.Info().Msgf("Updating templates from %s", TemplateRemoteRepoistory)
 	err = cloneTemplatesRepo(repoPath, true)

Note: This also references the typo'd constant TemplateRemoteRepoistory which should be fixed as noted in the previous review.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between db57ded and af36a52.

📒 Files selected for processing (1)
  • pkg/templates/template.go
🧰 Additional context used
🧬 Code graph analysis (1)
pkg/templates/template.go (2)
internal/logger/logger.go (1)
  • Init (34-36)
pkg/templates/parser.go (1)
  • LoadTemplate (12-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (go)
🔇 Additional comments (2)
pkg/templates/template.go (2)

74-89: LGTM!

The directory loading logic correctly iterates through category directories, appropriately skips hidden files and non-directory entries, and delegates template loading to the category-specific function.


91-113: LGTM!

The category loading logic is well-implemented. The validation that ensures template.ID matches the directory name (lines 108-110) is particularly good for maintaining consistency between the filesystem structure and template metadata.

@dogancanbakir dogancanbakir merged commit 2901522 into main Jan 2, 2026
6 checks passed
@dogancanbakir dogancanbakir deleted the feat/template-migration branch January 2, 2026 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

help wanted Extra attention is needed test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

output improvment - custom message for vt template status

5 participants