Merge PR #25 and PR #27 into fix-conflicts-and-merge, resolving all conflicts#28
Conversation
…l.sh Agent-Logs-Url: https://github.com/root-Manas/macaron/sessions/00996222-29d9-4f07-861f-153472cf533b Co-authored-by: root-Manas <97402139+root-Manas@users.noreply.github.com>
…trings Agent-Logs-Url: https://github.com/root-Manas/macaron/sessions/d6bb3d0b-c7d5-40b3-b491-e195b8b62ce5 Co-authored-by: root-Manas <97402139+root-Manas@users.noreply.github.com>
…t, uninstall, more tools, new README Agent-Logs-Url: https://github.com/root-Manas/macaron/sessions/bd43ae69-d546-45ff-aed6-3450bdddd865 Co-authored-by: root-Manas <97402139+root-Manas@users.noreply.github.com>
…n validation, append bug, comments Agent-Logs-Url: https://github.com/root-Manas/macaron/sessions/bd43ae69-d546-45ff-aed6-3450bdddd865 Co-authored-by: root-Manas <97402139+root-Manas@users.noreply.github.com>
… buffer constant, recursive flattenAPIKeys comment, uppercase TLD support Agent-Logs-Url: https://github.com/root-Manas/macaron/sessions/bd43ae69-d546-45ff-aed6-3450bdddd865 Co-authored-by: root-Manas <97402139+root-Manas@users.noreply.github.com>
…on' into copilot/fix-conflicts-and-merge # Conflicts: # README.md # cmd/macaron/main.go # internal/app/app.go # internal/cliui/banner.go Co-authored-by: root-Manas <97402139+root-Manas@users.noreply.github.com>
…tool' into copilot/fix-conflicts-and-merge # Conflicts: # README.md # cmd/macaron/main.go # internal/app/app.go # internal/ui/assets/index.html # internal/ui/server.go Co-authored-by: root-Manas <97402139+root-Manas@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR merges two previously open PRs with overlapping changes, resolving conflicts by keeping the post-cleanup architecture (semantic CLI + dashboard removal) while carrying over select pipeline/key-management additions.
Changes:
- Replaces legacy flag-driven CLI with semantic subcommands (
scan|status|results|setup|export|config|api|uninstall|guide|version) and updates docs accordingly. - Removes the dashboard/UI (
internal/ui/...) and related documentation. - Expands the recon pipeline/tooling (amass, naabu fallback, gau/katana URL discovery) and adds/extends API key import/bulk loading support.
Reviewed changes
Copilot reviewed 12 out of 14 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Rewritten usage/docs to match the semantic CLI and dashboard removal; adds API management and pipeline/tool tables. |
| internal/ui/server.go | Deletes the dashboard HTTP server implementation. |
| internal/ui/assets/index.html | Deletes dashboard frontend assets. |
| internal/store/store.go | Adds cross-scan analytics aggregation (currently appears dashboard-oriented). |
| internal/model/model.go | Adds analytics-related model types. |
| internal/engine/engine.go | Adds amass, subfinder provider-config injection, naabu-backed port scanning, gau+katana URL discovery; renames temp file prefix. |
| internal/cliui/banner.go | Refactors banner/log helpers (PrintBanner takes an io.Writer; new prefixed logging helpers). |
| internal/cfg/config.go | Adds bulk key loading and import-from-tool-config helpers; adds subfinder provider-config writer. |
| internal/app/app.go | Expands tool catalog/roles, adjusts status/setup rendering. |
| install.sh | Updates install messaging and expands PATH setup to zsh/profile. |
| cmd/macaron/main.go | Implements the semantic subcommand CLI, API key subcommands, uninstall flow, and updates version to 3.1.0. |
| cmd/macaron/main_test.go | Removes legacy arg-normalization tests; adds tests for profiles, domain heuristic, and storage override. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fs.StringVar(&stages, "stages", "all", "Comma-separated stages: subdomains,http,ports,urls,vulns") | ||
| fs.StringVarP(&profile, "profile", "p", "balanced", "Workflow profile: passive|balanced|aggressive") | ||
| fs.BoolVarP(&quiet, "quiet", "q", false, "Suppress progress output") | ||
| fs.StringVar(&storage, "storage", "", "Storage root (default: ./storage)") | ||
| _ = fs.Parse(args) | ||
|
|
||
| // Positional args are also targets. | ||
| for _, a := range fs.Args() { | ||
| targets = append(targets, a) |
There was a problem hiding this comment.
All subcommands ignore the return value of fs.Parse (using _ = fs.Parse(args)). With pflag.ContinueOnError, parse errors (unknown flags, missing values, etc.) will be silently ignored and can cause the remaining tokens to be treated as positional targets/args (e.g., scanning --threads as a target). Please handle the error from Parse (and consider setting fs.SetOutput(io.Discard) if you want to fully control messaging).
| func apiUnset(args []string) int { | ||
| // Reuse set with empty value to delete. | ||
| if len(args) == 0 { | ||
| cliui.Err("usage: macaron api unset key [key ...]") | ||
| return 1 | ||
| } | ||
| // Append "=" to each key so ApplySetAPI treats it as a deletion. | ||
| kvs := make([]string, len(args)) | ||
| for i, k := range args { | ||
| kvs[i] = strings.TrimSuffix(k, "=") + "=" | ||
| } | ||
| return apiSet(kvs) | ||
| } |
There was a problem hiding this comment.
apiUnset doesn’t parse flags (e.g. --storage) and blindly treats every token as a key name. This breaks calls like macaron api unset --storage /tmp shodan (it will attempt to delete keys named --storage and /tmp). Consider giving unset its own FlagSet like set/list, or refactor to a shared helper that parses flags and then applies deletions.
| } | ||
| if len(summaries) == 0 { | ||
| return "No scans found. Run: macaron scan example.com\n", nil | ||
| return "no scans on record. run: macaron scan <target>", nil |
There was a problem hiding this comment.
The empty-state message returned here lacks a trailing newline. Since callers print it with fmt.Print(out), the next shell prompt/output will appear on the same line.
| return "no scans on record. run: macaron scan <target>", nil | |
| return "no scans on record. run: macaron scan <target>\n", nil |
| // Analytics returns aggregated statistics across all scans for display in the | ||
| // dashboard analytics tab. | ||
| func (s *Store) Analytics() (model.AnalyticsReport, error) { | ||
| report := model.AnalyticsReport{} | ||
|
|
||
| // Per-day scan counts and cumulative findings. | ||
| rows, err := s.db.Query(` | ||
| SELECT id, target, finished_at, duration_ms, stats_json |
There was a problem hiding this comment.
This adds dashboard-oriented analytics aggregation (Store.Analytics) even though the dashboard/UI is deleted in this branch. As-is, this method (and the added analytics model types) appear to be unused, and the comment references a dashboard analytics tab that no longer exists. Either wire this into an actual consumer (CLI/API) or remove it to avoid dead code and confusing docs.
| // AnalyticsReport is the response returned by /api/analytics. | ||
| type AnalyticsReport struct { | ||
| ScanCount int `json:"scan_count"` | ||
| AvgDurationMS int64 `json:"avg_duration_ms"` | ||
| Totals ScanStats `json:"totals"` | ||
| Days []DayStat `json:"days"` | ||
| TopTargets []TargetRank `json:"top_targets"` | ||
| SeverityDist []SeverityCount `json:"severity_dist"` | ||
| } |
There was a problem hiding this comment.
The comment says this struct is returned by /api/analytics, but the UI/server (and any API endpoints) were removed in this merge. Since there’s no /api/analytics handler in the repo, this docstring is currently incorrect/misleading (and the type appears unused).
| if err != nil { | ||
| // naabu is installed but failed (e.g. permissions); warn and fall through. | ||
| emit(progress, model.StageEvent{Type: model.EventWarn, Stage: "ports", Message: "naabu failed (" + err.Error() + ") — falling back to native port scan"}) | ||
| } else if len(hits) > 0 { |
There was a problem hiding this comment.
If naabu is installed and runs successfully but returns zero hits, this code falls back to the native scanner anyway. That means paying for two scans in the common ‘no open ports’ case. Consider falling back only when naabu errors (or when it’s known to be unusable), and treat a successful empty result as authoritative.
| } else if len(hits) > 0 { | |
| } else { | |
| // A successful naabu run is authoritative, even when it finds no open ports. |
| lines, _ := runLines(ctx, fmt.Sprintf("katana -u https://%s -silent -jc -d 3", shellEscape(host)), 5*time.Minute) | ||
| return lines |
There was a problem hiding this comment.
runKatana forces an https:// URL for every host. This will miss URL discovery on HTTP-only services (and can bias coverage depending on target). Consider trying both schemes, using http:// first (matching the prober’s scheme results), or letting katana handle scheme selection via its flags.
| lines, _ := runLines(ctx, fmt.Sprintf("katana -u https://%s -silent -jc -d 3", shellEscape(host)), 5*time.Minute) | |
| return lines | |
| targets := []string{ | |
| fmt.Sprintf("https://%s", host), | |
| fmt.Sprintf("http://%s", host), | |
| } | |
| all := make([]string, 0, 128) | |
| for _, target := range targets { | |
| lines, _ := runLines(ctx, fmt.Sprintf("katana -u %s -silent -jc -d 3", shellEscape(target)), 5*time.Minute) | |
| all = append(all, lines...) | |
| } | |
| return dedupe(all) |
| // writeSubfinderProviderConfig creates a temporary provider-config.yaml with | ||
| // macaron's API keys so subfinder uses them without changing the user's own | ||
| // subfinder config. Returns the temp file path, or "" if nothing to write. | ||
| // The caller is responsible for removing the returned file when done. | ||
| func writeSubfinderProviderConfig(apiKeys map[string]string) string { | ||
| if len(apiKeys) == 0 { | ||
| return "" | ||
| } | ||
| // macaron key name → subfinder provider name | ||
| mapping := map[string]string{ | ||
| "securitytrails": "securitytrails", | ||
| "virustotal": "virustotal", | ||
| "shodan": "shodan", | ||
| "binaryedge": "binaryedge", | ||
| "c99": "c99", | ||
| "chaos": "chaos", | ||
| "hunter": "hunter", | ||
| "intelx": "intelx", | ||
| "urlscan": "urlscan", | ||
| "zoomeye": "zoomeye", | ||
| "fullhunt": "fullhunt", | ||
| "github": "github", | ||
| "leakix": "leakix", | ||
| "netlas": "netlas", | ||
| "quake": "quake", | ||
| } |
There was a problem hiding this comment.
There are now two separate implementations for generating a subfinder provider config: this local writeSubfinderProviderConfig (with a limited mapping) and cfg.WriteSubfinderProviderConfig (with a much broader mapping). This duplication risks config drift and means keys like censys_id/censys_secret, fofa_*, etc. won’t be injected when running subfinder from the engine. Consider consolidating into a single shared implementation/mapping (e.g., reuse cfg’s writer with a temp file).
| if !noColor { | ||
| label = "\033[" + code + "m[" + tag + "]\033[0m" | ||
| } | ||
| fmt.Printf(label+" "+format+"\n", a...) |
There was a problem hiding this comment.
Info/OK/Warn currently write to stdout via fmt.Printf, while Err writes to stderr. This makes it hard to pipe command output cleanly (stdout gets mixed with informational logs) and is inconsistent with the previous behavior in this same file (all logs to stderr). Consider sending all log-prefixed lines to stderr, reserving stdout for command output.
| fmt.Printf(label+" "+format+"\n", a...) | |
| fmt.Fprintf(os.Stderr, label+" "+format+"\n", a...) |
| latest.txt | ||
| ``` | ||
|
|
||
| Override with `--storage /path/to/dir` or `MACARON_HOME` (env not yet supported — use the flag). |
There was a problem hiding this comment.
This sentence is internally inconsistent: it suggests overriding with MACARON_HOME, but also says the env var is not yet supported. The current macaronHome() implementation only supports --storage (and otherwise defaults to ./storage). Please either remove the MACARON_HOME mention or implement env var support to match the docs.
| Override with `--storage /path/to/dir` or `MACARON_HOME` (env not yet supported — use the flag). | |
| Override with `--storage /path/to/dir`. |
Two open PRs with overlapping file changes and incompatible architectural directions needed to be merged into the working branch with conflicts resolved.
Conflict resolution strategy
PR #27 (cleanup-repo-configuration) — the authoritative redesign, merged first:
README.md,cmd/macaron/main.go,internal/app/app.go,internal/cliui/banner.go→ took PR feat: major cleanup — remove dashboard, semantic CLI, banner, API management, uninstall, more tools #27's versionPR #25 (build-e2e-flow-research-tool) — incremental analytics/UX additions, merged second:
internal/ui/assets/index.html,internal/ui/server.go— modify/delete conflicts; kept the deletion since PR feat: major cleanup — remove dashboard, semantic CLI, banner, API management, uninstall, more tools #27 intentionally removed the dashboardREADME.md,cmd/macaron/main.go,internal/app/app.go— took HEAD (PR feat: major cleanup — remove dashboard, semantic CLI, banner, API management, uninstall, more tools #27) as it supersedes PR End-to-end research workflow: analytics, graceful shutdown, CLI fixes, UX improvements, README #25's incremental changes