chore: DAH-3291 translation key parity and value checks in CI - #3026
chore: DAH-3291 translation key parity and value checks in CI#3026fwextensions wants to merge 4 commits into
Conversation
loadTranslations() in app/javascript/util/languageUtil.tsx loads en.json and then overlays the target locale on top of it. A key missing from es/tl/zh therefore renders the English string silently -- no raw key, no empty box, no console warning. Nothing in the app or in CI surfaces it. The existing translations-check workflow only runs jsonlint and verifies each file is `jq -S` sorted; it never compares key sets between locales. Adds scripts/check-translation-parity.js, which flattens each locale and diffs its key set against en.json, failing on keys that are missing or present-but- empty. Keys not in en.json are reported as dead weight without failing. Seeds scripts/known-translation-gaps.json with the 26 keys per locale that are already missing on main so the check can be turned on without blocking unrelated PRs. That allowlist is a ratchet, not a resolution -- every entry is an English string being shown to a non-English speaker today. Also ignores scripts/** in eslint.config.mjs; the flat config applies parserOptions.project globally, so a plain Node script outside tsconfig.lint.json fails `yarn lint` with a parsing error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- Don't flag an empty locale value when en.json is also empty for that key. Not hypothetical: listings.occupancyDescriptionAllSroPlural is an empty string in en.json, so a translator mirroring it would have failed the build for no benefit. - Report allowlist entries whose key no longer exists in en.json. Previously only entries that had been translated were flagged as stale, so exemptions for deleted keys could accumulate indefinitely. - Add an explicit `permissions: contents: read` block (CodeQL). - Validate locale names before building a path. Locale names only come from readdirSync on a fixed directory or from BASE_LOCALE, so the Wiz path traversal findings are false positives, but the guard is cheap and keeps that true if a caller is added later. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Devs have been verifying translations by hand with the Translation-Checker tool (https://github.com/chadbrokaw/Translation-Checker), a browser page you paste XLIFF into. Nothing in CI ran its rules, so the only thing standing between a mangled string and production was someone remembering to paste each file in. Ports its five checks into check-translation-parity.js rather than adding a second script, so both key-level and value-level problems surface from one CI step and share the known-translation-gaps.json suppression mechanism: - encoding errors (a literal &/nbsp; means double-encoding) - variables missing their % prefix - variables differing from en.json - plural separators that aren't exactly four bars - pluralized strings with the wrong number of forms Two rules are adapted rather than copied, because porting them literally produced 31 false positives on the current files: - Variable comparison uses the set of names, not the occurrence count. A pluralized translation repeats each variable once per form, so counting occurrences flags every plural. - Plural form count is checked against what the locale needs (1 for zh, 2 otherwise), not against en.json. English often has one form where Spanish legitimately needs two -- listings.habitat.incomeRange is exactly that, and is correct. Verified by injecting one defect per rule into es.json: each fired on its own key and nothing else, exit 1. Current files pass for es/tl/zh. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try the Wiz Code extension for VS Code, JetBrains, or Visual Studio. |
| console.error(`Refusing to read unexpected locale name: ${locale}`) | ||
| process.exit(1) | ||
| } | ||
| const file = path.join(TRANSLATIONS_DIR, `${locale}.json`) |
There was a problem hiding this comment.
Path Traversal Vulnerability in Node.js (CWE-22)
More Details
This rule detects potential path traversal vulnerabilities in Node.js applications. Path traversal vulnerabilities occur when user input is passed unsanitized to file system operations, allowing attackers to access arbitrary files and directories on the server.
This issue presents a significant security risk as it can lead to unauthorized access to sensitive data, code execution, and complete system compromise. If exploited, an attacker could read confidential files, modify system files, or even execute malicious code on the server.
| Attribute | Value |
|---|---|
| Impact | |
| Likelihood |
Remediation
Path traversal vulnerabilities allow an attacker to access arbitrary files on the system, potentially exposing sensitive data or enabling further attacks. This vulnerability arises when user input is passed unsanitized to path manipulation functions like path.join or path.resolve, allowing an attacker to traverse the file system using patterns like ../.
To fix this issue, user input should be sanitized or validated before being passed to path manipulation functions. This can be done by using allowlists, removing or encoding special characters, or validating the resulting path against a set of allowed directories.
Code examples
// VULNERABLE CODE - User input is passed directly to path.join
const userInput = req.query.file;
const filePath = path.join(__dirname, userInput);
fs.readFile(filePath, (err, data) => { ... });// SECURE CODE - User input is sanitized before path manipulation
const userInput = req.query.file;
const sanitizedInput = sanitizeInput(userInput); // Implement sanitization logic
const filePath = path.join(__dirname, sanitizedInput);
fs.readFile(filePath, (err, data) => { ... });Additional recommendations
- Follow the principle of least privilege and restrict file access as much as possible.
- Use allowlists instead of denylist-based input validation when possible.
- Consider using libraries like
path-sanitizerorsanitize-filenamefor input sanitization. - Adhere to the OWASP Input Validation Cheat Sheet and other relevant security standards.
- As an alternative approach, consider using a virtual file system or sandboxing techniques to isolate file operations from the main system.
Rule ID: WS-I013-JAVASCRIPT-00098
To ignore this finding as an exception, reply to this conversation with #wiz_ignore reason
If you'd like to ignore this finding in all future scans, add an exception in the .wiz file (learn more) or create an Ignore Rule (learn more).
To get more details on how to remediate this issue using AI, reply to this conversation with #wiz remediate
| } | ||
| const file = path.join(TRANSLATIONS_DIR, `${locale}.json`) | ||
| try { | ||
| return flatten(JSON.parse(fs.readFileSync(file, "utf8"))) |
There was a problem hiding this comment.
Improper Limitation of Pathname to Restricted Directory (Path Traversal) (CWE-22)
More Details
Path traversal vulnerabilities occur when user-supplied input is used to construct file paths without proper validation or sanitization. This allows an attacker to access files and directories outside the intended scope, potentially exposing sensitive data or enabling unauthorized system access.
The risk stems from the application's failure to properly restrict file operations to a limited directory. By manipulating the file path with special characters like "../", an attacker can traverse the file system hierarchy and access arbitrary files or directories.
Successful exploitation of a path traversal vulnerability can lead to data breaches, unauthorized access to system resources, and potentially complete system compromise. It is crucial to validate and sanitize all user input used in file operations to prevent such attacks.
| Attribute | Value |
|---|---|
| Impact | |
| Likelihood |
Remediation
Path traversal vulnerabilities occur when user-supplied input is used to construct file paths without proper validation or sanitization. This allows an attacker to access files and directories outside the intended scope, potentially exposing sensitive data or enabling unauthorized system access. Successful exploitation of a path traversal vulnerability can lead to data breaches, unauthorized access to system resources, and potentially complete system compromise.
To fix this issue, you should validate and sanitize all user input used in file operations to prevent path traversal attacks. This can be achieved by using a secure path resolution library or by implementing strict input validation and sanitization routines. Avoid concatenating user input directly into file paths, and instead, use platform-specific path normalization functions to resolve paths safely.
Code examples
// VULNERABLE CODE - User input is concatenated directly into the file path
const fs = require('fs');
const userInput = "../../../sensitive.txt";
fs.readFile(`/app/files/${userInput}`, (err, data) => {
// ...
});// SECURE CODE - User input is sanitized, and path is resolved securely
const fs = require('fs');
const path = require('path');
const userInput = "../../../sensitive.txt";
const sanitizedPath = path.resolve('/app/files', path.normalize(userInput));
fs.readFile(sanitizedPath, (err, data) => {
// ...
});Additional recommendations
- Use the built-in
pathmodule in Node.js to safely construct file paths. - Implement strict input validation and sanitization routines for all user input used in file operations.
- Follow the principle of least privilege and restrict file operations to a limited directory scope.
- Adhere to security best practices outlined in the OWASP Top 10 and CWE guidelines for handling user input and file operations.
- Consider using a secure path resolution library like
path-sanitizerorsecure-path-resolvefor additional protection against path traversal attacks.
Rule ID: WS-I011-TYPESCRIPT-00001
To ignore this finding as an exception, reply to this conversation with #wiz_ignore reason
If you'd like to ignore this finding in all future scans, add an exception in the .wiz file (learn more) or create an Ignore Rule (learn more).
To get more details on how to remediate this issue using AI, reply to this conversation with #wiz remediate
There was a problem hiding this comment.
Pull request overview
Adds a CI-enforced translation parity/content validation step so missing locale keys (which currently silently fall back to English) and common translation-string defects are caught automatically, with an allowlist to avoid blocking existing known gaps.
Changes:
- Introduces
scripts/check-translation-parity.jsto validate locale key parity againsten.jsonand run several value-level checks (variables, plural formatting, encoding artifacts). - Adds
scripts/known-translation-gaps.jsonas a documented allowlist (“ratchet”) for existing missing keys per locale. - Wires the new checks into CI (
translations-checkworkflow) and adds a localyarn check:translationsentrypoint.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/known-translation-gaps.json | Adds a documented allowlist of known missing keys per locale to keep the check from blocking existing gaps. |
| scripts/check-translation-parity.js | New Node script that compares locale key sets to en.json and validates translation value correctness. |
| package.json | Adds a check:translations script to run the parity checker locally/consistently. |
| eslint.config.mjs | Excludes scripts/** from ESLint since it’s standalone tooling outside the TS project config. |
| .github/workflows/translations-check.yml | Expands workflow scope and adds a job to run the new parity/value checks in CI. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| failed: (target, base) => { | ||
| const names = (value) => [...new Set(variablesIn(value))].sort().join(",") | ||
| return names(target) !== names(base) | ||
| }, |
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: | ||
| check-sorting: |
Merging main into this branch surfaced 31 createAccount.*/signIn.* keys that DAH-4198 added to en.json without translations, catching the check's first real find in review: es/tl/zh render these in English today. Allowlists them under the same ratchet pattern as the DAH-4213 housing counselor keys, rather than blocking this PR on a Phrase translation cycle. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Problem
loadTranslations()inapp/javascript/util/languageUtil.tsxloads English first and then overlays the target locale, so a key missing fromes.json/tl.json/zh.jsonsilently renders the English string. Nothing in the app surfaces it, and the existingtranslations-checkworkflow only ranjsonlintand a sort check — it never compared key sets or validated string content.Separately, devs have been checking translated values by hand with the Translation-Checker tool, a browser page you paste XLIFF into. Nothing ran its rules automatically. See DAH-3291.
What this adds
scripts/check-translation-parity.jsnow does two things againstapp/assets/json/translations/react/*.json, comparing each locale toen.json:Key parity — fails on keys missing from a locale (silently render in English) or present but empty (render blank, worse than the fallback). Reports, without failing, keys present in a locale but not in
en.json(dead weight).Value checks (ported from Translation-Checker, adapted for JSON rather than XLIFF):
&/nbsp;means double-encoding)%prefix ({name}vs%{name})en.json||||)Two of the value checks are adapted rather than copied 1:1 from the XLF tool, because a literal port produced false positives on real strings: variable comparison uses the set of variable names rather than occurrence counts (a pluralized string legitimately repeats a variable once per form), and plural form count is checked against what each locale needs (1 for
zh, 2 otherwise) rather than againsten.json(English often has one form where Spanish needs two).Runs as a job in the existing
translations-checkworkflow, and locally viayarn check:translations.The allowlist is a ratchet, not a resolution
scripts/known-translation-gaps.jsonis seeded with the keys already missing onmainper locale, grouped under a written reason, so this can merge without blocking unrelated PRs. It should only get shorter — the script flags stale/orphaned allowlist entries.Testing
mainas-is: passes (key parity + all five value checks).es.json, plus deleted/blanked keys — each check fires on exactly its own key, exit 1. Reverted,git diffclean.yarn lintandnpx prettier --checkboth pass.🤖 Generated with Claude Code