backend: require HTTPS AOPP callbacks - #4342
Open
benma-agent wants to merge 1 commit into
Open
Conversation
benma
marked this pull request as ready for review
August 20, 2026 15:08
benma-agent
force-pushed
the
benma-agent/restrict-aopp-callback-https
branch
from
August 20, 2026 15:08
9a77438 to
25f81c3
Compare
bznein
reviewed
Aug 20, 2026
bznein
left a comment
Collaborator
There was a problem hiding this comment.
Codex findings:
• The HTTPS restriction can still downgrade through redirects, and its newly rejected
HTTP path sends frontend state that crashes error rendering. Both defects undermine
the intended secure failure behavior.
Full review comments:
- [P1] Preserve rejected callbacks for error rendering — /home/bznein-coding-agent/
agent-work/bitbox-wallet-app/backend/aopp.go:260-262
When an AOPP URI contains a parseable HTTP callback, this branch sets the error
before backend.aopp.Callback is assigned, so the frontend receives an empty
callback. The error view calls new URL(aopp.callback).host in frontends/web/src/
components/aopp/aopp.tsx, which throws and blanks the flow instead of displaying
aoppInvalidRequest; preserve the parseable callback or make error rendering tolerate absent callbacks.
- [P1] Reject cleartext redirect targets — /home/bznein-coding-agent/agent-work/
bitbox-wallet-app/backend/aopp.go:257-259
When an accepted HTTPS callback responds with a 307 or 308 redirect to an HTTP URL,
Go's default http.Client.Post follows it and resends the signed address and optional xpub over cleartext. Validating only the initial URL therefore does not enforce the
new transport guarantee; disable redirects for this callback or reject every
redirect whose target is not HTTPS.
| backend.aoppSetError(errAOPPInvalidRequest) | ||
| return | ||
| } | ||
| if !callbackURL.IsAbs() || |
Collaborator
There was a problem hiding this comment.
[nit] I think we can avoid this check, because this simply does:
func (u *URL) IsAbs() bool {
return u.Scheme != ""
}And if the scheme is empty it would fail anyway the EqualFold check blow.
Reject AOPP requests whose callback is not an absolute HTTPS URL with a non-empty host. Allow redirects only while HTTPS is preserved so signed addresses and extended public keys cannot be downgraded to cleartext transport. Make AOPP error rendering tolerate missing or malformed callback URLs. Cover callback validation, HTTPS redirects, HTTP downgrade rejection, and missing callbacks in backend and frontend tests.
benma-agent
force-pushed
the
benma-agent/restrict-aopp-callback-https
branch
from
August 20, 2026 20:59
25f81c3 to
351a1b9
Compare
Contributor
|
Addressed. It's overengineered for sure for something that is basically a non-issue in practice, but oh well... |
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
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.
Reject AOPP requests whose callback is not an absolute HTTPS URL
with a non-empty host. Allow redirects only while HTTPS is preserved
so signed addresses and extended public keys cannot be downgraded to
cleartext transport.
Make AOPP error rendering tolerate missing or malformed callback URLs.
Cover callback validation, HTTPS redirects, HTTP downgrade rejection,
and missing callbacks in backend and frontend tests.