Skip to content

Add TT script enforcement tests using HTMLElement.innerText and Node.… #54548

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Aug 28, 2025
Merged

Add TT script enforcement tests using HTMLElement.innerText and Node.… #54548

merged 2 commits into from
Aug 28, 2025
+60 −0

Conversation

fred-wang
Copy link
Contributor

…textContent

Setters for HTMLElement.innerText and Node.textContent can be retrieve via HTMLElement.prototype.__lookupSetter__('innerText') and Node.prototype.__lookupSetter__('textContent') respectively. New tests are added to make sure they allow to set the script source of a <script> element (without requiring to pass a TrustedScript) but that execution of such a script with modified source is blocked.

@fred-wang fred-wang requested a review from lukewarlow August 27, 2025 14:46
@wpt-pr-bot wpt-pr-bot requested a review from mikewest August 27, 2025 14:46
lukewarlow
lukewarlow approved these changes Aug 27, 2025
View reviewed changes
@fred-wang
Copy link
Contributor Author

This aligns with the spec and WebKit's behavior.

These tests are failing in Chromium because it always treat this the same as setting via HTMLScriptElement.innerText and HTMLScriptElement.textContent.

cc @otherdaniel @smaug----

@lukewarlow
Copy link
Member

Btw is lookupsetter the only way to do this? Given it's deprecated should we use something like https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/getOwnPropertyDescriptor instead. So modern engines can pass these tests without needed unrelated legacy stuff?

@fred-wang fred-wang force-pushed the tt-script-innerText-textContent-lookupSetter branch from ace6c4d to fb2e546 Compare August 27, 2025 14:58
@fred-wang
Copy link
Contributor Author

Btw is lookupsetter the only way to do this? Given it's deprecated should we use something like https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/getOwnPropertyDescriptor instead. So modern engines can pass these tests without needed unrelated legacy stuff?

Right, I guess I can replace with Object.getOwnPropertyDescriptor(HTMLElement.prototype, "innerText").set

@lukewarlow
Copy link
Member

I'm just a little conscious of the fact both ladybird and servo are mid TT implementation so it would be nice to avoid obsolete stuff where possible. But perhaps that ship has sailed.

@fred-wang
Add TT script enforcement tests using HTMLElement.innerText and Node.…
5f3fdaf 
…textContent

Setters for `HTMLElement.innerText` and `Node.textContent` can be retrieve via
`Object.getOwnPropertyDescriptor()`. New tests are added to make sure they
allow to set the script source of a `<script>` element (without requiring to
pass a `TrustedScript`) but that execution of such a script with modified
source is blocked.
@fred-wang fred-wang force-pushed the tt-script-innerText-textContent-lookupSetter branch from fb2e546 to 5f3fdaf Compare August 27, 2025 15:23
@fred-wang
Copy link
Contributor Author

I'm just a little conscious of the fact both ladybird and servo are mid TT implementation so it would be nice to avoid obsolete stuff where possible. But perhaps that ship has sailed.

Yeah that makes sense. I've rewritten the test with Object.getOwnPropertyDescriptor. I think we don't really care how we retrieve the setters anyway, the important part is about how TT implementations will behave when they are used on scripts. So let's pick the standard API.

@community-tc-integration Community-TC Integration
Copy link

Uh oh! Looks like an error!

Client ID static/taskcluster/github does not have sufficient scopes and is missing the following scopes:

{
  "AnyOf": [
    "queue:rerun-task:taskcluster-github/WutXVwc0Qnu41CtdxoOJgA/WutXVwc0Qnu41CtdxoOJgA",
    "queue:rerun-task-in-project:none",
    {
      "AllOf": [
        "queue:rerun-task",
        "assume:scheduler-id:taskcluster-github/WutXVwc0Qnu41CtdxoOJgA"
      ]
    }
  ]
}

This request requires the client to satisfy the following scope expression:

{
  "AnyOf": [
    "queue:rerun-task:taskcluster-github/WutXVwc0Qnu41CtdxoOJgA/WutXVwc0Qnu41CtdxoOJgA",
    "queue:rerun-task-in-project:none",
    {
      "AllOf": [
        "queue:rerun-task",
        "assume:scheduler-id:taskcluster-github/WutXVwc0Qnu41CtdxoOJgA"
      ]
    }
  ]
}
  • method: rerunTask
  • errorCode: InsufficientScopes
  • statusCode: 403
  • time: 2025-08-28T07:16:50.773Z

@community-tc-integration Community-TC Integration
Copy link

Uh oh! Looks like an error!

Client ID static/taskcluster/github does not have sufficient scopes and is missing the following scopes:

{
  "AnyOf": [
    "queue:rerun-task:taskcluster-github/WutXVwc0Qnu41CtdxoOJgA/WutXVwc0Qnu41CtdxoOJgA",
    "queue:rerun-task-in-project:none",
    {
      "AllOf": [
        "queue:rerun-task",
        "assume:scheduler-id:taskcluster-github/WutXVwc0Qnu41CtdxoOJgA"
      ]
    }
  ]
}

This request requires the client to satisfy the following scope expression:

{
  "AnyOf": [
    "queue:rerun-task:taskcluster-github/WutXVwc0Qnu41CtdxoOJgA/WutXVwc0Qnu41CtdxoOJgA",
    "queue:rerun-task-in-project:none",
    {
      "AllOf": [
        "queue:rerun-task",
        "assume:scheduler-id:taskcluster-github/WutXVwc0Qnu41CtdxoOJgA"
      ]
    }
  ]
}
  • method: rerunTask
  • errorCode: InsufficientScopes
  • statusCode: 403
  • time: 2025-08-28T07:16:59.977Z

@fred-wang fred-wang closed this Aug 28, 2025
@fred-wang fred-wang reopened this Aug 28, 2025
@Ms2ger Ms2ger merged commit fa80f49 into master Aug 28, 2025
23 of 24 checks passed
@Ms2ger Ms2ger deleted the tt-script-innerText-textContent-lookupSetter branch August 28, 2025 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukewarlow lukewarlow lukewarlow approved these changes

@mikewest mikewest Awaiting requested review from mikewest

Assignees

@mikewest mikewest

Labels
trusted-types wg-s_webappsec
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@fred-wang @lukewarlow @mikewest @Ms2ger @wpt-pr-bot