Skip to content

fix(security): address Snyk Code SAST findings (path traversal, secrets, command injection, info exposure) - #41

Open
devin-ai-integration[bot] wants to merge 1 commit into
masterfrom
devin/1789368294-snyk-sast-fixes
Open

devin-ai-integration[bot] wants to merge 1 commit into
masterfrom
devin/1789368294-snyk-sast-fixes

Conversation

@devin-ai-integration

Copy link
Copy Markdown

SUMMARY

Minimal root-cause fixes for Snyk Code findings (companion to #40, which covers dependency findings). No behavior changes for valid inputs.

Fixed

  • Path Traversal superset/app.py:119,126SupersetApp.send_static_file rejects filenames that werkzeug.security.safe_join(static_folder, filename) cannot resolve inside the static folder (raise NotFound()) before delegating to Flask.
  • XSS superset/views/api.py:76Api.query returned a raw json.dumps string (served as text/html); wrapped with json_success(...) so the response is application/json.
  • Externally-Controlled Format String callApi.ts:158console.error uses a constant %s format string with key/url as arguments instead of interpolating them into the format string.
  • Hardcoded Secret docs/scripts/generate-database-docs.mjs:71SUPERSET_SECRET_KEY is taken from the environment or generated via crypto.randomBytes(32) for the throwaway docs-build app instead of the literal 'docs-build-key'.
  • Hardcoded Secret legacy-plugin-chart-map-box/src/stories/data.ts:5529 — Mapbox token read from process.env.STORYBOOK_MAPBOX_API_KEY (defaults to ''; the story still renders, tiles just don't load without a token).
  • Command Injection + Prototype Pollution docs/scripts/manage-versions.mjs:100,126,202execSync(shell string)execFileSync('yarn', [...args]); section must be one of docs|developer_portal|components and version must match ^\d+\.\d+(\.\d+)?$ before being used as an object key / path segment / CLI arg.
  • Information Exposure – Server Error Message webpack.proxy-config.js:211 — stack trace is logged server-side; client receives only e.message.

Assessed as false positives (no code change)

  • docs/docusaurus.config.ts:806 — Algolia DocSearch apiKey is the public, search-only key that must be shipped to browsers by design.
  • superset-websocket/src/config.ts:66 — flagged line is jwtCookieName: 'async-token', a cookie name, not a secret (jwtSecret defaults to '' and is supplied via config/env).
  • cypress-base/cypress/support/e2e.ts:184 (admin/general) — test-fixture credentials for the Cypress test instance.
  • "Use of Hardcoded Passwords" in superset/db_engine_specs/*.py — these are parameters_json_schema doc placeholders / example strings for the connection form, not credentials.

ReDoS callApi.ts:103,117 — no regular expression exists at these lines (Cache.match(url)); Snyk's taint path is the URL string flowing into the Cache API. Treated as a false positive; the format-string finding in the same file was fixed.

Snyk IDs addressed: Snyk Code findings have no SNYK-* IDs; the affected file:line locations are listed above.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TESTING INSTRUCTIONS

  • ruff check superset/app.py superset/views/api.py and ruff format --check pass; mypy reports no new errors on changed lines (674 pre-existing errors identical before/after).
  • cd superset-frontend && npx prettier --check + oxlint on changed files pass; jest packages/superset-ui-core/test/connection — 6 suites / 89 tests pass.
  • node docs/scripts/manage-versions.mjs add 'docs; echo pwned' 1.0.0Unknown section; ... add docs '1.0.0;touch x'Invalid version.
  • pre-commit run on staged files passes (only pre-existing TS6305 "lib not built" errors from the frontend type-check hook, unrelated to this change).
  • Manual: GET /static/../../etc/passwd-style paths (URL-encoded) on a running Superset return 404.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

Link to Devin session: https://app.devin.ai/sessions/d59c5fde422d413ebdccece790177516
Open in Devin Desktop: https://app.devin.ai/desktop/session/d59c5fde422d413ebdccece790177516?variant=devin

- app.py: reject static paths escaping static_folder (safe_join)
- views/api.py: return JSON with proper content type via json_success
- callApi.ts: use printf-style console.error instead of interpolated format string
- map-box stories: read Mapbox token from STORYBOOK_MAPBOX_API_KEY
- generate-database-docs.mjs: random/ env-provided SUPERSET_SECRET_KEY
- manage-versions.mjs: execFileSync + section/version validation
- webpack.proxy-config.js: don't leak stack traces to client

Co-Authored-By: shayan <shayan@cognition.ai>
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants