Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions src-tauri/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

43 changes: 25 additions & 18 deletions src-tauri/src/security.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use aes_gcm::{
};
use argon2::{Argon2, Params, Version};
use base64::{engine::general_purpose, Engine as _};
use rand::RngCore;
use rand::{Rng, RngCore};
use std::collections::HashMap;
use std::fs;
use std::io::Write;
Expand Down Expand Up @@ -968,8 +968,7 @@ impl Crypto {
let cipher =
Aes256Gcm::new_from_slice(key).map_err(|e| SecurityError::Encryption(e.to_string()))?;

let mut nonce_bytes = [0u8; NONCE_LEN];
OsRng.fill_bytes(&mut nonce_bytes);
let nonce_bytes: [u8; NONCE_LEN] = OsRng.gen();
let nonce = Nonce::from_slice(&nonce_bytes);

let ciphertext = cipher
Expand Down Expand Up @@ -1012,19 +1011,18 @@ impl Crypto {

let argon2 = Argon2::new(argon2::Algorithm::Argon2id, Version::V0x13, params);

let mut key = [0u8; KEY_LEN];
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()
Comment on lines +1014 to +1019
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 👍 / 👎.

.map_err(|_| SecurityError::KeyDerivation("invalid derived key length".into()))
}

/// Generate random salt
pub fn generate_salt() -> [u8; SALT_LEN] {
let mut salt = [0u8; SALT_LEN];
OsRng.fill_bytes(&mut salt);
salt
OsRng.gen()
}

/// Wrap master key with passphrase-derived key
Expand Down Expand Up @@ -1066,8 +1064,10 @@ impl Crypto {
return Err(SecurityError::InvalidKeyFormat);
}

let mut key = [0u8; KEY_LEN];
key.copy_from_slice(&key_bytes);
let key = key_bytes
.as_slice()
.try_into()
.map_err(|_| SecurityError::InvalidKeyFormat)?;

// Zeroize the intermediate buffer
key_bytes.zeroize();
Expand Down Expand Up @@ -1155,6 +1155,10 @@ pub fn secure_delete_file(path: &Path) -> std::io::Result<()> {
mod tests {
use super::*;

fn test_passphrase(label: &str) -> String {
format!("{label}-{}", rand::random::<u64>())
}

#[test]
fn test_master_key_generation() {
let key1 = MasterKey::generate();
Expand All @@ -1176,30 +1180,33 @@ mod tests {
#[test]
fn test_key_wrapping() {
let master_key = MasterKey::generate();
let passphrase = "test-passphrase-123";
let passphrase = test_passphrase("key-wrapping");

let wrapped = Crypto::wrap_key(&master_key, passphrase).unwrap();
let unwrapped = Crypto::unwrap_key(&wrapped, passphrase).unwrap();
let wrapped = Crypto::wrap_key(&master_key, &passphrase).unwrap();
let unwrapped = Crypto::unwrap_key(&wrapped, &passphrase).unwrap();

assert_eq!(master_key.as_bytes(), unwrapped.as_bytes());
}

#[test]
fn test_wrong_passphrase_fails() {
let master_key = MasterKey::generate();
let wrapped = Crypto::wrap_key(&master_key, "correct-passphrase").unwrap();
let correct_passphrase = test_passphrase("correct");
let wrong_passphrase = test_passphrase("wrong");
let wrapped = Crypto::wrap_key(&master_key, &correct_passphrase).unwrap();

let result = Crypto::unwrap_key(&wrapped, "wrong-passphrase");
let result = Crypto::unwrap_key(&wrapped, &wrong_passphrase);
assert!(result.is_err());
}

#[test]
fn test_export_crypto() {
let data = b"Export test data";
let password = "export-password";
let password = test_passphrase("export");

let (ciphertext, salt, nonce) = ExportCrypto::encrypt_for_export(data, password).unwrap();
let decrypted = ExportCrypto::decrypt_export(&ciphertext, &salt, &nonce, password).unwrap();
let (ciphertext, salt, nonce) = ExportCrypto::encrypt_for_export(data, &password).unwrap();
let decrypted =
ExportCrypto::decrypt_export(&ciphertext, &salt, &nonce, &password).unwrap();

assert_eq!(data.as_slice(), decrypted.as_slice());
}
Expand Down
38 changes: 23 additions & 15 deletions src-tauri/tests/security.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ use assistsupport_lib::security::{
use std::fs;
use tempfile::TempDir;

fn test_passphrase(label: &str) -> String {
format!("{label}-{}", rand::random::<u64>())
}

// ============================================================================
// Master Key Tests
// ============================================================================
Expand Down Expand Up @@ -236,10 +240,10 @@ fn test_encryption_large_data() {
#[test]
fn test_key_wrapping_with_passphrase() {
let master_key = MasterKey::generate();
let passphrase = "my-secure-passphrase-123";
let passphrase = test_passphrase("key-wrap");

let wrapped = Crypto::wrap_key(&master_key, passphrase).expect("Wrapping failed");
let unwrapped = Crypto::unwrap_key(&wrapped, passphrase).expect("Unwrapping failed");
let wrapped = Crypto::wrap_key(&master_key, &passphrase).expect("Wrapping failed");
let unwrapped = Crypto::unwrap_key(&wrapped, &passphrase).expect("Unwrapping failed");

assert_eq!(
master_key.as_bytes(),
Expand All @@ -251,9 +255,11 @@ fn test_key_wrapping_with_passphrase() {
#[test]
fn test_key_wrapping_wrong_passphrase_fails() {
let master_key = MasterKey::generate();
let wrapped = Crypto::wrap_key(&master_key, "correct-passphrase").expect("Wrapping failed");
let correct_passphrase = test_passphrase("correct");
let wrong_passphrase = test_passphrase("wrong");
let wrapped = Crypto::wrap_key(&master_key, &correct_passphrase).expect("Wrapping failed");

let result = Crypto::unwrap_key(&wrapped, "wrong-passphrase");
let result = Crypto::unwrap_key(&wrapped, &wrong_passphrase);
assert!(
result.is_err(),
"Unwrapping with wrong passphrase should fail"
Expand All @@ -263,11 +269,11 @@ fn test_key_wrapping_wrong_passphrase_fails() {
#[test]
fn test_key_wrapping_produces_different_output() {
let master_key = MasterKey::generate();
let passphrase = "test-passphrase";
let passphrase = test_passphrase("different-output");

// Wrap the same key twice
let wrapped1 = Crypto::wrap_key(&master_key, passphrase).expect("Wrapping failed");
let wrapped2 = Crypto::wrap_key(&master_key, passphrase).expect("Wrapping failed");
let wrapped1 = Crypto::wrap_key(&master_key, &passphrase).expect("Wrapping failed");
let wrapped2 = Crypto::wrap_key(&master_key, &passphrase).expect("Wrapping failed");

// Salts should be different
assert_ne!(
Expand All @@ -276,8 +282,8 @@ fn test_key_wrapping_produces_different_output() {
);

// Both should unwrap correctly
let unwrapped1 = Crypto::unwrap_key(&wrapped1, passphrase).expect("Unwrapping failed");
let unwrapped2 = Crypto::unwrap_key(&wrapped2, passphrase).expect("Unwrapping failed");
let unwrapped1 = Crypto::unwrap_key(&wrapped1, &passphrase).expect("Unwrapping failed");
let unwrapped2 = Crypto::unwrap_key(&wrapped2, &passphrase).expect("Unwrapping failed");

assert_eq!(unwrapped1.as_bytes(), master_key.as_bytes());
assert_eq!(unwrapped2.as_bytes(), master_key.as_bytes());
Expand All @@ -290,12 +296,12 @@ fn test_key_wrapping_produces_different_output() {
#[test]
fn test_export_crypto_roundtrip() {
let data = b"Export test data with various content: 123!@#";
let password = "export-password-456";
let password = test_passphrase("export");

let (ciphertext, salt, nonce) =
ExportCrypto::encrypt_for_export(data, password).expect("Export encryption failed");
ExportCrypto::encrypt_for_export(data, &password).expect("Export encryption failed");

let decrypted = ExportCrypto::decrypt_export(&ciphertext, &salt, &nonce, password)
let decrypted = ExportCrypto::decrypt_export(&ciphertext, &salt, &nonce, &password)
.expect("Decryption failed");

assert_eq!(
Expand All @@ -308,10 +314,12 @@ fn test_export_crypto_roundtrip() {
#[test]
fn test_export_crypto_wrong_password_fails() {
let data = b"Secret export data";
let (ciphertext, salt, nonce) = ExportCrypto::encrypt_for_export(data, "correct-password")
let correct_password = test_passphrase("correct-export");
let wrong_password = test_passphrase("wrong-export");
let (ciphertext, salt, nonce) = ExportCrypto::encrypt_for_export(data, &correct_password)
.expect("Export encryption failed");

let result = ExportCrypto::decrypt_export(&ciphertext, &salt, &nonce, "wrong-password");
let result = ExportCrypto::decrypt_export(&ciphertext, &salt, &nonce, &wrong_password);
assert!(
result.is_err(),
"Export decryption with wrong password should fail"
Expand Down
161 changes: 161 additions & 0 deletions src/components/Ingest/YouTubeIngest.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
// @vitest-environment jsdom
import React from "react";
import { afterEach, describe, expect, it, vi } from "vitest";
import { cleanup, render, screen, waitFor } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { isAllowedYouTubeUrl, YouTubeIngest } from "./YouTubeIngest";

const mocks = vi.hoisted(() => ({
ingestYoutube: vi.fn(),
ingesting: false,
}));

vi.mock("../../hooks/useIngest", () => ({
useIngest: () => ({
ingestYoutube: mocks.ingestYoutube,
ingesting: mocks.ingesting,
}),
}));

vi.mock("../shared/Button", () => ({
Button: ({
children,
onClick,
disabled,
}: {
children: React.ReactNode;
onClick?: () => void;
disabled?: boolean;
}) => (
<button type="button" onClick={onClick} disabled={disabled}>
{children}
</button>
),
}));

afterEach(() => {
cleanup();
mocks.ingestYoutube.mockReset();
mocks.ingesting = false;
});

describe("isAllowedYouTubeUrl", () => {
it("accepts canonical YouTube hosts", () => {
expect(
isAllowedYouTubeUrl("https://www.youtube.com/watch?v=dQw4w9WgXcQ"),
).toBe(true);
expect(isAllowedYouTubeUrl("https://youtube.com/watch?v=dQw4w9WgXcQ")).toBe(
true,
);
expect(isAllowedYouTubeUrl("https://youtu.be/dQw4w9WgXcQ")).toBe(true);
});

it("rejects lookalike hosts that only contain YouTube text", () => {
expect(
isAllowedYouTubeUrl(
"https://youtube.com.evil.example/watch?v=dQw4w9WgXcQ",
),
).toBe(false);
expect(
isAllowedYouTubeUrl("https://notyoutube.com/watch?v=dQw4w9WgXcQ"),
).toBe(false);
});

it("rejects malformed or unsupported URLs", () => {
expect(isAllowedYouTubeUrl("youtube.com/watch?v=dQw4w9WgXcQ")).toBe(false);
expect(
isAllowedYouTubeUrl("ftp://www.youtube.com/watch?v=dQw4w9WgXcQ"),
).toBe(false);
});
});

describe("YouTubeIngest", () => {
it("shows the install warning when yt-dlp is unavailable", () => {
render(
<YouTubeIngest
namespaceId="default"
ytdlpAvailable={false}
onSuccess={vi.fn()}
onError={vi.fn()}
/>,
);

expect(screen.getByText("yt-dlp Not Installed")).toBeTruthy();
expect(screen.getByText("brew install yt-dlp")).toBeTruthy();
expect(screen.queryByLabelText("YouTube URL")).toBeNull();
});

it("rejects invalid YouTube lookalike hosts before ingesting", async () => {
const user = userEvent.setup();
const onError = vi.fn();
render(
<YouTubeIngest
namespaceId="default"
ytdlpAvailable={true}
onSuccess={vi.fn()}
onError={onError}
/>,
);

await user.type(
screen.getByLabelText("YouTube URL"),
"https://youtube.com.evil.example/watch?v=dQw4w9WgXcQ",
);
await user.click(screen.getByRole("button", { name: "Ingest Transcript" }));

expect(onError).toHaveBeenCalledWith("Please enter a valid YouTube URL");
expect(mocks.ingestYoutube).not.toHaveBeenCalled();
});

it("ingests valid trimmed YouTube URLs and clears the field", async () => {
const user = userEvent.setup();
const onSuccess = vi.fn();
mocks.ingestYoutube.mockResolvedValue({
title: "Demo video",
chunk_count: 2,
word_count: 120,
});
render(
<YouTubeIngest
namespaceId="default"
ytdlpAvailable={true}
onSuccess={onSuccess}
onError={vi.fn()}
/>,
);

const input = screen.getByLabelText("YouTube URL") as HTMLInputElement;
await user.type(input, " https://youtu.be/dQw4w9WgXcQ ");
await user.click(screen.getByRole("button", { name: "Ingest Transcript" }));

await waitFor(() => {
expect(mocks.ingestYoutube).toHaveBeenCalledWith(
"https://youtu.be/dQw4w9WgXcQ",
"default",
);
});
expect(onSuccess).toHaveBeenCalledWith(
'Ingested "Demo video" (2 chunks, 120 words)',
);
expect(input.value).toBe("");
});

it("submits with Enter and shows the ingesting label", async () => {
const user = userEvent.setup();
mocks.ingesting = true;
render(
<YouTubeIngest
namespaceId="default"
ytdlpAvailable={true}
onSuccess={vi.fn()}
onError={vi.fn()}
/>,
);

expect(screen.getByRole("button", { name: "Ingesting..." })).toBeTruthy();
expect(screen.getByRole("button").hasAttribute("disabled")).toBe(true);

await user.keyboard("{Enter}");
expect(mocks.ingestYoutube).not.toHaveBeenCalled();
});
});
Loading
Loading