Comments (22)
@tosmolka We were indeed talking about union types. The new behavior is that, if tsec found the definition of Trusted Types is ambient, union types like string|TrustedScriptURL
are not longer acceptable at the sinks, since that may cause problems. For consistency, the following case will be flagged as well:
declare const trustedScriptUrl: string | TrustedScriptURL;
script.src = trustedScriptUrl as string;
On unit tests: we do have them internally, but for some reason we don't export the test code to github.
from tsec.
Thanks for reporting this. Due to internal issues, tsec can't accept external contributions (as stated in our CONTRIBUTING.md). I will patch this internally and release a new version.
from tsec.
Could you provide some details about your setup? Did you swap out the original lib.dom.d.ts and use an updated one? Or is the updated lib.dom.d.ts released as an npm package?
from tsec.
@uraj , I ended up patching lib.dom.d.ts in my local setup, I pushed patch to 27256dc as a reference.
from tsec.
@uraj , do you have enough info to investigate this further? Any updates? Thanks.
from tsec.
@tosmolka Sorry we're currently working on adding ESLint support. It should be done by the end of this week. After that I will add support for TT in DOM lib.
Another thing that might be useful to know: in which file do you define the Trusted Types? Did you put the definition in lib.dom.d.ts or you have a separate file for Type declaration?
from tsec.
@tosmolka I'm about to release a patch. Before that, want to confirm with you on how you amended lib.dom.d.ts. Did you just append something like below to the file?
declare class TrustedHTML {
//...
}
from tsec.
@uraj , I used this: 27256dc and applied patch using patch-package.
from tsec.
Should be fixed by 1ceed96. Will publish a new version soon.
from tsec.
Please try 0.2.4.
from tsec.
@uraj , I did a quick test and I think this needs little bit of more work. I tried with HTMLScriptElement.src sink and tsec still flags when I do assignments with string | TrustedScriptURL
or TrustedScriptURL
types.
I think I faced similar issues when I proposed the PR and I fixed that by adding isTrustedType
and isTrustedTypeUnionWithString
methods to is_trusted_type.ts
.
from tsec.
@tosmolka OK I got what you meant. I will discuss with the team and see whether we should support this form of assignments. With 0.2.4 you can now at least write
declare const trustedScriptUrl: string | TrustedScriptURL;
script.src = trustedScriptUrl as string;
where TrustedScriptURL
is defined in the patched lib.dom.d.ts.
from tsec.
@uraj , do you have any updates regarding support for this form of assignments? Thanks a lot.
declare const trustedScriptUrl: string | TrustedScriptURL;
script.src = trustedScriptUrl;
from tsec.
@tosmolka With lib.dom.d.ts patched, there is no longer the need to use string | TrustedScriptURL
. I think we can support the following
declare const trustedScriptUrl: TrustedScriptURL;
script.src = trustedScriptUrl;
Is that OK to you?
from tsec.
I think we will continue using string | TrustedScriptURL
in certain cases (e.g. Trusted Types are not enabled/supported).
Now, if tsec keeps flagging this as an issue because string
MIGHT flow into this sink - that's understandable and we would have to deal with it on our side.
But then I'd like tsec to be consistent and start flagging also this case:
declare const trustedScriptUrl: string | TrustedScriptURL;
script.src = trustedScriptUrl as string;
WDYT?
from tsec.
We can flag that case, if we detect that TrustedScriptURL
is defined as an ambient type, i.e., an indicator that lib.dom.d.ts is patched. We should really check if script.src
actually accepts an TrustedScriptURL
but that seems a much more complicated change.
from tsec.
@tosmolka I've submitted the patch, but for some reason I can't publish a new version right now. I will try over the weekends, but you can try out the patch before that if you want
from tsec.
0.2.6 is now published. Let me know if the new version doesn't work for your setup.
from tsec.
@uraj , I quickly checked the change.
It seems to be working fine for intersection types - string & TrustedScriptURL
. But what we need are union types - string | TrustedScriptURL
- this is still being flagged by tsec as an issue.
from tsec.
@tosmolka this is intended. I thought your project is OK with this, per our previous discussions.
from tsec.
@uraj , I thought we were discussion union types - string | TrustedScriptURL
- this is what we are using in our projects, this is what I . I am confused now, are you saying we should change our setup or are you saying you are going to fix this in tsec? Can you add unit tests for these cases into tsec so that the expected behavior is clearer? Thanks.
from tsec.
@uraj , OK, I misunderstood what is the intended behavior after your fix. Would be good to update docs accordingly at some point. Thanks.
from tsec.
Related Issues (13)
- Guidance on usage with trusted types HOT 2
- Suppress by error code HOT 1
- Publish tsec on npmjs.com HOT 4
- Casting tricks don't work HOT 4
- No support for project references? HOT 1
- Don't run tsc as a post install step HOT 4
- Exemptions not working? HOT 2
- Invalid warning on Worker assignment HOT 3
- Bazel Integration: support runfile resolution
- Why assigning Trusted Types to dangerous sinks still causes compilation error? HOT 5
- Add option for ignoring TS errors HOT 6
- [ban-worker-calls] Is there any best practice for such error ? HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from tsec.