Skip to content
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

Add security warning for ambiguous source maps #137

Merged

Conversation

nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Oct 10, 2024

It was pointed out during the October 2024 plenary that being able to cause a tool to perform a network request can be used for tracking purposes, and thus it should be possible to statically know whether a file links to a source map or not.

The problem is that, given that we have multiple linking methods that give different results, it's not unlikely that ambiguous source map comments can skip through reviews and checks.

There was a solution that we collectively came up with, but we did not want to change the specification at this point given that we need to discuss it in TG4 and go through the implications of it.

Approval on publishing the first edition of our spec was conditional of explicitly calling out (in a note / not normative section) the implications of the ambiguity, and how a potential solution would look like. Our specification already points to the living draft, and thus it's ok if we just work on the actual fix in the living draft.

The proposed fix is very likely to affect no actual usages of source maps, but we need to check in TG4 if it needs to be tweaked. I'll open an issue to better discuss it.

Examples of ambiguous comments:

let a = `
//# sourceMappingURL=foo.map
// `;
let a = "\
//# sourceMappingURL=foo.map"
let a = '\
//# sourceMappingURL=foo.map'
//# sourceMappingURL=bar.map
/*
//# sourceMappingURL=foo.map
// */

This PR needs to be merged today, because we need to start the 60 days period and this is a requirement for it. The pull request does not contain any normative changes, and is entirely an editorial decision.

@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Oct 10, 2024

This is how that section looks like now:

image

Copy link
Member

@littledan littledan left a comment

Choose a reason for hiding this comment

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

LGTM but note that, because this doesn’t have normative content, it doesn’t affect the 60 day opt out period.

@nicolo-ribaudo nicolo-ribaudo changed the title Add security notice warning for ambiguous source maps Add security warning for ambiguous source maps Oct 10, 2024
@nicolo-ribaudo nicolo-ribaudo merged commit 7c40f8c into tc39:main Oct 10, 2024
2 checks passed
@nicolo-ribaudo nicolo-ribaudo deleted the security-notice-ambiguous-source-maps branch October 10, 2024 09:28
github-actions bot added a commit that referenced this pull request Oct 10, 2024
SHA: 7c40f8c
Reason: push, by nicolo-ribaudo

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Oct 10, 2024
SHA: 0142f27
Reason: push, by nicolo-ribaudo

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.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.

3 participants