fix(security): resolve code-scanning alerts (TOCTOU, HTML sanitization, actions pinning) - #478
Open
BillyOutlast wants to merge 1 commit into
Open
fix(security): resolve code-scanning alerts (TOCTOU, HTML sanitization, actions pinning)#478BillyOutlast wants to merge 1 commit into
BillyOutlast wants to merge 1 commit into
Conversation
CodeQL flagged three classes of issues; this resolves all of them.
js/file-system-race (server/server/internal/objects/fsBackend.ts):
Replace existsSync check-then-use patterns with exception-based access.
All reads/writes now go through a single opened file descriptor
(fs.promises.open -> handle.stat()/createWriteStream/handle.write), so
there is no window between checking a path and using it. Object and
metadata creation uses exclusive-create ('wx') flags so concurrent
creates cannot clobber each other, and writes use 'r+' flags so they
only target files created via create().
js/incomplete-multi-character-sanitization +
js/double-escaping (server/server/internal/metadata/steam.ts):
Rewrite the Steam HTML entity decoder as a single-pass replacement so a
decoded '&' cannot recombine with later passes into new entities
(double-unescaping). Angle brackets are never emitted: </> are
excluded from the decode set and dropped, numeric references encoding
0x3C/0x3E (&Drop-OSS#60;, <, zero-padded variants) are dropped too, and any
residual '<' characters are removed outright since legitimate markup is
already converted to Markdown earlier in the pipeline. HTML comment
removal consumes both closed and unterminated comment openers.
actions/unpinned-tag + actions/missing-workflow-permissions:
Pin all GitHub Actions to full commit SHAs (original tag kept as an
inline comment) across all workflow files, and add explicit workflow-
level permissions to droplet-ci.yml.
Also fixes 7 eslint errors (vue/no-multiple-template-root,
no-useless-assignment) that fire under current plugin versions, so CI
stays green regardless of lockfile drift.
Verified: typecheck clean, build passes, eslint 0 errors, ast-grep scan
clean.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Resolves all open CodeQL code-scanning alerts on this repository across three classes of findings.
1. File-system race conditions (
js/file-system-race) —server/server/internal/objects/fsBackend.tsFsObjectBackendusedexistsSynccheck-then-use patterns, leaving a TOCTOU window between checking a path and acting on it. Rewritten to:stat, read stream, write) through that same fdwx) flags for object + metadata files so concurrent creates cannot clobber each otherr+flags for writes so they only ever target files created viacreate()2. HTML sanitization in Steam metadata (
js/incomplete-multi-character-sanitization,js/double-escaping) —server/server/internal/metadata/steam.tsThe entity decoder applied sequential
replace()passes, so a decoded&could recombine into new entities (double-unescaping), and<script>decoded to live markup. The decoder now:</>are excluded from the decode set and dropped outright, and numeric references encoding0x3C/0x3E(<,<, zero-padded variants) are dropped too<after decoding, since legitimate markup is already converted to Markdown earlier in the pipelineHTML comment removal also consumes unterminated comment openers (
<!-->tricks).3. CI supply-chain hardening (
actions/unpinned-tag,actions/missing-workflow-permissions)permissions: contents: readadded to workflows missing itAlso included
7 eslint errors that fire under current
eslint-plugin-vue/typescript-eslintversions (vue/no-multiple-template-rootfrom a commented-out template block inpages/account/index.vue, dead initializers flagged byno-useless-assignment) — fixed so CI stays green regardless of lockfile drift. The commented-out account-page layout lives in git history.Verification
pnpm run typecheck— cleanpnpm run build— passespnpm run lint— 0 errors (11 pre-existing warnings unchanged)<in output