sdk/ldaputil: fix malformed error messages from errwrap placeholder - #32064
Closed
drewmullen wants to merge 1 commit into
Closed
sdk/ldaputil: fix malformed error messages from errwrap placeholder#32064drewmullen wants to merge 1 commit into
drewmullen wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
dosubot
Bot
added
the
size:S
|
Deployment failed with the following error: Learn More: https://vercel.com/docs/concepts/projects/project-configuration |
drewmullen
requested a deployment
to
community-pull-request
July 31, 2026 14:10 — with 
GitHub Actions
Waiting
GitHub Actions
Waiting
Six call sites built a format string containing the errwrap placeholder
`{{err}}` and passed it to fmt.Errorf with an error argument. fmt.Errorf
does not interpret `{{err}}`, and the format string had no verb to consume
the argument, so the rendered message contained both the literal placeholder
and Go's `%!(EXTRA ...)` marker:
error connecting to host "ldaps://dc.example.com:636": {{err}}%!(EXTRA
*ldap.Error=LDAP Result Code 200 "Network Error": dial tcp ...)
Because %w was not used, the underlying error was also not wrapped, so
errors.Is and errors.As could not reach it.
go vet does not catch this: fmt.Sprintf makes the format string
non-constant, so printf analysis is skipped.
Affects the LDAP auth method and, via ldaputil, the LDAP/AD secrets engines.
Fixes hashicorp#32063
drewmullen
force-pushed
the
fix/ldaputil-errwrap-placeholder
branch
from
July 31, 2026 14:14
d607735 to
1699f0e
Compare
Contributor
Author
|
opened in enterprise repo, as requested: https://github.com/hashicorp/vault-enterprise/pull/17089 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #32063
What
Six call sites in
sdk/helper/ldaputil/client.gobuilt a format string containing theerrwrapplaceholder{{err}}and passed it tofmt.Errorfalong with anerror.fmt.Errorfdoes not interpret{{err}}, and the format string had no verb to consume the argument, so Go appended%!(EXTRA ...).Let me know if you want me to adjust the test to not hardcode old behavior. it was useful for validation purposes for this PR so i left it in.
Before:
After:
Switching to
%walso wraps the underlying error, soerrors.Isanderrors.Asreach it. They could not before.Why it went unnoticed
go vetdoes not flag the original form.fmt.Sprintfmakes the format string non-constant, so the printf analyser skips it. Confirmed by runninggo vet ./helper/ldaputil/against the unfixed code: clean.Elsewhere in the codebase
{{err}}is used correctly witherrwrap.Wrapf, which does interpret it. Only these six passed it tofmt.Errorf.Scope
All six call sites are in
sdk/helper/ldaputil/client.go:error parsing url %qerror connecting to host %qSID %#v convert failed reading RevisionSID %#v convert failed reading SubAuthorityCountSID %#v convert failed reading IdentifierAuthoritySID %#v convert failed reading SubAuthorityldaputilis used by the built-in LDAP auth method and, throughvault-plugin-secrets-openldap, by the LDAP/AD secrets engines, so this affects both authentication and dynamic credential paths.Test
Added
TestSIDBytesToStringErrorFormatting, which exercises the SID conversion path (no network required) and asserts the rendered error contains neither{{err}}nor%!(EXTRA, and thaterrors.Isreaches the wrappedio.EOF.Verified the test fails against the unfixed code and passes with the change:
go build,go vetandgo test ./helper/ldaputil/all pass on the change.Impact on users
Error text changes. Anything matching on the literal
{{err}}or%!(EXTRAstrings would need updating, though such matching seems unlikely to be deliberate.TODO only if you're a HashiCorp employee
backport/label that matches the desired release branch.PCI review checklist
Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.
Co-authored: 🤖 Claude Code