diff --git a/backend/secuscan/executor.py b/backend/secuscan/executor.py index a8523a245..d44baac7c 100644 --- a/backend/secuscan/executor.py +++ b/backend/secuscan/executor.py @@ -571,8 +571,12 @@ async def _execute_standard_scanner( # Apply Docker Sandboxing if enabled if settings.docker_enabled: + from .sandbox_executor import resolve_sandbox_config + sandbox_cfg = resolve_sandbox_config(getattr(plugin, 'sandbox', None)) + await self._ensure_docker_network() docker_image = plugin.docker_image or "alpine:latest" + docker_network = "none" if not sandbox_cfg.allow_network else settings.docker_network docker_cmd = [ "docker", "run", @@ -584,7 +588,7 @@ async def _execute_standard_scanner( "--cpus", str(settings.sandbox_cpu_quota), "--cap-drop", "NET_RAW", - "--network", settings.docker_network, + "--network", docker_network, docker_image, ] command = docker_cmd + command diff --git a/backend/secuscan/migrations/009_add_saved_view_ownership.sql b/backend/secuscan/migrations/009_add_saved_view_ownership.sql new file mode 100644 index 000000000..d928206ab --- /dev/null +++ b/backend/secuscan/migrations/009_add_saved_view_ownership.sql @@ -0,0 +1,34 @@ +-- Migration: 009_add_saved_view_ownership +-- Adds owner_id scoping and a shared flag to saved_views. +-- +-- owner_id prevents cross-user IDOR: user A cannot modify/delete user B's +-- private view. The shared flag lets teams publish views as read-only to +-- other users without exposing mutation. +-- +-- Backward-compatible: existing rows get owner_id = 'default' which +-- matches the DEFAULT_OWNER_ID constant in auth.py. +-- +-- NOTE: The original table had UNIQUE(name). Ownership scoping requires +-- per-owner uniqueness instead (same name allowed across owners), so we +-- recreate the table without the global UNIQUE constraint. + +CREATE TABLE saved_views_new ( + id TEXT PRIMARY KEY, + name TEXT NOT NULL, + filter_json TEXT NOT NULL, + owner_id TEXT NOT NULL DEFAULT 'default', + shared INTEGER NOT NULL DEFAULT 0, + created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP +); + +INSERT INTO saved_views_new (id, name, filter_json, created_at, updated_at) + SELECT id, name, filter_json, created_at, updated_at + FROM saved_views; + +DROP TABLE saved_views; + +ALTER TABLE saved_views_new RENAME TO saved_views; + +CREATE INDEX idx_saved_views_owner_id ON saved_views(owner_id); +CREATE INDEX idx_saved_views_shared ON saved_views(shared); diff --git a/backend/secuscan/models.py b/backend/secuscan/models.py index 75661c3ab..67746c688 100644 --- a/backend/secuscan/models.py +++ b/backend/secuscan/models.py @@ -166,6 +166,7 @@ class PluginMetadata(BaseModel): learning: Optional[Dict[str, Any]] = None dependencies: Optional[Dict[str, List[str]]] = None docker_image: Optional[str] = None + sandbox: Optional[SandboxConfig] = None checksum: Optional[str] = None signature: Optional[str] = None diff --git a/backend/secuscan/sandbox_executor.py b/backend/secuscan/sandbox_executor.py index bb80ef180..318d69cd6 100644 --- a/backend/secuscan/sandbox_executor.py +++ b/backend/secuscan/sandbox_executor.py @@ -114,6 +114,21 @@ async def sandbox_execute( violation_reason is None on success, or one of "timeout", "memory_limit", "output_limit". """ + # Enforce network isolation when allow_network is False. + # On Linux, use unshare(1) to create a new network namespace so the + # subprocess cannot reach any network interface. On non-Linux platforms + # we log a warning; full enforcement requires Docker --network none. + if not config.allow_network: + if IS_LINUX: + cmd = ["unshare", "--net", "--"] + cmd + logger.info("Network namespace isolation enabled (unshare --net)") + else: + logger.warning( + "allow_network=False requested but network namespace isolation " + "requires Linux unshare. Network access is NOT restricted on %s.", + platform.system(), + ) + preexec_fn = _build_preexec_fn(config) if IS_LINUX else None rss_before = 0 diff --git a/backend/secuscan/saved_views.py b/backend/secuscan/saved_views.py index 889ecc406..f0c4e7507 100644 --- a/backend/secuscan/saved_views.py +++ b/backend/secuscan/saved_views.py @@ -4,7 +4,7 @@ import uuid from typing import Any, Dict, List, Optional -from fastapi import APIRouter, HTTPException +from fastapi import APIRouter, Depends, HTTPException, Request from pydantic import BaseModel, Field, field_validator from .database import get_db @@ -14,6 +14,18 @@ _VALID_SORT_MODES = {"severity", "newest", "oldest", "target"} _VALID_SEVERITIES = {"all", "critical", "high", "medium", "low", "info"} +# Default owner for backward-compatible single-user deployments. +_DEFAULT_OWNER = "default" +_OWNER_HEADER = "x-user-id" + + +async def _get_owner(request: Request) -> str: + """FastAPI dependency that resolves the caller's owner identity.""" + user_id = request.headers.get(_OWNER_HEADER) + if user_id and user_id.strip(): + return f"user:{user_id.strip()}" + return _DEFAULT_OWNER + class FilterPreset(BaseModel): """Validated representation of the frontend filter state.""" @@ -44,6 +56,7 @@ class SavedViewCreate(BaseModel): """Request body for POST /saved-views.""" name: str = Field(..., min_length=1, max_length=60) filter_json: str + shared: bool = False @field_validator("name") @classmethod @@ -68,6 +81,7 @@ class SavedViewUpdate(BaseModel): """Request body for PUT /saved-views/{id}.""" name: Optional[str] = Field(None, min_length=1, max_length=60) filter_json: Optional[str] = None + shared: Optional[bool] = None @field_validator("name") @classmethod @@ -96,27 +110,30 @@ def validate_filter_json(cls, v: Optional[str]) -> Optional[str]: @saved_views_router.get("") -async def list_saved_views() -> Dict[str, Any]: - """Return all saved views ordered by creation date.""" +async def list_saved_views(owner: str = Depends(_get_owner)) -> Dict[str, Any]: + """Return saved views visible to the caller: own views + shared views from others.""" db = await get_db() rows: List[Dict] = await db.fetchall( - "SELECT id, name, filter_json, created_at, updated_at " - "FROM saved_views ORDER BY created_at ASC" + "SELECT id, name, filter_json, owner_id, shared, created_at, updated_at " + "FROM saved_views WHERE owner_id = ? OR shared = 1 " + "ORDER BY created_at ASC", + (owner,), ) return {"views": rows, "total": len(rows)} @saved_views_router.post("", status_code=201) -async def create_saved_view(body: SavedViewCreate) -> Dict[str, Any]: +async def create_saved_view(body: SavedViewCreate, owner: str = Depends(_get_owner)) -> Dict[str, Any]: """ Create a new saved view. - Returns 409 if a view with the same name already exists. + Returns 409 if a view with the same name already exists for this owner. """ db = await get_db() existing = await db.fetchone( - "SELECT id FROM saved_views WHERE LOWER(name) = LOWER(?)", (body.name,) + "SELECT id FROM saved_views WHERE LOWER(name) = LOWER(?) AND owner_id = ?", + (body.name, owner), ) if existing: raise HTTPException( @@ -128,34 +145,45 @@ async def create_saved_view(body: SavedViewCreate) -> Dict[str, Any]: view_id = str(uuid.uuid4()) await db.execute( """ - INSERT INTO saved_views (id, name, filter_json) - VALUES (?, ?, ?) + INSERT INTO saved_views (id, name, filter_json, owner_id, shared) + VALUES (?, ?, ?, ?, ?) """, - (view_id, body.name, body.filter_json), + (view_id, body.name, body.filter_json, owner, int(body.shared)), ) return {"id": view_id, "name": body.name, "created": True} @saved_views_router.put("/{view_id}") -async def update_saved_view(view_id: str, body: SavedViewUpdate) -> Dict[str, Any]: +async def update_saved_view(view_id: str, body: SavedViewUpdate, owner: str = Depends(_get_owner)) -> Dict[str, Any]: """ Overwrite name and/or filter_json for an existing view. Also accepts PATCH semantics — only supplied fields are updated. + Only the owner can modify a view; shared views are read-only to others. """ db = await get_db() - row = await db.fetchone("SELECT id FROM saved_views WHERE id = ?", (view_id,)) + row = await db.fetchone( + "SELECT id, owner_id, shared FROM saved_views WHERE id = ?", (view_id,) + ) if not row: raise HTTPException(status_code=404, detail="Saved view not found") + if row["owner_id"] != owner: + if row["shared"]: + raise HTTPException( + status_code=403, + detail="Cannot modify a shared view owned by another user.", + ) + raise HTTPException(status_code=403, detail="Not authorized to modify this view.") + updates: List[str] = [] params: List[Any] = [] if body.name is not None: - # Check for name collision with a *different* record + # Check for name collision with a *different* record owned by the same user collision = await db.fetchone( - "SELECT id FROM saved_views WHERE LOWER(name) = LOWER(?) AND id != ?", - (body.name, view_id), + "SELECT id FROM saved_views WHERE LOWER(name) = LOWER(?) AND id != ? AND owner_id = ?", + (body.name, view_id, owner), ) if collision: raise HTTPException( @@ -169,6 +197,10 @@ async def update_saved_view(view_id: str, body: SavedViewUpdate) -> Dict[str, An updates.append("filter_json = ?") params.append(body.filter_json) + if body.shared is not None: + updates.append("shared = ?") + params.append(int(body.shared)) + if not updates: raise HTTPException(status_code=400, detail="No fields to update") @@ -183,8 +215,17 @@ async def update_saved_view(view_id: str, body: SavedViewUpdate) -> Dict[str, An @saved_views_router.delete("/{view_id}") -async def delete_saved_view(view_id: str) -> Dict[str, Any]: - """Delete a saved view by id. Idempotent — returns 200 even if not found.""" +async def delete_saved_view(view_id: str, owner: str = Depends(_get_owner)) -> Dict[str, Any]: + """Delete a saved view by id. Only the owner can delete their views.""" db = await get_db() + row = await db.fetchone( + "SELECT id, owner_id FROM saved_views WHERE id = ?", (view_id,) + ) + if not row: + return {"id": view_id, "deleted": True} + + if row["owner_id"] != owner: + raise HTTPException(status_code=403, detail="Not authorized to delete this view.") + await db.execute("DELETE FROM saved_views WHERE id = ?", (view_id,)) return {"id": view_id, "deleted": True} \ No newline at end of file diff --git a/frontend/src/pages/Toolkit.tsx b/frontend/src/pages/Toolkit.tsx index 6398072f1..c92e4b82e 100644 --- a/frontend/src/pages/Toolkit.tsx +++ b/frontend/src/pages/Toolkit.tsx @@ -432,7 +432,7 @@ export default function Scanner() {
-

+

{tool.name}

@@ -443,7 +443,7 @@ export default function Scanner() {

{tool.isPlugin && tool.availability && tool.availability.missing_binaries.length > 0 && ( -
+
{tool.availability.guidance || `Unavailable: Requires external binaries (${tool.availability.missing_binaries.join(', ')})`}
diff --git a/frontend/testing/unit/pages/Scans.test.tsx b/frontend/testing/unit/pages/Scans.test.tsx index c863d17ee..7d87346f1 100644 --- a/frontend/testing/unit/pages/Scans.test.tsx +++ b/frontend/testing/unit/pages/Scans.test.tsx @@ -279,4 +279,96 @@ describe('Scans — task list', () => { // Error is shown via toast, not the banner — no Close alert button expected expect(screen.queryByRole('button', { name: /Close alert/i })).not.toBeInTheDocument() }) + + it('handles bulkDeleteTasks failure: rows persist and error toast shown', async () => { + const { bulkDeleteTasks } = await import('../../../src/api') + vi.mocked(bulkDeleteTasks).mockRejectedValueOnce(new Error('Network Error or Internal Server Error')) + + const tasks = [ + makeTask({ task_id: 'task-1', tool: 'nmap', target: 'target1.com' }), + makeTask({ task_id: 'task-2', tool: 'nmap', target: 'target2.com' }), + ] + mockFetch(tasks) + renderScans() + + await waitFor(() => expect(screen.getByText('target1.com')).toBeInTheDocument()) + expect(screen.getByText('target2.com')).toBeInTheDocument() + + // Select all tasks + await userEvent.click(screen.getByRole('button', { name: /Select_All/i })) + await waitFor(() => { + expect(screen.getByText('2')).toBeInTheDocument() + }) + + // Click the bulk delete button + const deleteBtn = screen.getByRole('button', { name: /Prune_Selected/i }) + await userEvent.click(deleteBtn) + + // Confirm modal should appear + await waitFor(() => expect(screen.getByText('Bulk Delete Records')).toBeInTheDocument()) + + // Click Confirm + const confirmBtn = screen.getByRole('button', { name: /Confirm/i }) + await userEvent.click(confirmBtn) + + // Assert error toast is shown + await waitFor(() => { + const alertEl = screen.getByRole('alert') + expect(alertEl).toBeInTheDocument() + expect(alertEl.textContent).toMatch(/Failed to delete some tasks/) + }) + + // Assert rows are NOT removed — they persist despite the API failure + expect(screen.getByText('target1.com')).toBeInTheDocument() + expect(screen.getByText('target2.com')).toBeInTheDocument() + + // Assert selection count is still shown (not cleared) + expect(screen.getByText('2')).toBeInTheDocument() + }) + + it('bulkDeleteTasks failure does not clear selection state', async () => { + const { bulkDeleteTasks } = await import('../../../src/api') + vi.mocked(bulkDeleteTasks).mockRejectedValueOnce(new Error('Server Error')) + + const tasks = [ + makeTask({ task_id: 'task-1', tool: 'nmap', target: 'target1.com' }), + makeTask({ task_id: 'task-2', tool: 'nmap', target: 'target2.com' }), + makeTask({ task_id: 'task-3', tool: 'nmap', target: 'target3.com' }), + ] + mockFetch(tasks) + renderScans() + + await waitFor(() => expect(screen.getByText('target1.com')).toBeInTheDocument()) + + // Select only first two tasks by clicking their selection divs (material icon "add") + const addIcons = screen.getAllByText('add') + await userEvent.click(addIcons[0]) + await userEvent.click(addIcons[1]) + + await waitFor(() => { + expect(screen.getByText('2')).toBeInTheDocument() + }) + + // Click bulk delete + const deleteBtn = screen.getByRole('button', { name: /Prune_Selected/i }) + await userEvent.click(deleteBtn) + + await waitFor(() => expect(screen.getByText('Bulk Delete Records')).toBeInTheDocument()) + + const confirmBtn = screen.getByRole('button', { name: /Confirm/i }) + await userEvent.click(confirmBtn) + + // Wait for error + await waitFor(() => { + const alertEl = screen.getByRole('alert') + expect(alertEl).toBeInTheDocument() + }) + + // Verify the two tasks are still visible + expect(screen.getByText('target1.com')).toBeInTheDocument() + expect(screen.getByText('target2.com')).toBeInTheDocument() + + // Verify selection count is still 2 (not cleared) + expect(screen.getByText('2')).toBeInTheDocument() + }) }) diff --git a/testing/backend/unit/test_sandbox_executor.py b/testing/backend/unit/test_sandbox_executor.py index 49e946ec5..3713e3fe4 100644 --- a/testing/backend/unit/test_sandbox_executor.py +++ b/testing/backend/unit/test_sandbox_executor.py @@ -5,15 +5,18 @@ sandbox_execute end-to-end): - classify_memory_violation: exit-code, stderr, and RSS threshold heuristics - resolve_sandbox_config: global defaults merged with per-plugin overrides +- sandbox_execute network namespace enforcement (unshare --net) """ -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, patch, AsyncMock +import asyncio import pytest from backend.secuscan.sandbox_executor import ( classify_memory_violation, resolve_sandbox_config, + sandbox_execute, ) from backend.secuscan.models import SandboxConfig @@ -186,3 +189,71 @@ def test_resolve_sandbox_config_full_override(): assert config.max_memory_mb == 256 assert config.max_output_bytes == 1_000_000 assert config.allow_network is False + + +# --------------------------------------------------------------------------- +# sandbox_execute – network namespace enforcement +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_sandbox_execute_wraps_with_unshare_net_when_network_disabled(): + """When allow_network=False on Linux, the command is prefixed with unshare --net.""" + config = SandboxConfig(allow_network=False, timeout_seconds=5) + + mock_process = AsyncMock() + mock_process.stdout = AsyncMock() + mock_process.stderr = AsyncMock() + mock_process.returncode = 0 + + async def fake_read(*args, **kwargs): + return b"" + + mock_process.stdout.read = fake_read + mock_process.stderr.read = fake_read + + with patch("backend.secuscan.sandbox_executor.IS_LINUX", True), \ + patch("backend.secuscan.sandbox_executor.asyncio.create_subprocess_exec", new_callable=AsyncMock) as mock_exec, \ + patch("backend.secuscan.sandbox_executor._build_preexec_fn", return_value=None): + + mock_exec.return_value = mock_process + mock_process.wait = AsyncMock(return_value=0) + + await sandbox_execute(["echo", "hello"], config) + + mock_exec.assert_called_once() + called_args = mock_exec.call_args[0] + assert called_args[0] == "unshare" + assert called_args[1] == "--net" + assert called_args[2] == "--" + assert called_args[3:] == ("echo", "hello") + + +@pytest.mark.asyncio +async def test_sandbox_execute_no_unshare_when_network_allowed(): + """When allow_network=True, the command is NOT wrapped with unshare.""" + config = SandboxConfig(allow_network=True, timeout_seconds=5) + + mock_process = AsyncMock() + mock_process.stdout = AsyncMock() + mock_process.stderr = AsyncMock() + mock_process.returncode = 0 + + async def fake_read(*args, **kwargs): + return b"" + + mock_process.stdout.read = fake_read + mock_process.stderr.read = fake_read + + with patch("backend.secuscan.sandbox_executor.IS_LINUX", True), \ + patch("backend.secuscan.sandbox_executor.asyncio.create_subprocess_exec", new_callable=AsyncMock) as mock_exec, \ + patch("backend.secuscan.sandbox_executor._build_preexec_fn", return_value=None): + + mock_exec.return_value = mock_process + mock_process.wait = AsyncMock(return_value=0) + + await sandbox_execute(["echo", "hello"], config) + + mock_exec.assert_called_once() + called_args = mock_exec.call_args[0] + assert called_args[0] == "echo" + assert called_args[1] == "hello" diff --git a/testing/backend/unit/test_saved_views.py b/testing/backend/unit/test_saved_views.py index e52b55f3c..36462a104 100644 --- a/testing/backend/unit/test_saved_views.py +++ b/testing/backend/unit/test_saved_views.py @@ -510,3 +510,163 @@ async def test_database_newer_than_application_fails(tmp_path): with pytest.raises(RuntimeError, match="Database schema is newer"): await db.connect() + + +# ─── Ownership scoping (issue #1879) ──────────────────────────────────────── + +OWNER_A = "user:alice" +OWNER_B = "user:bob" + + +@pytest.mark.asyncio +async def test_create_stores_owner_id(app_client: AsyncClient): + """Created views carry the caller's owner_id.""" + res = await app_client.post( + "/api/v1/saved-views", + json=make_body("Owned View"), + headers={"X-User-Id": "alice"}, + ) + assert res.status_code == 201 + + list_res = await app_client.get( + "/api/v1/saved-views", + headers={"X-User-Id": "alice"}, + ) + views = list_res.json()["views"] + assert len(views) == 1 + assert views[0]["owner_id"] == OWNER_A + + +@pytest.mark.asyncio +async def test_user_b_cannot_see_user_a_private_views(app_client: AsyncClient): + """Private views of user A are invisible to user B.""" + await app_client.post( + "/api/v1/saved-views", + json=make_body("Alice Private"), + headers={"X-User-Id": "alice"}, + ) + + list_res = await app_client.get( + "/api/v1/saved-views", + headers={"X-User-Id": "bob"}, + ) + assert list_res.json()["views"] == [] + assert list_res.json()["total"] == 0 + + +@pytest.mark.asyncio +async def test_user_b_can_see_user_a_shared_views(app_client: AsyncClient): + """Shared views from user A are visible to user B.""" + await app_client.post( + "/api/v1/saved-views", + json={**make_body("Alice Shared"), "shared": True}, + headers={"X-User-Id": "alice"}, + ) + + list_res = await app_client.get( + "/api/v1/saved-views", + headers={"X-User-Id": "bob"}, + ) + assert list_res.json()["total"] == 1 + assert list_res.json()["views"][0]["name"] == "Alice Shared" + + +@pytest.mark.asyncio +async def test_user_b_cannot_modify_user_a_shared_view(app_client: AsyncClient): + """Shared views are read-only to non-owners.""" + create_res = await app_client.post( + "/api/v1/saved-views", + json={**make_body("Shared View"), "shared": True}, + headers={"X-User-Id": "alice"}, + ) + view_id = create_res.json()["id"] + + put_res = await app_client.put( + f"/api/v1/saved-views/{view_id}", + json={"name": "Hacked Name"}, + headers={"X-User-Id": "bob"}, + ) + assert put_res.status_code == 403 + + +@pytest.mark.asyncio +async def test_user_b_cannot_delete_user_a_private_view(app_client: AsyncClient): + """Non-owners cannot delete private views.""" + create_res = await app_client.post( + "/api/v1/saved-views", + json=make_body("Alice Private"), + headers={"X-User-Id": "alice"}, + ) + view_id = create_res.json()["id"] + + del_res = await app_client.delete( + f"/api/v1/saved-views/{view_id}", + headers={"X-User-Id": "bob"}, + ) + assert del_res.status_code == 403 + + # Verify view still exists for alice + list_res = await app_client.get( + "/api/v1/saved-views", + headers={"X-User-Id": "alice"}, + ) + assert list_res.json()["total"] == 1 + + +@pytest.mark.asyncio +async def test_owner_can_delete_own_view(app_client: AsyncClient): + """Owners can delete their own views.""" + create_res = await app_client.post( + "/api/v1/saved-views", + json=make_body("Delete Me"), + headers={"X-User-Id": "alice"}, + ) + view_id = create_res.json()["id"] + + del_res = await app_client.delete( + f"/api/v1/saved-views/{view_id}", + headers={"X-User-Id": "alice"}, + ) + assert del_res.status_code == 200 + assert del_res.json()["deleted"] is True + + +@pytest.mark.asyncio +async def test_duplicate_name_scoped_to_owner(app_client: AsyncClient): + """Two different owners can create views with the same name.""" + await app_client.post( + "/api/v1/saved-views", + json=make_body("Same Name"), + headers={"X-User-Id": "alice"}, + ) + res = await app_client.post( + "/api/v1/saved-views", + json=make_body("Same Name"), + headers={"X-User-Id": "bob"}, + ) + assert res.status_code == 201 + + +@pytest.mark.asyncio +async def test_shared_flag_toggle(app_client: AsyncClient): + """Owner can toggle the shared flag on their view.""" + create_res = await app_client.post( + "/api/v1/saved-views", + json={**make_body("Toggle Me"), "shared": False}, + headers={"X-User-Id": "alice"}, + ) + view_id = create_res.json()["id"] + + put_res = await app_client.put( + f"/api/v1/saved-views/{view_id}", + json={"shared": True}, + headers={"X-User-Id": "alice"}, + ) + assert put_res.status_code == 200 + + # Now bob can see it + list_res = await app_client.get( + "/api/v1/saved-views", + headers={"X-User-Id": "bob"}, + ) + assert list_res.json()["total"] == 1