Skip to content

fix: config validation#527

Merged
wolf31o2 merged 1 commit into
mainfrom
fix/config-validation
Feb 21, 2026
Merged

fix: config validation#527
wolf31o2 merged 1 commit into
mainfrom
fix/config-validation

Conversation

@wolf31o2

@wolf31o2 wolf31o2 commented Feb 21, 2026

Copy link
Copy Markdown
Member

Summary by cubic

Validate the submit URL during config load and harden request/file handling to block invalid endpoints, avoid leaks, and secure seed file permissions.

  • Bug Fixes
    • Config: parse Submit.Url, require http/https and host; fail fast if invalid.
    • Tx submit: justify #nosec G107 (URL validated) and defer resp.Body.Close() earlier.
    • Wallet: write seed.txt with OpenFile + O_TRUNC and 0600; log after successful write.

Written for commit 330b816. Summary will update on new commits.

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced validation of submit URL configuration to enforce proper protocol requirements and format standards.
    • Improved network request handling to prevent potential memory leaks during transaction operations.
    • Refined seed file creation with stricter security permissions and corrected logging sequence.

@wolf31o2
wolf31o2 requested a review from a team as a code owner February 21, 2026 22:57
@coderabbitai

coderabbitai Bot commented Feb 21, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@wolf31o2 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 24 minutes and 31 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

Walkthrough

The pull request implements three targeted improvements across configuration, transaction handling, and wallet operations. A new validateSubmit() method in the Config struct validates the Submit URL during configuration load, ensuring proper HTTP/HTTPS schemes and non-empty hosts. In transaction submission, the HTTP response body is now closed earlier via defer to prevent potential memory leaks. In wallet operations, seed file creation is replaced with an explicit OpenFile call using specific flags (O_WRONLY, O_CREATE, O_TRUNC) with 0600 permissions, and debug logging is repositioned to occur after successful write completion rather than before.

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title 'fix: config validation' is overly broad and vague. While the PR does add config validation (validateSubmit), it also includes unrelated fixes to HTTP response handling and file creation logic that have nothing to do with config validation. Revise the title to accurately reflect all significant changes, such as 'fix: add submit URL validation and improve resource cleanup' to capture both config validation and the other fixes included in this changeset.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/config-validation

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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No issues found across 3 files

Signed-off-by: Chris Gianelloni <wolf31o2@blinklabs.io>
@wolf31o2
wolf31o2 force-pushed the fix/config-validation branch from f53f8bd to 330b816 Compare February 21, 2026 23:01

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/tx/tx.go`:
- Line 542: The gosec suppression on the HTTP request call only silences G107
but not G704; update the inline lint annotation on the client.Do(req) call to
also suppress G704 (for example by extending the `#nosec` tag to include G704, or
restore the //nolint:gosec form) so the SSRF warning is intentionally silenced;
locate the client.Do(req) line in the tx-related function (the code using
variables client and req) and modify its comment to include both G107 and G704.

In `@internal/wallet/wallet.go`:
- Line 57: The file internal/wallet/wallet.go is failing gofumpt formatting at
the os.OpenFile call using seedPath; run the formatter and commit the change to
fix CI: execute gofumpt -w internal/wallet/wallet.go locally (or use your
editor's Go formatting command), verify the diff around the os.OpenFile/seedPath
usage and any nearby imports are updated, then add and commit the formatted file
so the pipeline passes.

Comment thread internal/tx/tx.go Outdated
Comment thread internal/wallet/wallet.go Outdated
@wolf31o2
wolf31o2 merged commit d8f530c into main Feb 21, 2026
11 checks passed
@wolf31o2
wolf31o2 deleted the fix/config-validation branch February 21, 2026 23:07
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