generated from SAP/repository-template
-
Notifications
You must be signed in to change notification settings - Fork 7
feat: Check script tags for inline JS #48
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
Merged
Merged
Changes from all commits
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
5d59158
feat: Add html linter
d3xter666 2b19bf6
feat: Add html parser
d3xter666 61c869c
feat: Implement html script linter
d3xter666 5a9700a
refactor: Rename files
d3xter666 ccc974e
fix: Eslint issues
d3xter666 fec7d7f
fix: Add new files to the tests
d3xter666 ecb1350
refactor: Reword messageDetails
d3xter666 ae2f229
feat: Add reporting to the script linter
d3xter666 7c5eca9
feat: Add test cases
d3xter666 7a3500d
fix: Eslint issues
d3xter666 5f2c6ba
feat: Add messageDetails
d3xter666 cb755df
fix: --file-paths with non-js resources
d3xter666 15ae266
fix: Eslint
d3xter666 2604e63
fix: Tests
d3xter666 6e58189
feat: Add test cases
d3xter666 dcfa43b
fix: Eslint
d3xter666 efdbd26
fix: Tests for Windows
d3xter666 9242980
fix: Tests for Windows
d3xter666 4dd42cf
refactor: Remove redundant checks
d3xter666 69ef393
refactor: Rewrite --file-paths filtering fallback to js
d3xter666 f3fce8d
refactor: Treat html files as JSX
d3xter666 dc916c0
feat: Parse HTML files as JSX
d3xter666 95ad30c
fix: Fix tests
d3xter666 8042a49
refactor: Address GH comments
d3xter666 7ece8f9
fix: Use jsx: preserve option for scanned documents
d3xter666 615f5c1
refactor: Optimize resource filtering
d3xter666 810c8d0
refactor: Enforce utf8 encoding for html fixtures
d3xter666 8992d55
refactor: Adjust report details for CSP
d3xter666 4914973
refactor: Avoid parsing of HTML content as JSX
d3xter666 ed9d9e7
refactor: Move js script checks in the parser
d3xter666 6f6bb45
fix: Rename html files to JSX
d3xter666 3fd956f
fix: Add new lines at the end of html tests
d3xter666 0cd6efb
fix: Ignore case when checking script's type
d3xter666 93495c6
feat: Add tests for application/javascript
d3xter666 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
import type {ReadStream} from "node:fs"; | ||
import {Detail, SaxEventType, SAXParser, Tag as SaxTag} from "sax-wasm"; | ||
import {finished} from "node:stream/promises"; | ||
import fs from "node:fs/promises"; | ||
import {createRequire} from "node:module"; | ||
const require = createRequire(import.meta.url); | ||
|
||
let saxWasmBuffer: Buffer; | ||
async function initSaxWasm() { | ||
if (!saxWasmBuffer) { | ||
const saxPath = require.resolve("sax-wasm/lib/sax-wasm.wasm"); | ||
saxWasmBuffer = await fs.readFile(saxPath); | ||
} | ||
|
||
return saxWasmBuffer; | ||
} | ||
|
||
async function parseHtml(contentStream: ReadStream, parseHandler: (type: SaxEventType, tag: Detail) => void) { | ||
const options = {highWaterMark: 32 * 1024}; // 32k chunks | ||
const saxWasmBuffer = await initSaxWasm(); | ||
const saxParser = new SAXParser(SaxEventType.CloseTag, options); | ||
|
||
saxParser.eventHandler = parseHandler; | ||
|
||
// Instantiate and prepare the wasm for parsing | ||
if (!await saxParser.prepareWasm(saxWasmBuffer)) { | ||
throw new Error("Unknown error during WASM Initialization"); | ||
} | ||
|
||
// stream from a file in the current directory | ||
contentStream.on("data", (chunk: Uint8Array) => { | ||
try { | ||
saxParser.write(chunk); | ||
} catch (err) { | ||
if (err instanceof Error) { | ||
// In case of an error, destroy the content stream to make the | ||
// error bubble up to our callers | ||
contentStream.destroy(err); | ||
} else { | ||
throw err; | ||
} | ||
} | ||
}); | ||
await finished(contentStream); | ||
saxParser.end(); | ||
} | ||
|
||
export async function extractJSScriptTags(contentStream: ReadStream) { | ||
const scriptTags: SaxTag[] = []; | ||
|
||
await parseHtml(contentStream, (event, tag) => { | ||
if (tag instanceof SaxTag && | ||
event === SaxEventType.CloseTag && | ||
tag.value === "script") { | ||
const isJSScriptTag = tag.attributes.every((attr) => { | ||
// The "type" attribute of the script tag should be | ||
// 1. not set (default), | ||
// 2. an empty string, | ||
// 3. or a JavaScript MIME type (text/javascript) | ||
// https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script/type#attribute_is_not_set_default_an_empty_string_or_a_javascript_mime_type | ||
return attr.name.value !== "type" || | ||
(attr.name.value === "type" && | ||
["", | ||
"text/javascript", | ||
"application/javascript", /* legacy */ | ||
].includes(attr.value.value.toLowerCase())); | ||
}); | ||
|
||
if (isJSScriptTag) { | ||
scriptTags.push(tag); | ||
} | ||
} | ||
}); | ||
|
||
return scriptTags; | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
import type {BaseReporter, ReporterMessage, ReporterCoverageInfo} from "../../detectors/BaseReporter.js"; | ||
import type {LintMessage} from "../../detectors/AbstractDetector.js"; | ||
import {Tag as SaxTag} from "sax-wasm"; | ||
import {LintMessageSeverity, CoverageInfo} from "../../detectors/AbstractDetector.js"; | ||
import {resolveLinks} from "../../formatter/lib/resolveLinks.js"; | ||
|
||
export default class HtmlReporter implements BaseReporter { | ||
#filePath: string; | ||
#messages: LintMessage[] = []; | ||
#coverageInfo: CoverageInfo[] = []; | ||
|
||
constructor(filePath: string) { | ||
this.#filePath = filePath; | ||
} | ||
|
||
addMessage({node, message, messageDetails, severity, ruleId, fatal = undefined}: ReporterMessage) { | ||
if (fatal && severity !== LintMessageSeverity.Error) { | ||
throw new Error(`Reports flagged as "fatal" must be of severity "Error"`); | ||
} | ||
|
||
let line = 0, column = 0; | ||
if (node instanceof SaxTag) { | ||
({line, character: column} = node.openStart); | ||
} | ||
|
||
const msg: LintMessage = { | ||
ruleId, | ||
severity, | ||
fatal, | ||
line: line + 1, | ||
column: column + 1, | ||
message, | ||
}; | ||
|
||
if (messageDetails) { | ||
msg.messageDetails = resolveLinks(messageDetails); | ||
} | ||
|
||
this.#messages.push(msg); | ||
} | ||
|
||
addCoverageInfo({node, message, category}: ReporterCoverageInfo) { | ||
let line = 0, column = 0, endLine = 0, endColumn = 0; | ||
if (node instanceof SaxTag) { | ||
({line, character: column} = node.openStart); | ||
({line: endLine, character: endColumn} = node.closeEnd); | ||
} | ||
|
||
this.#coverageInfo.push({ | ||
category, | ||
// One-based to be aligned with most IDEs | ||
line: line + 1, | ||
column: column + 1, | ||
endLine: endLine + 1, | ||
endColumn: endColumn + 1, | ||
message, | ||
}); | ||
} | ||
|
||
getReport() { | ||
let errorCount = 0; | ||
let warningCount = 0; | ||
let fatalErrorCount = 0; | ||
for (const {severity, fatal} of this.#messages) { | ||
if (severity === LintMessageSeverity.Error) { | ||
errorCount++; | ||
if (fatal) { | ||
fatalErrorCount++; | ||
} | ||
} else { | ||
warningCount++; | ||
} | ||
} | ||
|
||
return { | ||
filePath: this.#filePath, | ||
messages: this.#messages, | ||
coverageInfo: this.#coverageInfo, | ||
errorCount, | ||
warningCount, | ||
fatalErrorCount, | ||
}; | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
import {taskStart} from "../../detectors/util/perf.js"; | ||
import {extractJSScriptTags} from "../../detectors/transpilers/html/parser.js"; | ||
import {LintMessageSeverity} from "../../detectors/AbstractDetector.js"; | ||
import HtmlReporter from "./HtmlReporter.js"; | ||
|
||
import type {TranspileResult} from "../../detectors/transpilers/AbstractTranspiler.js"; | ||
import type {ReadStream} from "node:fs"; | ||
|
||
export async function lintHtml(resourceName: string, contentStream: ReadStream): Promise<TranspileResult> { | ||
const taskLintEnd = taskStart("Linting HTML", resourceName); | ||
const report = new HtmlReporter(resourceName); | ||
const jsScriptTags = await extractJSScriptTags(contentStream); | ||
|
||
jsScriptTags.forEach((tag) => { | ||
const scriptContent = tag.textNodes?.map((tNode) => tNode.value).join("").trim(); | ||
|
||
if (scriptContent) { | ||
report.addMessage({ | ||
node: tag, | ||
severity: LintMessageSeverity.Warning, | ||
ruleId: "ui5-linter-csp-unsafe-inline-script", | ||
message: `Use of unsafe inline script`, | ||
messageDetails: "{@link topic:fe1a6dba940e479fb7c3bc753f92b28c Content Security Policy}", | ||
}); | ||
} | ||
}); | ||
|
||
taskLintEnd(); | ||
|
||
const {messages} = report.getReport(); | ||
return {messages, source: "", map: ""}; | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
<html> | ||
|
||
<head> | ||
<meta charset="UTF-8"> | ||
<title>No inline JS</title> | ||
matz3 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
</head> | ||
|
||
<body> | ||
<script type="text/javascript"> | ||
sap.ui.controller("my.own.controller", { | ||
doSomething: function () { | ||
alert("Hello World!"); | ||
} | ||
}); | ||
</script> | ||
|
||
<script type="application/javascript"> | ||
sap.ui.controller("my.own.controller", { | ||
doSomething: function () { | ||
alert("Hello World!"); | ||
} | ||
}); | ||
</script> | ||
|
||
<script type=""> | ||
(function() { | ||
alert("test") | ||
})(); | ||
</script> | ||
|
||
<script> | ||
sap.ui.controller("myController", { | ||
onInit: function () { | ||
var model = new sap.ui.model.json.JSONModel(); | ||
model.setData({ | ||
buttonText: "Click Me!" | ||
}); | ||
this.getView().setModel(model); | ||
}, | ||
doSomething: function () { | ||
alert("Hello World!"); | ||
} | ||
}); | ||
sap.ui.xmlview({ viewContent: jQuery('#myXml').html() }).placeAt("content"); | ||
</script> | ||
</body> | ||
|
||
</html> |
32 changes: 32 additions & 0 deletions
32
test/fixtures/linter/rules/CSPCompliance/NoInlineJS_negative.html
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
<html> | ||
|
||
<head> | ||
<meta charset="UTF-8"> | ||
<title>No inline JS</title> | ||
</head> | ||
|
||
<body> | ||
<script id="myXml" type="text/xmldata"> | ||
<mvc:View xmlns:mvc="sap.ui.core.mvc" xmlns="sap.m" controllerName="myController" displayBlock="true"> | ||
<App> | ||
<Page title="Hello"> | ||
<Button text="{/buttonText}" press= "doSomething"/> | ||
</Page> | ||
</App> | ||
</mvc:View> | ||
</script> | ||
|
||
<script type="module"> | ||
import { log } from "utils"; | ||
|
||
log("Exporting dog names."); | ||
|
||
export const names = ["Kayla", "Bentley", "Gilligan"]; | ||
</script> | ||
|
||
<script type="" src="./path/to/js.js"></script> | ||
|
||
<script src="./another/path/to/js.js"></script> | ||
</body> | ||
|
||
</html> | ||
d3xter666 marked this conversation as resolved.
Show resolved
Hide resolved
|
Binary file modified
BIN
-61 Bytes
(99%)
test/lib/detectors/transpilers/xml/snapshots/transpiler.ts.snap
Binary file not shown.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import path from "node:path"; | ||
import {fileURLToPath} from "node:url"; | ||
import {createTestsForFixtures} from "../_linterHelper.js"; | ||
|
||
const filePath = fileURLToPath(import.meta.url); | ||
const __dirname = path.dirname(filePath); | ||
const fileName = path.basename(filePath, ".ts"); | ||
const fixturesPath = path.join(__dirname, "..", "..", "..", "fixtures", "linter", "rules", fileName); | ||
|
||
createTestsForFixtures(fixturesPath); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.