Skip to content

Conversation

@licanhua
Copy link
Contributor

@licanhua licanhua commented Sep 8, 2022

Fixes #6692
Trusted when arbitrary HTML assignments into .innerHTML

code examples

const ttPolicy = window.trustedTypes?.createPolicy('adaptivecards-ui-testapp', {
    createHTML: value => value,
});

const trustedHtml = ttPolicy?.createHTML(inputsAsJson) ?? inputsAsJson;
retrievedInputsDiv.innerHTML = trustedHtml as string;

TT docs: https://developer.mozilla.org/en-US/docs/Web/API/Trusted_Types_API
A Guide to TT: https://web.dev/trusted-types/
TT w3c spec: https://w3c.github.io/webappsec-trusted-types/dist/spec/

Microsoft Reviewers: Open in CodeFlow

@ghost
Copy link

ghost commented Sep 8, 2022

Hi @licanhua. Thanks for helping make the AdaptiveCards JS renderer + tooling better. As additional verification, once the JS build succeeds, please go to the test site to test out your website/designer changes.

Copy link
Member

@paulcam206 paulcam206 left a comment

Choose a reason for hiding this comment

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

This change also needs to be tested -- please let me know how that's being done...

@ghost ghost removed the Needs: Author Feedback label Sep 9, 2022
@licanhua licanhua requested a review from paulcam206 September 9, 2022 17:40
Copy link
Member

@paulcam206 paulcam206 left a comment

Choose a reason for hiding this comment

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

changes look good -- how did you test?

@licanhua
Copy link
Contributor Author

licanhua commented Sep 9, 2022

changes look good -- how did you test?

I did manual testing, and the designer looks good.
ideally, I should follow https://web.dev/trusted-types/#identify-trusted-types-violations, but I don't have time to do it right now

@licanhua
Copy link
Contributor Author

changes look good -- how did you test?

I did manual testing, and the designer looks good. ideally, I should follow https://web.dev/trusted-types/#identify-trusted-types-violations, but I don't have time to do it right now

Add this line to head of html and use the https link

    <meta http-equiv="Content-Security-Policy" content="require-trusted-types-for 'script';" />

Here is the error without the fix.
image

@licanhua licanhua requested a review from paulcam206 September 12, 2022 18:43
Copy link
Member

@paulcam206 paulcam206 left a comment

Choose a reason for hiding this comment

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

one last thing 😃

@licanhua licanhua merged commit bd24a4e into main Sep 13, 2022
@licanhua licanhua deleted the paulcam/trusted-types branch September 13, 2022 20:02
return false;
}

// Markdown processing is handled outside of Adaptive Cards. It's up to the host to ensure that markdown is safely
Copy link
Member

Choose a reason for hiding this comment

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

@licanhua , if you are shifting responsibility for markdown HTML sanitization outside of AC SDK, can you change onProcessMarkdown callback so that host can return TrustedHTML instead of string and then AC SDK will pass-through the value to innerHTML setter without converting to string?

This way the host will indeed be responsible (will have to deal with Trusted Types) and AC SDK will not blindly mark as Trusted data that is outside of its control.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@paulcam206 and @dclaux, any concern for me to change to signature of onProcessMarkdown to TrustedHTML?

image

@licanhua licanhua mentioned this pull request Sep 15, 2022
michaelfarnsworth pushed a commit to michaelfarnsworth/AdaptiveCards that referenced this pull request Nov 10, 2022
* [JS] Implement Trusted Types

Fixes microsoft#6692

* use trustedHtml for innerHtml=

* fix build error on calendar.ts

* revert change on tsconfig

* revert change on adaptivecards-designer and use emptyhtml

* use emptyHTML

* remove setInnerHtml for adaptivecards-controls

* apply policy to adaptivecards-designer

* Revert "apply policy to adaptivecards-designer"

This reverts commit 4cfd284.

Co-authored-by: Paul Campbell <paulcam@microsoft.com>
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.

[Feature Request] Trusted Types support in JavaScript SDK

4 participants