-
Notifications
You must be signed in to change notification settings - Fork 394
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Filling in threat mitigations in privacy-security-explainer.md #746
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor comments - this isn't a full review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking really good overall! A few nits, but I'm giving you my approval now because I'm not gonna be around to re-review till Monday.
The thought occurred to me last night that I might need to call out XRInputSource threats in their own section. Leaving myself a note to think about it on Monday |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is generally really great, thanks Nell! I've filed #747 and #748 as follow-ups after this PR lands. Some text comments inline as well.
Per discussion there are some additional topics (e.g. private browsing modes) in #638 that are not covered in this explainer (and also not blocking for this PR). @NellWaliczek I'll file issues for those topics as well.
23c5461
to
6f3581a
Compare
Ok, I think I've addressed everything except the XRInputSource concerns. I'll get to that after lunch. Meanwhile, I think folks can go through and double check the rest of the changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found some typos referencing the 'implied consent' anchor
7575ecb
to
7699636
Compare
After immersive-web#746 is merged, carry-forward work from this document into the privacy & security explainer is tracked at immersive-web#748, immersive-web#750, #753, immersive-web#754
Any last feedback? @avadacatavra? I'm going out of internet range for a few days starting tomorrow morning and want to get this landed before I do, so folks can start filing PRs on top of it. |
Already left my approval previously, but reaffirming that the latest text looks good to me! 👍 |
privacy-security-explainer.md
Outdated
|
||
If these requirements are not met and `unbounded` is listed in `XRSessionInit.requiredFeatures` then the promise returned from `requestSession()` must be rejected. Otherwise, the promise may be fulfilled but future calls to `XRSession.requestReferenceSpace()` must fail when passed `unbounded`. | ||
|
||
Once a session is created, developers may attempt to create a bounded reference space by passing `unbounded` into `XRSession.requestReferenceSpace()`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this supposed to be 'create an unbounded reference space' or am I missing something?
Looks great! One miniscule nit---are we using '6DOF' or '6-DOF'? 👍 from me |
File an issue to clarify no additional threats exist with the currently known hardware
94f3281
to
119a3bd
Compare
Looks like the answer was: neither, we use '6DoF'. Fixed both pieces of feedback. Sounds like we're good to go on this PR, so I'm gonna merge it and then disappear into the woods. |
Ok, hopefully in the home stretch now!
This PR is based on top of #739 and it is somewhat related, but is primarily attempting to incorporate the guidance that was laid out in #638 in a way that aligns with developer intent and the structure of the specification. In other words, it is intended to be the foundation of the spec text changes we need to make next. In that next PR, I'm expecting that the algorithms for each "signal" API call will be updated to depend on a combination of algorithms and internal variables which are described (roughly) in the bulleted lists in this PR.
Odds are I've missed or misrepresented some of the threats/mitigations covered in #638, so please do let me know what needs fixing up.
/fixes #393, #485