Skip to content

fix(sonar): constrain update-platforms CLI paths to the repo (tssecurity:S8707) - #206

Merged
setchy merged 5 commits into
mainfrom
fix/sonar/tssecurity-S8707
Oct 7, 2026
Merged

setchy merged 5 commits into
mainfrom
fix/sonar/tssecurity-S8707

Conversation

@setchy

@setchy setchy commented Oct 3, 2026 •

Copy link
Copy Markdown
Member

Summary

The update-platforms.mts scripts (visual + e2e) accepted process.argv results-dir and target-file paths and passed them straight into readFileSync/readdirSync/writeFileSync, which rule tssecurity:S8707 flags as a path-traversal vector.

Both scripts now validate the resolved path stays within the working directory and refuse anything that escapes it. The validation lives in one shared helper so the two scripts don't carry duplicate copies (which also keeps the SonarCloud duplication gate green).

Changes

Path Change
tests/util/resolve-from-cwd.mts new shared resolveFromCwd(): resolves a CLI path from the working directory and refuses anything that escapes it
tests/visual/update-platforms.mts import resolveFromCwd() and validate process.argv[2] / argv[3]
tests/e2e/update-platforms.mts import resolveFromCwd() and validate process.argv[2] / argv[3]

Verification

  • pnpm lint (oxlint + oxfmt) — clean
  • pnpm typecheck — clean
  • pnpm test — 129/129 passing
  • Smoke-tested under plain node (the CI invocation): traversal arguments (../evil) are refused with exit 1; in-repo paths resolve normally
  • SonarCloud PR analysis — 0 open issues, quality gate OK

Findings

SonarQube rule tssecurity:S8707 flags path traversal through the
command-line arguments accepted by the platform table updater scripts. Both
scripts now resolve any provided results/target path relative to the
working directory and exit when the path would escape it.
@setchy setchy changed the title fix(tests): address SonarQube tssecurity:S8707 findings fix(sonar): constrain update-platforms CLI paths to the repo (tssecurity:S8707) Oct 3, 2026
@setchy
setchy marked this pull request as ready for review October 7, 2026 12:15
@setchy
setchy requested a review from afonsojramos as a code owner October 7, 2026 12:15
The previous guard only checked for a leading '..' in the relative path,
which path.relative() does not produce for a target on a different Windows
drive (it returns the target verbatim), letting e.g. D:\evil through. It
also rejected legitimate names that merely start with '..' (e.g. '..foo').

Reject rel === '..' or rel starting with '..' + sep, plus any isAbsolute(rel),
and return the resolved absolute path so callers cannot re-resolve a stale
relative value.
@sonarqubecloud

sonarqubecloud Bot commented Oct 7, 2026

Copy link
Copy Markdown

@setchy
setchy merged commit 4e1ce13 into main Oct 7, 2026
28 checks passed
@setchy
setchy deleted the fix/sonar/tssecurity-S8707 branch October 7, 2026 12:40
setchy added a commit that referenced this pull request Oct 7, 2026
…:S8707) (#233)

#206 wrapped process.argv path overrides in a custom resolveFromCwd guard,
but Sonar's taint analysis does not recognize the guard: the flow still
reaches readdirSync/readFileSync/writeFileSync, so all 6 S8707 issues stayed
open on main after that merge.

Both workflows invoke these scripts with no arguments, so the CLI overrides
were unused. Use fixed relative paths and delete the now-dead helper.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant