Skip to content

Commit

Permalink
Failing build if translation files contain invalid values (#1367)
Browse files Browse the repository at this point in the history
## Context

Latest PR #1354 was breaking docker build and took the wrong approach for sanitizing translation files. More in the comments. 

## Proposed solution

1. Docker build was fixed by copying translation files sooner (just for validating them).
2. Build now only fails if some translations keys have values with invalid tags. Previously it was fixing them.

## Related issues

#1350
#1354
#1361

## Has this been tested?

<!-- Put an `x` in the box that applies: -->

- [x] 👍 yes, I added tests to the test suite
- [ ] 💭 no, because this PR is a draft and still needs work
- [ ] 🙅 no, because this is not relevant here
- [ ] 🙋 no, because I need help <!-- Detail how we can help you -->
  • Loading branch information
berhalak authored Jan 7, 2025
1 parent a1fff81 commit dd6c093
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 21 deletions.
5 changes: 5 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,12 @@ COPY test/chai-as-promised.js /grist/test/chai-as-promised.js
COPY app /grist/app
COPY stubs /grist/stubs
COPY buildtools /grist/buildtools
# Copy locales files early. During build process they are validated.
COPY static/locales /grist/static/locales
RUN yarn run build:prod
# We don't need them anymore, they will by copied to the final image.
RUN rm -rf /grist/static/locales


# Prepare material for optional pyodide sandbox
COPY sandbox/pyodide /grist/sandbox/pyodide
Expand Down
91 changes: 70 additions & 21 deletions buildtools/sanitize_translations.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ function handleSanitizeAttribute(node) {
node.setAttribute('target', '_blank');
}

// If the the first arg is test, do the self test.
if (process.argv[2] === "test") {
selfTest();
process.exit(0);
}

const directoryPath = readDirectoryPath();

const fileStream = fs.readdirSync(directoryPath)
Expand All @@ -25,38 +31,37 @@ const fileStream = fs.readdirSync(directoryPath)
// Read the contents and put it into an array [path, json]
.map((file) => [file, JSON.parse(fs.readFileSync(file, "utf8"))]);

console.debug(`Found ${fileStream.length} files to sanitize`);

const sanitized = fileStream.map(([file, json]) => {
return [file, json, sanitizedJson(json)];
return [file, json, invalidValues(json)];
});

const onlyDifferent = sanitized.filter(([file, json, sanitizedJson]) => {
return JSON.stringify(json) !== JSON.stringify(sanitizedJson);
});

console.debug(`Found ${onlyDifferent.length} files that need sanitizing`);

// Write the sanitized json back to the files
onlyDifferent.forEach(([file, json, sanitizedJson]) => {
console.info(`Sanitizing ${file}`);
fs.writeFileSync(file, JSON.stringify(sanitizedJson, null, 4) + "\n");
const onlyDifferent = sanitized.filter(([file, json, invalidKeys]) => {
return invalidKeys.length > 0;
});

console.info("Sanitization complete");
if (onlyDifferent.length > 0) {
console.error("The following files contain invalid values:");
onlyDifferent.forEach(([file, json, invalidKeys]) => {
console.error(`File: ${file}`);
console.error(`Values: ${invalidKeys.join(", ")}`);
});
process.exit(1);
}

function sanitizedJson(json) {
function invalidValues(json) {
// This is recursive function as some keys can be objects themselves, but all values are either
// strings or objects.
return Object.keys(json).reduce((acc, key) => {
const value = json[key];
return Object.values(json).reduce((acc, value) => {
if (typeof value === "string") {
acc[key] = purify(value);
const sanitized = purify(value);
if (value !== sanitized) {
acc.push(value);
}
} else if (typeof value === "object") {
acc[key] = sanitizedJson(value);
acc.push(...invalidValues(value));
}
return acc;
}, {});
}, []);
}


Expand All @@ -72,5 +77,49 @@ function readDirectoryPath() {

function purify(inputString) {
// This removes any html tags from the string
return DOMPurify.sanitize(inputString);
return DOMPurify.sanitize(inputString, { ALLOWED_TAGS: [] });
}

function selfTest() {
const okDir = createTmpDir();
const okFile = path.join(okDir, "ok.json");
fs.writeFileSync(okFile, JSON.stringify({ "key": "value" }));

const badDir = createTmpDir();
const badFile = path.join(badDir, "bad.json");
fs.writeFileSync(badFile, JSON.stringify({ "key": "<script>alert('xss')</script>" }));

// Run this script in the okDir, it should pass (return value 0)
const okResult = exitCode(`node ${__filename} ${okDir}`);
if (okResult !== 0) {
console.error("Self test failed, expected 0 for okDir");
process.exit(1);
}

// Run this script in the badDir, it should fail (return value 1)
const badResult = exitCode(`node ${__filename} ${badDir}`);
if (badResult !== 1) {
console.error("Self test failed, expected 1 for badDir");
process.exit(1);
}

console.log("Self test passed");

function createTmpDir() {
const os = require('os');
const tmpDir = os.tmpdir();
const prefix = path.join(tmpDir, 'tmp-folder-');
const tmpFolderPath = fs.mkdtempSync(prefix);
return tmpFolderPath;
}

function exitCode(args) {
const {execSync} = require('child_process');
try {
execSync(args); // will throw if exit code is not 0
return 0;
} catch (e) {
return 1;
}
}
}

0 comments on commit dd6c093

Please sign in to comment.