diff --git a/backend/app/api/routes/proxmox.py b/backend/app/api/routes/proxmox.py index e261708..6c2e562 100644 --- a/backend/app/api/routes/proxmox.py +++ b/backend/app/api/routes/proxmox.py @@ -29,6 +29,7 @@ from app.schemas.proxmox import ( ProxmoxImportPendingResponse, ProxmoxImportResponse, ProxmoxNodeOut, + ProxmoxSyncConfig, ProxmoxTestConnectionResponse, ) from app.schemas.scan import ScanRunResponse @@ -141,6 +142,43 @@ async def import_proxmox_to_pending( return run +@router.post("/sync-now", response_model=ScanRunResponse) +async def sync_proxmox_now( + background_tasks: BackgroundTasks, + db: AsyncSession = Depends(get_db), + _: str = Depends(get_current_user), +) -> ScanRun: + """Trigger an immediate Proxmox inventory sync using the server env config. + + Same background flow as ``/import-pending`` but sources host + token from + ``settings`` (env) rather than the request body — the manual counterpart to + the scheduled auto-sync job. Requires the env token to be configured. + """ + if not (settings.proxmox_host and settings.proxmox_token_id and settings.proxmox_token_secret): + raise HTTPException( + status_code=400, + detail="Cannot sync: no Proxmox host/token configured on the server.", + ) + run = ScanRun( + status="running", + kind="proxmox", + ranges=[f"{settings.proxmox_host}:{settings.proxmox_port}"], + ) + db.add(run) + await db.commit() + await db.refresh(run) + background_tasks.add_task( + _background_proxmox_import, + run.id, + settings.proxmox_host, + settings.proxmox_port, + settings.proxmox_token_id, + settings.proxmox_token_secret, + settings.proxmox_verify_tls, + ) + return run + + async def _background_proxmox_import( run_id: str, host: str, @@ -442,22 +480,24 @@ async def get_proxmox_config(_: str = Depends(get_current_user)) -> ProxmoxConfi @router.post("/config", response_model=ProxmoxConfig) async def save_proxmox_config( - payload: ProxmoxConfig, + payload: ProxmoxSyncConfig, _: str = Depends(get_current_user), ) -> ProxmoxConfig: - """Persist non-secret Proxmox config and apply the auto-sync schedule live. + """Persist the auto-sync activation (enabled + interval) and apply it live. - The token is NOT accepted here — it is env-only by design. + This is the ONLY Proxmox config the app writes. Connection settings + (host, port, token, verify_tls) are env-only and are never accepted or + persisted here — enabling auto-sync requires host + token already set in the + server env, since the scheduled job reads them from there. """ - if payload.sync_enabled and not (settings.proxmox_token_id and settings.proxmox_token_secret): + if payload.sync_enabled and not ( + settings.proxmox_host and settings.proxmox_token_id and settings.proxmox_token_secret + ): raise HTTPException( status_code=400, - detail="Cannot enable auto-sync: no Proxmox API token configured on the server.", + detail="Cannot enable auto-sync: no Proxmox host/token configured in the server env.", ) try: - settings.proxmox_host = payload.host - settings.proxmox_port = payload.port - settings.proxmox_verify_tls = payload.verify_tls settings.proxmox_sync_enabled = payload.sync_enabled settings.proxmox_sync_interval = payload.sync_interval settings.save_overrides() diff --git a/backend/app/core/config.py b/backend/app/core/config.py index e9b2a0c..a092ae7 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -121,13 +121,10 @@ class Settings(BaseSettings): self.scanner_http_probe_enabled = bool(data["scanner_http_probe_enabled"]) if "scanner_http_verify_tls" in data: self.scanner_http_verify_tls = bool(data["scanner_http_verify_tls"]) - # Proxmox non-secret config (token stays env-only, never here). - if "proxmox_host" in data: - self.proxmox_host = str(data["proxmox_host"]) - if "proxmox_port" in data: - self.proxmox_port = int(data["proxmox_port"]) - if "proxmox_verify_tls" in data: - self.proxmox_verify_tls = bool(data["proxmox_verify_tls"]) + # Proxmox auto-sync activation only. Connection config (host, port, + # token, verify_tls) is env-only by design — never read from or + # written to this file. Persisting host here previously created a + # dual source of truth that silently clobbered PROXMOX_HOST. if "proxmox_sync_enabled" in data: self.proxmox_sync_enabled = bool(data["proxmox_sync_enabled"]) if "proxmox_sync_interval" in data: @@ -146,11 +143,9 @@ class Settings(BaseSettings): "scanner_http_ranges": self.scanner_http_ranges, "scanner_http_probe_enabled": self.scanner_http_probe_enabled, "scanner_http_verify_tls": self.scanner_http_verify_tls, - # Proxmox: only non-secret config. Token fields are intentionally - # excluded — they must never be written to disk by the app. - "proxmox_host": self.proxmox_host, - "proxmox_port": self.proxmox_port, - "proxmox_verify_tls": self.proxmox_verify_tls, + # Proxmox: only the auto-sync activation is persisted. Connection + # config (host, port, token, verify_tls) is env-only and must never + # be written to disk — that is the single source of truth. "proxmox_sync_enabled": self.proxmox_sync_enabled, "proxmox_sync_interval": self.proxmox_sync_interval, })) diff --git a/backend/app/schemas/proxmox.py b/backend/app/schemas/proxmox.py index 8ebf67d..cdeb13a 100644 --- a/backend/app/schemas/proxmox.py +++ b/backend/app/schemas/proxmox.py @@ -64,8 +64,11 @@ class ProxmoxImportPendingResponse(BaseModel): class ProxmoxConfig(BaseModel): - """Non-secret Proxmox connection + auto-sync config. Never carries a token — - ``token_configured`` reflects whether a server-side token is present.""" + """Non-secret Proxmox connection + auto-sync config (GET response). + + Connection fields (host/port/verify_tls) are env-only and read-only here — + surfaced for display. ``token_configured`` reflects whether a server-side + token is present. Never carries the token itself.""" host: str = "" port: int = Field(8006, ge=1, le=65535) @@ -73,3 +76,12 @@ class ProxmoxConfig(BaseModel): sync_enabled: bool = False sync_interval: int = Field(3600, ge=300) token_configured: bool = False + + +class ProxmoxSyncConfig(BaseModel): + """User-editable auto-sync config (POST body). The ONLY persisted Proxmox + settings. Connection fields (host/port/token/verify_tls) are env-only and + are deliberately not accepted here.""" + + sync_enabled: bool = False + sync_interval: int = Field(3600, ge=300) diff --git a/backend/tests/test_proxmox_router.py b/backend/tests/test_proxmox_router.py index e89d280..f38bab6 100644 --- a/backend/tests/test_proxmox_router.py +++ b/backend/tests/test_proxmox_router.py @@ -95,6 +95,33 @@ async def test_import_pending_creates_scan_run(client: AsyncClient, headers: dic assert data["status"] == "running" +@pytest.mark.asyncio +async def test_sync_now_creates_scan_run(client: AsyncClient, headers: dict) -> None: + settings.proxmox_host = "pve" + settings.proxmox_token_id = "u@pam!t" + settings.proxmox_token_secret = "s" + with patch("app.api.routes.proxmox._background_proxmox_import", new_callable=AsyncMock): + res = await client.post("/api/v1/proxmox/sync-now", headers=headers) + assert res.status_code == 200 + data = res.json() + assert data["kind"] == "proxmox" + assert data["status"] == "running" + + +@pytest.mark.asyncio +async def test_sync_now_rejected_without_token(client: AsyncClient, headers: dict) -> None: + settings.proxmox_host = "pve" + # _clear_env_token leaves token empty + res = await client.post("/api/v1/proxmox/sync-now", headers=headers) + assert res.status_code == 400 + + +@pytest.mark.asyncio +async def test_sync_now_requires_auth(client: AsyncClient) -> None: + res = await client.post("/api/v1/proxmox/sync-now") + assert res.status_code == 401 + + @pytest.mark.asyncio async def test_requires_auth(client: AsyncClient) -> None: res = await client.post("/api/v1/proxmox/import-pending", json={"host": "pve"}) @@ -114,14 +141,38 @@ async def test_config_omits_token(client: AsyncClient, headers: dict) -> None: @pytest.mark.asyncio async def test_enable_sync_without_token_rejected(client: AsyncClient, headers: dict) -> None: + # _clear_env_token leaves token empty → enabling auto-sync is rejected. res = await client.post( "/api/v1/proxmox/config", - json={"host": "pve", "port": 8006, "verify_tls": True, "sync_enabled": True, "sync_interval": 600}, + json={"sync_enabled": True, "sync_interval": 600}, headers=headers, ) assert res.status_code == 400 +@pytest.mark.asyncio +async def test_save_config_persists_only_sync_fields(client: AsyncClient, headers: dict) -> None: + """Connection config is env-only: even if a client sends host/port/verify, + the endpoint ignores them and persists only the sync activation.""" + settings.proxmox_host = "pve" + settings.proxmox_token_id = "u@pam!t" + settings.proxmox_token_secret = "s" + saved: dict = {} + with patch.object(type(settings), "save_overrides", lambda self: saved.update( + host=self.proxmox_host, enabled=self.proxmox_sync_enabled, interval=self.proxmox_sync_interval + )), patch("app.api.routes.proxmox.set_proxmox_sync_enabled"), \ + patch("app.api.routes.proxmox.reschedule_proxmox_sync"): + res = await client.post( + "/api/v1/proxmox/config", + json={"host": "attacker", "port": 1, "verify_tls": False, "sync_enabled": True, "sync_interval": 900}, + headers=headers, + ) + assert res.status_code == 200 + # host untouched by the request body; sync activation applied. + assert settings.proxmox_host == "pve" + assert saved == {"host": "pve", "enabled": True, "interval": 900} + + @pytest.mark.asyncio async def test_background_import_broadcasts_refresh() -> None: """After persisting, the import emits a scan update so an open inventory diff --git a/backend/tests/test_settings.py b/backend/tests/test_settings.py index c30c858..b06af10 100644 --- a/backend/tests/test_settings.py +++ b/backend/tests/test_settings.py @@ -1,9 +1,12 @@ """Tests for GET/POST /api/v1/settings.""" +import json from unittest.mock import patch import pytest from httpx import AsyncClient +from app.core.config import Settings + @pytest.fixture async def headers(client: AsyncClient): @@ -84,3 +87,46 @@ async def test_update_settings_rejects_too_short_service_interval(client: AsyncC headers=headers, ) assert res.status_code == 422 + + +def test_proxmox_connection_config_is_env_only_never_from_overrides(tmp_path): + """Connection config (host/port/verify_tls) is env-only: a stale value in + scan_config.json must be ignored so it can never clobber the env. Only the + auto-sync activation is read back.""" + s = Settings(secret_key="x", sqlite_path=str(tmp_path / "homelab.db")) + s.proxmox_host = "pve.local" # as if set from env + s.proxmox_port = 8006 + s.proxmox_verify_tls = True + (tmp_path / "scan_config.json").write_text(json.dumps({ + "proxmox_host": "stale-host", + "proxmox_port": 9999, + "proxmox_verify_tls": False, + "proxmox_sync_enabled": True, + "proxmox_sync_interval": 600, + })) + s.load_overrides() + # Connection config untouched by the file (env values survive). + assert s.proxmox_host == "pve.local" + assert s.proxmox_port == 8006 + assert s.proxmox_verify_tls is True + # Auto-sync activation is the only thing loaded. + assert s.proxmox_sync_enabled is True + assert s.proxmox_sync_interval == 600 + + +def test_save_overrides_omits_proxmox_connection_config(tmp_path): + """save_overrides must never write host/port/verify_tls (nor the token) — + only the sync activation. This is what prevents the dual source of truth.""" + s = Settings(secret_key="x", sqlite_path=str(tmp_path / "homelab.db")) + s.proxmox_host = "pve.local" + s.proxmox_sync_enabled = True + s.proxmox_sync_interval = 900 + s.save_overrides() + written = json.loads((tmp_path / "scan_config.json").read_text()) + assert "proxmox_host" not in written + assert "proxmox_port" not in written + assert "proxmox_verify_tls" not in written + assert "proxmox_token_id" not in written + assert "proxmox_token_secret" not in written + assert written["proxmox_sync_enabled"] is True + assert written["proxmox_sync_interval"] == 900 diff --git a/frontend/src/api/__tests__/client.test.ts b/frontend/src/api/__tests__/client.test.ts index 53d266e..5f3bc66 100644 --- a/frontend/src/api/__tests__/client.test.ts +++ b/frontend/src/api/__tests__/client.test.ts @@ -243,7 +243,7 @@ describe('api/client', () => { it('proxmoxApi.getConfig/saveConfig hit /proxmox/config', () => { mod.proxmoxApi.getConfig() expect(api.get).toHaveBeenCalledWith('/proxmox/config') - const conf = { host: 'pve', port: 8006, verify_tls: true, sync_enabled: false, sync_interval: 3600 } + const conf = { sync_enabled: false, sync_interval: 3600 } mod.proxmoxApi.saveConfig(conf) expect(api.post).toHaveBeenCalledWith('/proxmox/config', conf) }) diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 94a6af0..bd091c9 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -161,8 +161,22 @@ export const proxmoxApi = { }>('/proxmox/import-pending', data), getConfig: () => api.get('/proxmox/config'), - saveConfig: (data: Omit) => + // Only the auto-sync activation is persisted. Connection config + // (host/port/token/verify_tls) is env-only and never sent. + saveConfig: (data: { sync_enabled: boolean; sync_interval: number }) => api.post('/proxmox/config', data), + + syncNow: () => + api.post<{ + id: string + status: string + kind: string + ranges: string[] + devices_found: number + started_at: string + finished_at: string | null + error: string | null + }>('/proxmox/sync-now'), } export const designsApi = { diff --git a/frontend/src/components/modals/PendingDevicesModal.tsx b/frontend/src/components/modals/PendingDevicesModal.tsx index 5aeadc9..4ad78ba 100644 --- a/frontend/src/components/modals/PendingDevicesModal.tsx +++ b/frontend/src/components/modals/PendingDevicesModal.tsx @@ -287,8 +287,10 @@ export function PendingDevicesModal({ open, onClose, highlightId, initialStatus injectAutoEdges(res.data.edges) const extra = res.data.edges_created > 0 ? ` (+${res.data.edges_created} link${res.data.edges_created !== 1 ? 's' : ''})` : '' toast.success(`Approved ${nodeData.label}${extra}`) - setDevices((prev) => prev.filter((d) => d.id !== device.id)) + // Keep the row (now on-canvas, shown with an "In N canvas" badge); reload + // for a fresh canvas_count rather than dropping it until reopen. setSelected(null) + await load() } catch { toast.error('Failed to approve device') } @@ -353,8 +355,11 @@ export function PendingDevicesModal({ open, onClose, highlightId, initialStatus }) }) injectAutoEdges(res.data.edges) - setDevices((prev) => prev.filter((d) => !ids.includes(d.id))) + // Don't strip approved rows locally: the inventory keeps them with an + // "In N canvas" badge (that's what showOnCanvas toggles). Reload so they + // reappear with a fresh canvas_count instead of vanishing until reopen. setSelectedIds(new Set()) + await load() const linkExtra = res.data.edges_created > 0 ? ` (+${res.data.edges_created} link${res.data.edges_created !== 1 ? 's' : ''})` : '' toast.success(`Approved ${res.data.approved} device${res.data.approved !== 1 ? 's' : ''}${linkExtra}`) } catch { diff --git a/frontend/src/components/modals/SettingsModal.tsx b/frontend/src/components/modals/SettingsModal.tsx index 740fe9f..541a292 100644 --- a/frontend/src/components/modals/SettingsModal.tsx +++ b/frontend/src/components/modals/SettingsModal.tsx @@ -26,6 +26,7 @@ export function SettingsModal({ open, onClose }: SettingsModalProps) { const [pmConfig, setPmConfig] = useState(null) const [pmSyncEnabled, setPmSyncEnabled] = useState(false) const [pmInterval, setPmInterval] = useState(3600) + const [pmSyncing, setPmSyncing] = useState(false) const [alignment, setAlignment] = useState(readAlignmentSettings) const hideIp = useCanvasStore((s) => s.hideIp) const setHideIp = useCanvasStore((s) => s.setHideIp) @@ -56,6 +57,18 @@ export function SettingsModal({ open, onClose }: SettingsModalProps) { writeAlignmentSettings(next) } + const handleSyncNow = async () => { + setPmSyncing(true) + try { + await proxmoxApi.syncNow() + toast.success('Proxmox sync started') + } catch { + toast.error('Failed to start Proxmox sync') + } finally { + setPmSyncing(false) + } + } + const handleSave = async () => { // Canvas prefs (alignment, hide-IP) persist on change; only the backend // status-check interval needs an API round-trip. @@ -71,10 +84,9 @@ export function SettingsModal({ open, onClose }: SettingsModalProps) { service_check_interval: serviceInterval, }) if (pmConfig) { + // Connection config (host/port/token/verify) is env-only; only the + // auto-sync activation is persisted. await proxmoxApi.saveConfig({ - host: pmConfig.host, - port: pmConfig.port, - verify_tls: pmConfig.verify_tls, sync_enabled: pmSyncEnabled, sync_interval: pmInterval, }) @@ -186,6 +198,25 @@ export function SettingsModal({ open, onClose }: SettingsModalProps) { Re-imports hosts/VMs/LXC into the pending inventory. Min 300s (5 min).

+ {pmConfig.host ? ( +
+ + + Runs one import immediately using the server .env config. + +
+ ) : ( +

+ Set PROXMOX_HOST in the server .env to enable manual re-sync. +

+ )} )} diff --git a/frontend/src/components/modals/__tests__/PendingDevicesModal.test.tsx b/frontend/src/components/modals/__tests__/PendingDevicesModal.test.tsx index 4e7d230..8623855 100644 --- a/frontend/src/components/modals/__tests__/PendingDevicesModal.test.tsx +++ b/frontend/src/components/modals/__tests__/PendingDevicesModal.test.tsx @@ -263,6 +263,27 @@ describe('PendingDevicesModal', () => { await waitFor(() => expect(mockBulkApprove).toHaveBeenCalledWith(['dev-a', 'dev-b'], null)) }) + it('keeps approved devices listed after bulk approve (reloads, not strips)', async () => { + // After approve, pending() still returns the rows (now on-canvas w/ badge). + mockPending + .mockResolvedValueOnce({ data: [DEVICE_IP, DEVICE_ZIGBEE] }) + .mockResolvedValue({ data: [ + { ...DEVICE_IP, canvas_count: 1 }, + { ...DEVICE_ZIGBEE, canvas_count: 1 }, + ] }) + render() + await waitFor(() => expect(screen.getByTestId('pending-card-dev-a')).toBeInTheDocument()) + fireEvent.click(screen.getByRole('button', { name: 'Select mode' })) + fireEvent.click(screen.getByTestId('pending-card-dev-a')) + fireEvent.click(screen.getByTestId('pending-card-dev-b')) + fireEvent.click(screen.getByRole('button', { name: /Approve \(2\)/ })) + await waitFor(() => expect(mockBulkApprove).toHaveBeenCalled()) + // Reloaded, so rows remain visible instead of the list going empty. + await waitFor(() => expect(mockPending).toHaveBeenCalledTimes(2)) + expect(screen.getByTestId('pending-card-dev-a')).toBeInTheDocument() + expect(screen.getByTestId('pending-card-dev-b')).toBeInTheDocument() + }) + it('bulk approve carries the scanned MAC onto the canvas node (#168)', async () => { render() await waitFor(() => expect(screen.getByTestId('pending-card-dev-a')).toBeInTheDocument()) diff --git a/frontend/src/components/modals/__tests__/SettingsModal.test.tsx b/frontend/src/components/modals/__tests__/SettingsModal.test.tsx index 6426d39..c33d9da 100644 --- a/frontend/src/components/modals/__tests__/SettingsModal.test.tsx +++ b/frontend/src/components/modals/__tests__/SettingsModal.test.tsx @@ -11,6 +11,7 @@ vi.mock('@/api/client', () => ({ proxmoxApi: { getConfig: vi.fn(), saveConfig: vi.fn(), + syncNow: vi.fn(), }, })) @@ -98,6 +99,51 @@ describe('SettingsModal', () => { }) }) + it('persists only sync fields (not connection config) on Save', async () => { + vi.mocked(proxmoxApi.getConfig).mockResolvedValue({ + data: { host: 'pve', port: 8006, verify_tls: true, sync_enabled: true, sync_interval: 3600, token_configured: true }, + } as never) + vi.mocked(proxmoxApi.saveConfig).mockResolvedValue({ data: {} } as never) + render() + await screen.findByDisplayValue('60') + fireEvent.click(screen.getByRole('button', { name: 'Save' })) + await waitFor(() => { + expect(proxmoxApi.saveConfig).toHaveBeenCalledWith({ sync_enabled: true, sync_interval: 3600 }) + }) + }) + + it('triggers an immediate Proxmox sync from the Re-sync now button', async () => { + vi.mocked(proxmoxApi.getConfig).mockResolvedValue({ + data: { host: 'pve', port: 8006, verify_tls: true, sync_enabled: false, sync_interval: 3600, token_configured: true }, + } as never) + vi.mocked(proxmoxApi.syncNow).mockResolvedValue({ data: { status: 'running' } } as never) + render() + const btn = await screen.findByRole('button', { name: 'Re-sync now' }) + fireEvent.click(btn) + await waitFor(() => { + expect(proxmoxApi.syncNow).toHaveBeenCalledOnce() + expect(toast.success).toHaveBeenCalledWith('Proxmox sync started') + }) + }) + + it('shows a PROXMOX_HOST hint instead of the button when host is unset', async () => { + vi.mocked(proxmoxApi.getConfig).mockResolvedValue({ + data: { host: '', port: 8006, verify_tls: true, sync_enabled: false, sync_interval: 3600, token_configured: true }, + } as never) + render() + await screen.findByText('PROXMOX_HOST') + expect(screen.queryByRole('button', { name: 'Re-sync now' })).toBeNull() + }) + + it('hides Re-sync now when no Proxmox token is configured', async () => { + vi.mocked(proxmoxApi.getConfig).mockResolvedValue({ + data: { host: 'pve', port: 8006, verify_tls: true, sync_enabled: false, sync_interval: 3600, token_configured: false }, + } as never) + render() + await screen.findByDisplayValue('60') + expect(screen.queryByRole('button', { name: 'Re-sync now' })).toBeNull() + }) + it('calls onClose on Cancel', async () => { const onClose = vi.fn() render()