fix(security): resolve open code-scanning alerts (XSS, int-conversion, cookie, CI perms)#108
Merged
Merged
Conversation
…, cookie, CI perms) Addresses all 12 open alerts from GitHub code scanning: High: - go/incorrect-integer-conversion (classifier.go, skills/importer.go): both inet_aton-style IP parsers now reject out-of-range parts instead of silently truncating (300.1.1.1 no longer parses as 44.1.1.1; 10.300.1.1 no longer slips into the private range as 10.44.1.1), matching browser semantics and protecting the SSRF/internal-IP checks downstream. - go/incomplete-url-scheme-check (browser_tool.go): link extraction now filters javascript:, data:, and vbscript: case-insensitively. - js/xss (ui/app.js): formatNum coerces to a finite number and the latency stat is coerced, so crafted non-numeric values can never reach innerHTML as markup. - js/xss-through-dom (ui/app.js): renderFileChips builds DOM nodes with textContent instead of innerHTML. - js/double-escaping (ui/app.js): escapeAttr replaces & first so quote entities are no longer double-escaped. Medium: - go/cookie-secure-not-set (serve.go): the WS-token cookie sets Secure: true. The UI always also authenticates via WebSocket subprotocol and /api header, so browsers that drop the cookie lose nothing. - actions/missing-workflow-permissions: top-level permissions: contents: read in test.yml and release.yml (the release job keeps its explicit contents: write). Regression tests: IP bound cases in both packages, browser scheme-filter test. Full test suite, vet, golangci-lint, and node --check all pass.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
odek | 6813f02 | Commit Preview URL Branch Preview URL |
Jul 26 2026, 10:26 AM |
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 12 open alerts from GitHub code scanning in one PR. Each was assessed before fixing — all were valid (none dismissed as false positives).
High
go/incorrect-integer-conversion(internal/danger/classifier.go,internal/skills/importer.go) — both inet_aton-style IP parsers truncated out-of-range parts:300.1.1.1parsed as44.1.1.1, and10.300.1.1as10.44.1.1(a private address — directly relevant to the SSRF/internal-IP checks these parsers feed). Both now validate per inet_aton rules (leading parts ≤0xFF; final part fits the remaining bytes) and reject invalid input, matching browser semantics. Regression cases pinned inTestParseBrowserIPand the importer'sisPrivateHosttable.go/incomplete-url-scheme-check(cmd/odek/browser_tool.go) — link extraction only skipped lowercasejavascript:. Now filtersjavascript:,data:, andvbscript:case-insensitively. NewTestBrowser_SkipsDangerousLinkSchemesproves all three schemes (plus caps variants) are excluded while safe links survive.js/xss(cmd/odek/ui/app.js) —formatNumpassed non-numeric values throughString(n)straight intoinnerHTML. It now coerces to a finite number; the latency stat is coerced the same way, so a crafted event value can never reach the DOM as markup.js/xss-through-dom(cmd/odek/ui/app.js) —renderFileChipsbuilt chips viainnerHTMLwith a file name in the flow. Rewritten withcreateElement/textContent+addEventListener(also drops the inlineonclick).js/double-escaping(cmd/odek/ui/app.js) —escapeAttrreplaced&last, producing".&is now replaced first.Medium
go/cookie-secure-not-set(cmd/odek/serve.go) — the WS-token cookie now setsSecure: true. This is safe for the bundled UI because it always also sends the token as a WebSocket subprotocol (odek.<token>) and as theX-Odek-Ws-Tokenheader on/api/*, so a browser that drops the cookie on plain-http loopback loses nothing (comment explains this).actions/missing-workflow-permissions(×4) — top-levelpermissions: contents: readadded totest.ymlandrelease.yml. The release job keeps its explicitpermissions: contents: writeoverride; no job needs more.Verification
go test ./... -count=1✅ (28 packages),go vet ./...✅,golangci-lint run ./...→ 0 issues ✅node --check cmd/odek/ui/app.js✅