fix: prevent unnecessary template updates#6379
Conversation
Signed-off-by: Dwi Siswanto <git@dw1.io>
when version API fails. * fix `catalog/config.IsOutdatedVersion` logic for empty version strings * add GitHub API fallback when PDTM API is unavail * only show outdated msg for actual version mismatches Signed-off-by: Dwi Siswanto <git@dw1.io>
WalkthroughUpdates version comparison logic to avoid treating empty latest as outdated, refines fallback comparison behavior, adjusts installer’s update decision and logging based on availability of latest version (including a GitHub fallback), and adds unit tests for IsOutdatedVersion. Changes
Sequence Diagram(s)sequenceDiagram
participant N as Nuclei
participant PDTM as PDTM API
participant GH as GitHub Releases
participant FS as Filesystem
N->>PDTM: Fetch LatestNucleiTemplatesVersion
alt Latest available
N->>N: Compare current vs latest
else Latest empty/unavailable
N->>GH: Fetch latest release tag
N->>N: Compare current vs GH latest
end
alt Outdated
N->>FS: Download and write templates
N->>FS: Update checksums and index
N->>N: Info log: updated
else Not outdated
N->>N: Debug log: forced/no update
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
pkg/catalog/config/constants.go (1)
59-62: Normalize fallback comparison to avoid spurious updates on minor formatting differencesWhen semver parsing fails, a straight string inequality can still generate false positives due to whitespace or a “-dev” suffix on latest. Normalize both sides before comparing.
Apply this localized diff:
- // fallback to naive comparison - return true only if they are different - return current != latest + // fallback to naive comparison - normalize to avoid spurious mismatches + c := strings.TrimSpace(current) + l := strings.TrimSpace(trimDevIfExists(latest)) + return c != lpkg/installer/template_test.go (1)
63-100: Add a couple more edge cases (optional) to harden behavior
- Latest is a dev tag while current is release (should not trigger update).
- Parallelize test for speed when running full suite.
You can extend the table like this:
@@ }{ // Test the empty latest version case (main bug fix) {"v10.2.7", "", false, "Empty latest version should not trigger update"}, @@ // Test dev versions {"v10.2.7-dev", "v10.2.7", false, "Dev version matching release should not trigger update"}, {"v10.2.6-dev", "v10.2.7", true, "Outdated dev version should trigger update"}, + {"v10.2.7", "v10.2.7-dev", false, "Release vs dev latest should not trigger update"}, @@ for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { + t.Parallel() result := config.IsOutdatedVersion(tc.current, tc.latest)Note: If you adopt the fallback normalization suggested in constants.go, you may also want to add a whitespace-only case to ensure it doesn’t cause false positives.
pkg/installer/template.go (1)
162-169: Clarify log message when forcing an update without a version bump (nit)“forced update” can be ambiguous; consider indicating that you’re refreshing templates without a version bump to reduce confusion.
For example:
- gologger.Debug().Msgf("Updating nuclei-templates from %s to %s (forced update)\n", currentVersion, latestVersion) + gologger.Debug().Msgf("Refreshing nuclei-templates without a version bump: %s -> %s\n", currentVersion, latestVersion)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pkg/catalog/config/constants.go(1 hunks)pkg/installer/template.go(2 hunks)pkg/installer/template_test.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
pkg/installer/template_test.go (1)
pkg/catalog/config/constants.go (1)
IsOutdatedVersion(47-65)
pkg/installer/template.go (2)
pkg/catalog/config/nucleiconfig.go (1)
DefaultConfig(23-23)pkg/catalog/config/constants.go (2)
OfficialNucleiTemplatesRepoName(26-26)IsOutdatedVersion(47-65)
⏰ 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: Lint
🔇 Additional comments (3)
pkg/catalog/config/constants.go (2)
49-52: Returning false when latest is empty prevents false-positive updates — good fixThis directly addresses the root cause from #6375 and aligns with the PR objective to avoid unnecessary template updates/logging when the latest version is unavailable.
49-52: All call sites correctly handle “empty latest ⇒ not outdated”I’ve audited every reference to IsOutdatedVersion and uses of LatestNucleiTemplatesVersion and found no callers that assume an empty “latest” means “outdated.” In particular:
• pkg/installer/util.go
– Always passes a non‐empty literal for latest, so the new behavior is a no‐op.
• pkg/installer/template.go
– Wraps IsOutdatedVersion in a guard that falls back to GitHub when LatestNucleiTemplatesVersion (“PDTM API”) is empty.
• pkg/catalog/config/nucleiconfig.go
– Uses(c.TemplateVersion == "" || IsOutdatedVersion(...) || !FolderExists)so an empty latest simply skips the outdated check (new semantics).
• internal/runner/runner.go
– Only logs version info; does not drive “outdated” logic.
• lib/sdk_private.go
– Invokes the onUpdateAvailableCallback with the LatestNucleiTemplatesVersion, but does not treat an empty string as a trigger for an update.All existing fallbacks or guards remain intact, and no callers break under the new “return false on empty latest” rule. No further changes are needed.
pkg/installer/template_test.go (1)
63-100: Solid coverage of IsOutdatedVersion across key scenariosTable-driven tests validate empty-latest, equal, older/newer, dev, and invalid fallback paths. This guards the changed semantics well.
| needsUpdate := config.DefaultConfig.NeedsTemplateUpdate() | ||
|
|
||
| // NOTE(dwisiswant0): if PDTM API data is not available | ||
| // (LatestNucleiTemplatesVersion is empty) but we have a current template | ||
| // version, so we MUST verify against GitHub directly. | ||
| if !needsUpdate && config.DefaultConfig.LatestNucleiTemplatesVersion == "" && config.DefaultConfig.TemplateVersion != "" { | ||
| ghrd, err := updateutils.NewghReleaseDownloader(config.OfficialNucleiTemplatesRepoName) | ||
| if err == nil { | ||
| latestVersion := ghrd.Latest.GetTagName() | ||
| if config.IsOutdatedVersion(config.DefaultConfig.TemplateVersion, latestVersion) { | ||
| needsUpdate = true | ||
| gologger.Debug().Msgf("PDTM API unavailable, verified update needed via GitHub API: %s -> %s", config.DefaultConfig.TemplateVersion, latestVersion) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if needsUpdate { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden GitHub fallback: log failures and ignore empty tag names
Good call adding a GH fallback when PDTM is unavailable. To aid diagnostics and avoid calling IsOutdatedVersion with an empty tag, log the failure path and guard against empty tag names.
Apply this diff:
if !needsUpdate && config.DefaultConfig.LatestNucleiTemplatesVersion == "" && config.DefaultConfig.TemplateVersion != "" {
ghrd, err := updateutils.NewghReleaseDownloader(config.OfficialNucleiTemplatesRepoName)
- if err == nil {
- latestVersion := ghrd.Latest.GetTagName()
- if config.IsOutdatedVersion(config.DefaultConfig.TemplateVersion, latestVersion) {
- needsUpdate = true
- gologger.Debug().Msgf("PDTM API unavailable, verified update needed via GitHub API: %s -> %s", config.DefaultConfig.TemplateVersion, latestVersion)
- }
- }
+ if err != nil {
+ gologger.Debug().Msgf("GitHub API fallback unavailable: %v", err)
+ } else {
+ latestVersion := strings.TrimSpace(ghrd.Latest.GetTagName())
+ if latestVersion != "" && config.IsOutdatedVersion(config.DefaultConfig.TemplateVersion, latestVersion) {
+ needsUpdate = true
+ gologger.Debug().Msgf("PDTM API unavailable; verified update needed via GitHub API: %s -> %s", config.DefaultConfig.TemplateVersion, latestVersion)
+ }
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| needsUpdate := config.DefaultConfig.NeedsTemplateUpdate() | |
| // NOTE(dwisiswant0): if PDTM API data is not available | |
| // (LatestNucleiTemplatesVersion is empty) but we have a current template | |
| // version, so we MUST verify against GitHub directly. | |
| if !needsUpdate && config.DefaultConfig.LatestNucleiTemplatesVersion == "" && config.DefaultConfig.TemplateVersion != "" { | |
| ghrd, err := updateutils.NewghReleaseDownloader(config.OfficialNucleiTemplatesRepoName) | |
| if err == nil { | |
| latestVersion := ghrd.Latest.GetTagName() | |
| if config.IsOutdatedVersion(config.DefaultConfig.TemplateVersion, latestVersion) { | |
| needsUpdate = true | |
| gologger.Debug().Msgf("PDTM API unavailable, verified update needed via GitHub API: %s -> %s", config.DefaultConfig.TemplateVersion, latestVersion) | |
| } | |
| } | |
| } | |
| if needsUpdate { | |
| needsUpdate := config.DefaultConfig.NeedsTemplateUpdate() | |
| // NOTE(dwisiswant0): if PDTM API data is not available | |
| // (LatestNucleiTemplatesVersion is empty) but we have a current template | |
| // version, so we MUST verify against GitHub directly. | |
| if !needsUpdate && config.DefaultConfig.LatestNucleiTemplatesVersion == "" && config.DefaultConfig.TemplateVersion != "" { | |
| ghrd, err := updateutils.NewghReleaseDownloader(config.OfficialNucleiTemplatesRepoName) | |
| if err != nil { | |
| gologger.Debug().Msgf("GitHub API fallback unavailable: %v", err) | |
| } else { | |
| latestVersion := strings.TrimSpace(ghrd.Latest.GetTagName()) | |
| if latestVersion != "" && config.IsOutdatedVersion(config.DefaultConfig.TemplateVersion, latestVersion) { | |
| needsUpdate = true | |
| gologger.Debug().Msgf("PDTM API unavailable; verified update needed via GitHub API: %s -> %s", config.DefaultConfig.TemplateVersion, latestVersion) | |
| } | |
| } | |
| } | |
| if needsUpdate { |
🤖 Prompt for AI Agents
In pkg/installer/template.go around lines 98 to 114, the GitHub fallback
currently ignores errors and may call IsOutdatedVersion with an empty tag;
update it to (1) log the error when updateutils.NewghReleaseDownloader(...)
returns a non-nil err so failures are visible, (2) obtain latestVersion only
when ghrd != nil and ensure latestVersion is non-empty before calling
config.IsOutdatedVersion(...), and (3) log a debug/info message when the latest
tag is empty and the check is skipped. Ensure logs include context (e.g., that
PDTM was unavailable and this is the GitHub fallback) and do not change control
flow beyond setting needsUpdate when a valid newer tag is found.
this shouldn’t occur anymore if nuclei-templates rolls out a new release for projectdiscovery/nuclei-templates#12881. |
ehsandeep
deleted the
dwisiswant0/fix/prevent-unnecessary-template-updates
branch
3 participants
Proposed changes
when version API fails.
catalog/config.IsOutdatedVersionlogic forempty version strings
mismatches
fixes #6375.
Proof
Checklist
Summary by CodeRabbit
Bug Fixes
Tests