Skip to content

Commit

Permalink
Allow the any type to bind to security sensitive properties (#276)
Browse files Browse the repository at this point in the history
* Remove a `.only` call in the 'security-system' rule tests.

* Remove incorrect `</script>` closing tags.

* Allow the `any` type to bind to security sensitive properties

Note that this is not security sensitive code. The actual security boundary is at runtime. This is just the type checker assisting the developer, predicting what the runtime
enforcement will allow/reject.

Allowing `any` here aligns with TypeScript's general type checking principles,
where you're allowed to do just about anything you want with an `any` typed value.

Co-authored-by: Peter Burns <rictic@google.com>

* Fix `const` redeclarations in tests.

Co-authored-by: Peter Burns <rictic@google.com>
  • Loading branch information
bicknellr and rictic authored Sep 15, 2022
1 parent 3218622 commit 315252f
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 29 deletions.
4 changes: 2 additions & 2 deletions packages/lit-analyzer/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ function checkClosureSecurityAssignability(typeB: SimpleType, htmlAttr: HtmlNode
if (overriddenTypes === undefined) {
return undefined;
}
// `any` is allowed to bind to anything.
if (typeB.kind === "ANY") {
return undefined;
}
// Directives are responsible for their own security.
if (isLitDirective(typeB)) {
return undefined;
Expand Down
94 changes: 70 additions & 24 deletions packages/lit-analyzer/src/test/rules/security-system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ const preface = `
const trustedResourceUrl = new TrustedResourceUrl();
const safeUrl = new SafeUrl();
const safeStyle = new SafeStyle();
const anyValue: any = {};
`;

tsTest("May bind string to script src with default config", t => {
Expand Down Expand Up @@ -39,7 +41,7 @@ tsTest(testName, t => {
});

testName = "May not pass a TrustedResourceUrl to script .src with default config";
tsTest.only(testName, t => {
tsTest(testName, t => {
const { diagnostics } = getDiagnostics(preface + "html`<script .src=${trustedResourceUrl}></script>`");
hasDiagnostic(t, diagnostics, "no-incompatible-type-binding");
});
Expand Down Expand Up @@ -68,36 +70,68 @@ tsTest(testName, t => {
hasDiagnostic(t, diagnostics, "no-incompatible-type-binding");
});

testName = "May pass either a SafeUrl or a TrustedResourceUrl or a string to img src with ClosureSafeTypes config";
testName = "May pass `any` to script src with ClosureSafeTypes config";
tsTest(testName, t => {
const { diagnostics } = getDiagnostics(preface + "html`<img src=${safeUrl}></script>`", { securitySystem: "ClosureSafeTypes" });
const { diagnostics } = getDiagnostics(preface + "html`<script src=${anyValue}></script>`", { securitySystem: "ClosureSafeTypes" });
hasNoDiagnostics(t, diagnostics);

const { diagnostics: moreDiagnostics } = getDiagnostics(preface + "html`<img src=${trustedResourceUrl}></script>`", {
securitySystem: "ClosureSafeTypes"
});
hasNoDiagnostics(t, moreDiagnostics);

const { diagnostics: evenMoreDiagnostics } = getDiagnostics(preface + "html`<img src=${'/img.webp'}></script>`", {
securitySystem: "ClosureSafeTypes"
});
hasNoDiagnostics(t, evenMoreDiagnostics);
});

testName = "May pass either a SafeUrl or a TrustedResourceUrl or a string to img .src with ClosureSafeTypes config";
testName = "May pass `any` to script .src with ClosureSafeTypes config";
tsTest(testName, t => {
const { diagnostics } = getDiagnostics(preface + "html`<img .src=${safeUrl}></script>`", { securitySystem: "ClosureSafeTypes" });
const { diagnostics } = getDiagnostics(preface + "html`<script .src=${anyValue}></script>`", { securitySystem: "ClosureSafeTypes" });
hasNoDiagnostics(t, diagnostics);
});

const { diagnostics: moreDiagnostics } = getDiagnostics(preface + "html`<img .src=${trustedResourceUrl}></script>`", {
securitySystem: "ClosureSafeTypes"
});
hasNoDiagnostics(t, moreDiagnostics);

const { diagnostics: evenMoreDiagnostics } = getDiagnostics(preface + "html`<img .src=${'/img.webp'}></script>`", {
securitySystem: "ClosureSafeTypes"
});
hasNoDiagnostics(t, evenMoreDiagnostics);
testName = "May pass either a SafeUrl, a TrustedResourceUrl, a string, or `any` to img src with ClosureSafeTypes config";
tsTest(testName, t => {
hasNoDiagnostics(t, getDiagnostics(preface + "html`<img src=${safeUrl}>`", { securitySystem: "ClosureSafeTypes" }).diagnostics);

hasNoDiagnostics(
t,
getDiagnostics(preface + "html`<img src=${trustedResourceUrl}>`", {
securitySystem: "ClosureSafeTypes"
}).diagnostics
);

hasNoDiagnostics(
t,
getDiagnostics(preface + "html`<img src=${'/img.webp'}>`", {
securitySystem: "ClosureSafeTypes"
}).diagnostics
);

hasNoDiagnostics(
t,
getDiagnostics(preface + "html`<img src=${anyValue}>`", {
securitySystem: "ClosureSafeTypes"
}).diagnostics
);
});

testName = "May pass either a SafeUrl, a TrustedResourceUrl, a string, or `any` to img .src with ClosureSafeTypes config";
tsTest(testName, t => {
hasNoDiagnostics(t, getDiagnostics(preface + "html`<img .src=${safeUrl}>`", { securitySystem: "ClosureSafeTypes" }).diagnostics);

hasNoDiagnostics(
t,
getDiagnostics(preface + "html`<img .src=${trustedResourceUrl}>`", {
securitySystem: "ClosureSafeTypes"
}).diagnostics
);

hasNoDiagnostics(
t,
getDiagnostics(preface + "html`<img .src=${'/img.webp'}>`", {
securitySystem: "ClosureSafeTypes"
}).diagnostics
);

hasNoDiagnostics(
t,
getDiagnostics(preface + "html`<img .src=${anyValue}>`", {
securitySystem: "ClosureSafeTypes"
}).diagnostics
);
});

testName = "May pass a string to style with ClosureSafeTypes config";
Expand All @@ -123,3 +157,15 @@ tsTest(testName, t => {
const { diagnostics } = getDiagnostics(preface + "html`<div .style=${safeStyle}></div>`", { securitySystem: "ClosureSafeTypes" });
hasNoDiagnostics(t, diagnostics);
});

testName = "May pass a `any` to style with ClosureSafeTypes config";
tsTest(testName, t => {
const { diagnostics } = getDiagnostics(preface + "html`<div style=${anyValue}></div>`", { securitySystem: "ClosureSafeTypes" });
hasNoDiagnostics(t, diagnostics);
});

testName = "May pass a `any` to .style with ClosureSafeTypes config";
tsTest(testName, t => {
const { diagnostics } = getDiagnostics(preface + "html`<div .style=${anyValue}></div>`", { securitySystem: "ClosureSafeTypes" });
hasNoDiagnostics(t, diagnostics);
});
6 changes: 3 additions & 3 deletions packages/vscode-lit-plugin/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 315252f

Please sign in to comment.