Skip to content

Conversation

@definitelynotagoblin
Copy link
Contributor

@definitelynotagoblin definitelynotagoblin commented Nov 17, 2025

Prior to Go 1.23, negative serial numbers were allowed in certificates. Some clients are reporting errors using Azurehound in their proxy environments under their current configurations. We can relax our need for perfectly compliant certificates here to accommodate.

Summary by CodeRabbit

  • Chores
    • Added build-time configuration for enhanced certificate handling on non-Windows platforms.

@definitelynotagoblin definitelynotagoblin self-assigned this Nov 17, 2025
@definitelynotagoblin definitelynotagoblin added the bug Something isn't working label Nov 17, 2025
@coderabbitai
Copy link

coderabbitai bot commented Nov 17, 2025

Walkthrough

A //go:debug x509negativeserial=1 directive is added to main.go following the existing non-Windows build tags. This is a build-time debug flag directive with no impact on runtime behavior or code logic.

Changes

Cohort / File(s) Summary
Build Debug Directive
main.go
Added //go:debug x509negativeserial=1 directive after existing //go:build !windows and +build !windows lines

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

  • Minimal change: single debug directive addition with no functional impact
  • No code logic modifications or runtime behavior changes
  • No testing implications

Poem

🐰 A whisker-twitch of build flags bright,
No logic changed, just debug light!
The rabbit hops through tagged lines clean,
Where x509 secrets now are seen.
Compile with care, the directive's small—
But builds stronger, one and all! 🔐

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly references the main change: adding support for negative x509 certificate serials via a go:debug directive.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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 anemeth/x509-negative-serials

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining why this PR is needed, why this solution was chosen, and what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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

@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

🧹 Nitpick comments (1)
main.go (1)

21-22: The x509negativeserial directive is valid and correctly implemented.

This directive enables Go 1.23's pre-1.23 behavior for parsing certificates with negative serial numbers, addressing the breaking change in crypto/x509.ParseCertificate. The //go:debug pragma syntax shown is correct and documented.

Adding an inline comment explaining the directive's purpose would improve maintainability:

 //go:build !windows
 // +build !windows
 
+// Allow negative serial numbers in x509 certificates for proxy compatibility.
 //go:debug x509negativeserial=1
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 267ed0d and dab7635.

📒 Files selected for processing (1)
  • main.go (1 hunks)

@definitelynotagoblin definitelynotagoblin merged commit 41ed6d1 into main Nov 25, 2025
11 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants