implement ptero 1.12.3 security fix

[parkervcp]

implement security change from ptero 1.12.3

code cleanup

Summary by CodeRabbit

Release Notes

• Refactor

  • Improved error handling in configuration value lookup to better manage placeholder substitution.

• Bug Fixes

  • Fixed Docker configuration to properly populate interface settings from network configuration.

[coderabbitai]

📝 Walkthrough

Walkthrough

Configuration placeholder substitution in LookupConfigurationValue was refactored to use named return values and improved error handling within the closure. A new templatableConfig helper was added to bridge Docker.Interface and Docker.Network.Interface fields, with Parse updated to marshal the templated config.

Changes

Configuration Parsing Updates

Layer / File(s)	Summary
LookupConfigurationValue error handling refactor parser/helpers.go	Method now uses named return parameters result and err. The closure calls jsonparser.Get for each placeholder, preserves placeholders when paths are missing (with logging), avoids substituting non-scalar JSON types, and properly propagates errors via the named err variable.
Docker interface templating helper parser/parser.go	New templatableConfig struct and newTemplatableConfig constructor copy Docker.Network.Interface into both Docker.Interface and Docker.Network.Interface. The Parse method now marshals the templated config instead of the raw config returned by config.Get().

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 With named returns and closures tight,
Placeholders dance through config's night.
Docker interfaces now align with care,
Templates bloom in the parsing air! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'implement ptero 1.12.3 security fix' directly references a specific version security update and matches the PR's stated objective to implement the security change from ptero 1.12.3.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

parser/helpers.go (1)

207-223: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Stop silently swallowing config lookup errors caused by err shadowing
In parser/helpers.go (LookupConfigurationValue), the closure does match, dataType, _, err := jsonparser.Get(...), which shadows the function’s named return err. On non-jsonparser.KeyPathNotFoundError failures, the closure returns placeholder without assigning to the named return variable, so the caller doesn’t see the lookup error.

💡 Suggested fix

 result = configMatchRegex.ReplaceAllStringFunc(cfr.ReplaceWith.String(), func(placeholder string) string {
 	if err != nil {
 		return placeholder
 	}
 	keyPath := configMatchRegex.ReplaceAllString(placeholder, "$1")
 
 	var path []string
 	for _, part := range strings.Split(keyPath, ".") {
 		path = append(path, strcase.ToSnake(part))
 	}
 
 	// Look for the key in the Wings configuration and substitute the placeholder.
-	match, dataType, _, err := jsonparser.Get(f.configuration, path...)
-	if err != nil {
-		if err != jsonparser.KeyPathNotFoundError {
+	match, dataType, _, getErr := jsonparser.Get(f.configuration, path...)
+	if getErr != nil {
+		if getErr != jsonparser.KeyPathNotFoundError {
+			err = getErr
 			return placeholder
 		}
 		log.WithFields(log.Fields{"path": path, "filename": f.FileName}).Debug("attempted to load a configuration value that does not exist")
 		// Leave placeholder intact so the misconfiguration is visible.
 		return placeholder
 	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@parser/helpers.go` around lines 207 - 223, The closure inside
LookupConfigurationValue is shadowing the outer named return err by using :=
when calling jsonparser.Get, causing real lookup errors to be swallowed; change
the variable binding so the call to jsonparser.Get assigns to the outer err (use
= instead of := or assign to a new local errVar and set the outer err before
returning) and ensure any non-jsonparser.KeyPathNotFoundError paths set the
function's named err before returning the placeholder; update the closure around
configMatchRegex.ReplaceAllStringFunc (the block using cfr.ReplaceWith.String(),
keyPath, path, and jsonparser.Get) to fix the shadowing so callers receive the
actual error.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@parser/helpers.go`:
- Around line 207-223: The closure inside LookupConfigurationValue is shadowing
the outer named return err by using := when calling jsonparser.Get, causing real
lookup errors to be swallowed; change the variable binding so the call to
jsonparser.Get assigns to the outer err (use = instead of := or assign to a new
local errVar and set the outer err before returning) and ensure any
non-jsonparser.KeyPathNotFoundError paths set the function's named err before
returning the placeholder; update the closure around
configMatchRegex.ReplaceAllStringFunc (the block using cfr.ReplaceWith.String(),
keyPath, path, and jsonparser.Get) to fix the shadowing so callers receive the
actual error.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ca042a16-088b-42fa-b04f-f17098b25ddc

📥 Commits

Reviewing files that changed from the base of the PR and between 89b5212 and 4efe31c.

📒 Files selected for processing (2)

• parser/helpers.go
• parser/parser.go

📜 Review details

⏰ 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: Build and Test (ubuntu-22.04, 1.26.0, linux, amd64)
• GitHub Check: Build and Test (ubuntu-22.04, 1.25.7, linux, amd64)
• GitHub Check: Analyze (go)

🔇 Additional comments (1)

parser/parser.go (1)

216-230: LGTM!

Also applies to: 238-238