Skip to content

Conversation

asurkov
Copy link
Collaborator

@asurkov asurkov commented Oct 3, 2025

No description provided.

@asurkov asurkov requested a review from sidvishnoi October 3, 2025 21:03
@asurkov asurkov self-assigned this Oct 3, 2025
@asurkov asurkov changed the base branch from main to incomingpayment October 3, 2025 21:08
@asurkov asurkov changed the title Flow session-flow: add session initiation/termination sections Oct 3, 2025
Comment on lines 47 to 50
<dfn data-lt="wallet monetization restricted">Wallet monetization restricted</dfn>
given a |document:Document| and a [=wallet address=] |walletAddress:DOMString|
if the <a href="https://webmonetization.org/docs/references/csp-monetization-src/">`monetization-src`</a> directive
in the [=content security policy=] of the |document| restricts the |walletAddress|'s [=origin=].
Copy link
Member

Choose a reason for hiding this comment

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

Q: If there are redirects from a non-restricted origin to a restricted one, are they allowed by CSP?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good question, CSP should be applied to the final document I believe but which redirects do you refer specifically?

Copy link
Member

Choose a reason for hiding this comment

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

Consider using wallet address: https://sidvishnoi.com/.well-known/pay. It redirects to a GateHub wallet. On my website (https://sidvishnoi.com), if add monetization-src: 'self' CSP, I would expect my wallet address to not be blocked, regardless of where it redirects to later on. Otherwise, I'd need to add GateHub to my CSP as well.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I get it. So there's nothing to change/tweak in wording here, right?

@asurkov asurkov force-pushed the flow branch 2 times, most recently from 62f47de to eab16dc Compare October 7, 2025 18:49
Base automatically changed from incomingpayment to main October 8, 2025 09:34
<div class="algorithm">
<p>To <dfn>initiate sessions scoped to</dfn> the |root:HTMLElement|, run these steps:</p>
<ol>
<li>Let |linkElements:NodeList| be all [^link^] elements with `rel="monetization"` that are descendants of |root|.</li>
Copy link
Member

Choose a reason for hiding this comment

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

Do we include disabled elements here? Or handle them in "initiate a session"?
Maybe we should have an algorithm "get link elements".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it looks like initiate a session alg is the right place
not sure about specific "get link elements" alg because technically "initiate sessions scoped to" is that one

Let |websiteWalletDetails:WalletAddressDetails?| be the result of [=send a wallet address request=] with |websiteWalletAddress|.
</li>
<li>
If |websiteWalletDetails| is null, fire an `error` event on |linkElement| and return.
Copy link
Member

Choose a reason for hiding this comment

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

At what stage CSP is handled? Will a CSP error also result in error event on linkElement?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's part of [=wallet monetization restricted=] of the "initiate a session" alg. Not quite sure if we should keep silent or send an error. Do you want me to send an issue on this one?

Copy link
Member

Choose a reason for hiding this comment

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

Let's keep the same behavior as with other link tags.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

apparently it varies from browser to browser, it seems all are agree that error is reported in console such as "Content-Security-Policy: The page’s settings blocked a style (style-src-elem) at https://example.com/blocked.css from being applied because it violates the following directive: “style-src 'self'”, but nothing else in Firefox. Chromium fires error event. It's weird but apparently it's implementation dependent.

Do you think we should reflect that somehow in the spec?

Copy link
Member

Choose a reason for hiding this comment

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

I'd rather file an interop bug and see what browsers agree on. But adding a note here would be good as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd rather file an interop bug and see what browsers agree on. But adding a note here would be good as well.

right, do you know what's a typical procedure for this (cc'ing @lukewarlow )

Copy link
Member

@sidvishnoi sidvishnoi left a comment

Choose a reason for hiding this comment

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

Looks good to me. I've some concerns around CSP, but we can address them as a follow up.
I'll do some experiments around existing CSP to understand it better.

@asurkov asurkov closed this Oct 9, 2025
@sidvishnoi
Copy link
Member

Why closed?

@asurkov
Copy link
Collaborator Author

asurkov commented Oct 9, 2025

Why closed?

I merged the branch from command line since it UI was failing for me so closed this one

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