Skip to content

fix: clear AssistSupport security alerts#116

Merged
saagpatel merged 2 commits into
masterfrom
codex/fix/security-alerts
May 17, 2026
Merged

fix: clear AssistSupport security alerts#116
saagpatel merged 2 commits into
masterfrom
codex/fix/security-alerts

Conversation

@saagpatel
Copy link
Copy Markdown
Owner

What

  • Tightens YouTube URL validation to require a parsed YouTube host.
  • Updates Rust crypto tests/helpers to avoid hard-coded passphrase values and fixed seed buffers.
  • Updates OpenSSL lockfile entries to patched releases.

Why

  • GitHub code scanning flagged incomplete URL substring validation and hard-coded cryptographic values.
  • OSV flagged the locked OpenSSL crate version.

How

  • Uses URL parsing plus exact/subdomain host checks for YouTube ingest.
  • Generates test passphrases dynamically and uses RNG-generated byte arrays for nonce/salt material.
  • Refreshes the Cargo lockfile for OpenSSL only.

Testing

  • Commands run:
    • pnpm vitest run src/components/Ingest/YouTubeIngest.test.ts
    • pnpm build
    • pnpm lint
    • pnpm typecheck
    • pnpm exec tsc --noEmit && pnpm exec eslint src/components/Ingest/YouTubeIngest.tsx src/components/Ingest/YouTubeIngest.test.ts
    • cargo test --manifest-path src-tauri/Cargo.toml security --tests
    • cargo test --manifest-path src-tauri/Cargo.toml --test security
    • pnpm test:security:audit:rust
    • pnpm test:security-regression
    • git diff --check
  • Results: passed locally. Rust audit still reports the repo's allowed lower-priority transitive warnings, but no OpenSSL CVE remains in the local audit output.

Performance impact

  • Bundle delta: none expected for this validation/helper change.
  • Build time delta: none expected.
  • Lighthouse delta: not run; no UI rendering flow changed.
  • API latency delta: none expected.
  • DB query delta: none expected.

Risk / Notes

  • GitHub CodeQL and OSV checks remain the authoritative closeout signal after this PR runs.
  • URL validation still permits http: YouTube URLs to preserve existing ingest behavior.

Screenshots (UI only)

  • N/A; validation logic only.

Lockfile rationale (if lockfile changed)

  • Cargo.lock was updated only for OpenSSL packages to remediate OSV-reported OpenSSL advisories.

Validate YouTube hosts with URL parsing.

Randomize crypto test passphrases and avoid fixed seed buffers.

Update OpenSSL lockfile entries to patched releases.

Tests: focused frontend, Rust, audit, and security regression checks passed.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a156ec6036

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src-tauri/src/security.rs
Comment on lines +1014 to +1019
let mut key = vec![0; KEY_LEN];
argon2
.hash_password_into(passphrase.as_bytes(), salt, &mut key)
.map_err(|e| SecurityError::KeyDerivation(e.to_string()))?;

Ok(key)
key.try_into()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Zeroize the derived key buffer before deallocation

When passphrase-derived keys are generated, this now stores the Argon2 output in a heap Vec and immediately consumes it with try_into(). In every passphrase wrap/unwrap/export path, that drops the original heap allocation without zeroizing it, so the KEK can remain in freed heap memory even though the callers only zeroize the returned array and the surrounding docs say the derived key is zeroized after use. Please derive into a fixed [u8; KEY_LEN] as before or wrap the temporary buffer in a zeroizing type before converting it.

Useful? React with 👍 / 👎.

Add component-level coverage for URL rejection and ingest success paths.

Tests: YouTube ingest test, frontend coverage, diff coverage, lint, and typecheck passed.
@saagpatel saagpatel merged commit 9f5580e into master May 17, 2026
33 checks passed
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.

1 participant