diff --git a/apps/backend/fastapi/main.py b/apps/backend/fastapi/main.py index fd07ee0c9..7211a720f 100644 --- a/apps/backend/fastapi/main.py +++ b/apps/backend/fastapi/main.py @@ -1,16 +1,21 @@ import math import os +import secrets import sys from contextlib import asynccontextmanager from datetime import date, datetime from decimal import Decimal from pathlib import Path +from typing import Annotated import numpy as np import pandas as pd +import sqlglot +from sqlglot import exp +from sqlglot.errors import ParseError import uvicorn from dotenv import load_dotenv -from fastapi import FastAPI, HTTPException +from fastapi import Depends, FastAPI, Header, HTTPException from fastapi.middleware.cors import CORSMiddleware from pydantic import BaseModel @@ -23,6 +28,8 @@ from nao_core.context import get_context_provider port = int(os.environ.get("PORT", 8005)) +INTERNAL_AUTH_HEADER = "X-Nao-Internal-Secret" +MIN_INTERNAL_SECRET_LENGTH = 20 # Global scheduler instance scheduler = None @@ -97,6 +104,7 @@ class ExecuteSQLRequest(BaseModel): database_id: str | None = None env_vars: dict[str, str] | None = None azure_access_token: str | None = None + dangerously_write_permission_enabled: bool = False class ExecuteSQLResponse(BaseModel): @@ -161,6 +169,45 @@ def _convert_value(v: object): return v +def _require_internal_auth( + provided_secret: Annotated[ + str | None, + Header(alias=INTERNAL_AUTH_HEADER), + ] = None, +): + expected_secret = os.environ.get("BETTER_AUTH_SECRET") + if not expected_secret or len(expected_secret) < MIN_INTERNAL_SECRET_LENGTH: + raise HTTPException( + status_code=503, + detail="Internal API authentication is not configured", + ) + + provided_bytes = provided_secret.encode() if provided_secret is not None else b"" + if not secrets.compare_digest(provided_bytes, expected_secret.encode()): + raise HTTPException(status_code=401, detail="Invalid internal API credentials") + + +def _is_read_only_sql(sql: str) -> bool: + try: + statements = sqlglot.parse(sql, error_level=sqlglot.ErrorLevel.RAISE) + except (ParseError, ValueError): + return False + + if not statements: + return False + + forbidden_expression_types = (exp.DDL, exp.DML, exp.Into, exp.Lock) + return all( + statement is not None + and isinstance(statement, exp.Query) + and not any( + isinstance(expression, forbidden_expression_types) + for expression in statement.walk() + ) + for statement in statements + ) + + # ============================================================================= # API Endpoints # ============================================================================= @@ -187,7 +234,11 @@ async def health_check(): ) -@app.post("/api/refresh", response_model=RefreshResponse) +@app.post( + "/api/refresh", + response_model=RefreshResponse, + dependencies=[Depends(_require_internal_auth)], +) async def refresh_context(): """Trigger a context refresh (git pull if using git source). @@ -219,9 +270,21 @@ async def refresh_context(): ) -@app.post("/execute_sql", response_model=ExecuteSQLResponse) +@app.post( + "/execute_sql", + response_model=ExecuteSQLResponse, + dependencies=[Depends(_require_internal_auth)], +) async def execute_sql(request: ExecuteSQLRequest): try: + if not request.dangerously_write_permission_enabled and not _is_read_only_sql( + request.sql + ): + raise HTTPException( + status_code=403, + detail="Write SQL operations are disabled", + ) + project_path = Path(request.nao_project_folder) config = NaoConfig.try_load( project_path, @@ -300,4 +363,4 @@ async def execute_sql(request: ExecuteSQLRequest): if __name__ == "__main__": - uvicorn.run("main:app", host="0.0.0.0", port=port, reload=True) + uvicorn.run("main:app", host="127.0.0.1", port=port, reload=True) diff --git a/apps/backend/fastapi/test_main.py b/apps/backend/fastapi/test_main.py index fe7d6860e..5bfee41ae 100644 --- a/apps/backend/fastapi/test_main.py +++ b/apps/backend/fastapi/test_main.py @@ -1,3 +1,4 @@ +import os import tempfile from pathlib import Path @@ -6,6 +7,11 @@ from fastapi.testclient import TestClient from main import app +TEST_INTERNAL_SECRET = "test-internal-secret-at-least-20-characters" +os.environ["BETTER_AUTH_SECRET"] = TEST_INTERNAL_SECRET + +AUTH_HEADERS = {"X-Nao-Internal-Secret": TEST_INTERNAL_SECRET} + def assert_sql_result( data: dict, *, row_count: int, columns: list[str], expected_data: list[dict] @@ -43,6 +49,7 @@ def test_execute_sql_simple_duckdb(duckdb_project_folder): response = client.post( "/execute_sql", + headers=AUTH_HEADERS, json={ "sql": "SELECT 1 AS id, 'hello' AS message", "nao_project_folder": duckdb_project_folder, @@ -64,6 +71,7 @@ def test_execute_sql_with_cte_duckdb(duckdb_project_folder): response = client.post( "/execute_sql", + headers=AUTH_HEADERS, json={ "sql": "WITH test AS (SELECT 1 AS id, 'hello' AS message) SELECT * FROM test", "nao_project_folder": duckdb_project_folder, @@ -79,6 +87,114 @@ def test_execute_sql_with_cte_duckdb(duckdb_project_folder): ) +def test_health_does_not_require_internal_auth(): + response = TestClient(app).get("/health") + + assert response.status_code == 200 + + +@pytest.mark.parametrize( + "headers", + [ + {}, + {"X-Nao-Internal-Secret": "incorrect-secret-at-least-20-characters"}, + ], +) +def test_execute_sql_requires_internal_auth(duckdb_project_folder, headers): + response = TestClient(app).post( + "/execute_sql", + headers=headers, + json={ + "sql": "SELECT 1", + "nao_project_folder": duckdb_project_folder, + }, + ) + + assert response.status_code == 401 + + +def test_execute_sql_fails_closed_without_configured_secret( + duckdb_project_folder, + monkeypatch, +): + monkeypatch.delenv("BETTER_AUTH_SECRET") + + response = TestClient(app).post( + "/execute_sql", + headers=AUTH_HEADERS, + json={ + "sql": "SELECT 1", + "nao_project_folder": duckdb_project_folder, + }, + ) + + assert response.status_code == 503 + + +def test_refresh_requires_internal_auth(): + response = TestClient(app).post("/api/refresh") + + assert response.status_code == 401 + + +@pytest.mark.parametrize( + "sql", + [ + "DELETE FROM users", + "SELECT 1; DROP TABLE users", + "WITH deleted AS (DELETE FROM users RETURNING *) SELECT * FROM deleted", + "SELECT * INTO copied_users FROM users", + "SELECT * FROM users FOR UPDATE", + "SELECT (", + ], +) +def test_execute_sql_rejects_non_read_only_sql(duckdb_project_folder, sql): + response = TestClient(app).post( + "/execute_sql", + headers=AUTH_HEADERS, + json={ + "sql": sql, + "nao_project_folder": duckdb_project_folder, + }, + ) + + assert response.status_code == 403 + assert response.json()["detail"] == "Write SQL operations are disabled" + + +def test_execute_sql_allows_authenticated_write_permission(monkeypatch): + class FakeDatabase: + name = "test" + type = "duckdb" + auth_mode = None + + def execute_sql(self, sql): + import pandas as pd + + assert sql == "DELETE FROM users" + return pd.DataFrame() + + class FakeConfig: + databases = [FakeDatabase()] + + monkeypatch.setattr( + "main.NaoConfig.try_load", + lambda *args, **kwargs: FakeConfig(), + ) + + response = TestClient(app).post( + "/execute_sql", + headers=AUTH_HEADERS, + json={ + "sql": "DELETE FROM users", + "nao_project_folder": "/tmp/test-project", + "dangerously_write_permission_enabled": True, + }, + ) + + assert response.status_code == 200 + + # BigQuery tests (requires SSO authentication) @@ -109,6 +225,7 @@ def test_execute_sql_simple_bigquery(bigquery_project_folder): response = client.post( "/execute_sql", + headers=AUTH_HEADERS, json={ "sql": "SELECT 1 AS id, 'hello' AS message", "nao_project_folder": bigquery_project_folder, @@ -139,6 +256,7 @@ def test_execute_sql_with_cte_bigquery(bigquery_project_folder): response = client.post( "/execute_sql", + headers=AUTH_HEADERS, json={ "sql": cte_sql, "nao_project_folder": bigquery_project_folder, diff --git a/apps/backend/src/agents/tools/execute-sql.ts b/apps/backend/src/agents/tools/execute-sql.ts index 04c658d72..ee9e05c69 100644 --- a/apps/backend/src/agents/tools/execute-sql.ts +++ b/apps/backend/src/agents/tools/execute-sql.ts @@ -30,10 +30,12 @@ export async function executeQuery( method: 'POST', headers: { 'Content-Type': 'application/json', + 'X-Nao-Internal-Secret': env.BETTER_AUTH_SECRET, }, body: JSON.stringify({ sql: sql_query, nao_project_folder: naoProjectFolder, + dangerously_write_permission_enabled: writePermEnabled, ...(database_id && { database_id }), ...(Object.keys(envVars).length > 0 && { env_vars: envVars }), ...(context.azureAccessToken && { azure_access_token: context.azureAccessToken }), diff --git a/apps/backend/src/services/context-explorer.service.ts b/apps/backend/src/services/context-explorer.service.ts index 3908b6b76..acd263e2c 100644 --- a/apps/backend/src/services/context-explorer.service.ts +++ b/apps/backend/src/services/context-explorer.service.ts @@ -9,7 +9,7 @@ export async function getFileTree(projectFolder: string): Promise { - const realPath = resolveAndValidatePath(filePath, projectFolder); + const realPath = await resolveAndValidatePath(filePath, projectFolder); const stat = await fs.stat(realPath); const MAX_FILE_SIZE = 1024 * 1024; // 1 MB @@ -59,14 +59,16 @@ async function readDirectoryRecursive(dirPath: string, projectFolder: string): P return entries; } -function resolveAndValidatePath(virtualPath: string, projectFolder: string): string { +async function resolveAndValidatePath(virtualPath: string, projectFolder: string): Promise { const relativePath = virtualPath.startsWith('/') ? virtualPath.slice(1) : virtualPath; - const resolvedPath = path.resolve(projectFolder, relativePath); + const realProjectFolder = await fs.realpath(projectFolder); + const resolvedPath = path.resolve(realProjectFolder, relativePath); + const realPath = await fs.realpath(resolvedPath); - const withinFolder = resolvedPath === projectFolder || resolvedPath.startsWith(projectFolder + path.sep); + const withinFolder = realPath === realProjectFolder || realPath.startsWith(realProjectFolder + path.sep); if (!withinFolder) { throw new Error(`Access denied: path is outside the project folder`); } - return resolvedPath; + return realPath; } diff --git a/apps/backend/src/services/live-story.ts b/apps/backend/src/services/live-story.ts index e82bb24fb..91a2d24e6 100644 --- a/apps/backend/src/services/live-story.ts +++ b/apps/backend/src/services/live-story.ts @@ -130,7 +130,10 @@ async function executeRawSql( ): Promise<{ data: unknown[]; columns: string[] }> { const response = await fetch(`http://localhost:${env.FASTAPI_PORT}/execute_sql`, { method: 'POST', - headers: { 'Content-Type': 'application/json' }, + headers: { + 'Content-Type': 'application/json', + 'X-Nao-Internal-Secret': env.BETTER_AUTH_SECRET, + }, body: JSON.stringify({ sql: sqlQuery, nao_project_folder: projectFolder, diff --git a/apps/backend/src/trpc/account.routes.ts b/apps/backend/src/trpc/account.routes.ts index a380c83ca..1064295c3 100644 --- a/apps/backend/src/trpc/account.routes.ts +++ b/apps/backend/src/trpc/account.routes.ts @@ -2,11 +2,13 @@ import { TRPCError } from '@trpc/server'; import { hashPassword } from 'better-auth/crypto'; import { z } from 'zod/v4'; +import { isCloud } from '../env'; import * as accountQueries from '../queries/account.queries'; import * as projectQueries from '../queries/project.queries'; import * as userQueries from '../queries/user.queries'; import { emailService } from '../services/email'; import { buildResetPasswordEmail } from '../utils/email-builders'; +import { assertAdminPasswordResetAllowed } from '../utils/password-reset'; import { regexPassword } from '../utils/utils'; import { adminProtectedProcedure, protectedProcedure } from './trpc'; @@ -18,6 +20,8 @@ export const accountRoutes = { }), ) .mutation(async ({ input, ctx }) => { + assertAdminPasswordResetAllowed(isCloud); + const account = await accountQueries.getAccountById(input.userId); if (!account || !account.password) { throw new TRPCError({ diff --git a/apps/backend/src/trpc/organization.routes.ts b/apps/backend/src/trpc/organization.routes.ts index 0094aa5a1..f15969387 100644 --- a/apps/backend/src/trpc/organization.routes.ts +++ b/apps/backend/src/trpc/organization.routes.ts @@ -10,6 +10,7 @@ import { emailService } from '../services/email'; import { addTeamMember } from '../services/team-member'; import { ORG_ROLES } from '../types/organization'; import { buildResetPasswordEmail, buildUserAddedEmail } from '../utils/email-builders'; +import { assertAdminPasswordResetAllowed } from '../utils/password-reset'; import { isPublicEmailDomain, normalizeEmailDomains } from '../utils/utils'; import { protectedProcedure } from './trpc'; @@ -138,6 +139,8 @@ export const organizationRoutes = { resetMemberPassword: orgAdminOnlyProcedure .input(z.object({ userId: z.string() })) .mutation(async ({ input, ctx }) => { + assertAdminPasswordResetAllowed(isCloud); + const memberRole = await orgQueries.getUserRoleInOrg(ctx.org.id, input.userId); if (!memberRole) { throw new TRPCError({ code: 'FORBIDDEN', message: 'User is not a member of this organization.' }); diff --git a/apps/backend/src/trpc/project.routes.ts b/apps/backend/src/trpc/project.routes.ts index e591b2222..d9c9ea840 100644 --- a/apps/backend/src/trpc/project.routes.ts +++ b/apps/backend/src/trpc/project.routes.ts @@ -3,6 +3,7 @@ import { type LlmProvider, MAX_PYTHON_EXECUTION_DURATION_SECS, MIN_PYTHON_EXECUTION_DURATION_SECS, + USER_ROLES, } from '@nao/shared/types'; import { TRPCError } from '@trpc/server'; import { z } from 'zod/v4'; @@ -28,6 +29,7 @@ import { customModelMetadataSchema, llmConfigSchema, llmProviderSchema, modelSet import { isValidIsoDateString } from '../utils/date'; import { getEnvApiKey, getEnvBaseUrls, getEnvProviders, getProjectAvailableModels } from '../utils/llm'; import { extractRequiredEnvVars } from '../utils/nao-config'; +import { toCurrentProjectDto } from '../utils/project-dto'; import { buildCredentialPreviews } from '../utils/utils'; import { adminProtectedProcedure, @@ -56,14 +58,25 @@ export const projectRoutes = { })); }), - getCurrent: protectedProcedure.query(async ({ ctx }) => { - const project = await projectQueries.getProjectByUserId(ctx.user.id, ctx.selectedProjectId); - if (!project) { - return null; - } - const userRole = await projectQueries.getUserRoleInProject(project.id, ctx.user.id); - return { ...project, userRole }; - }), + getCurrent: protectedProcedure + .output( + z + .object({ + id: z.string(), + name: z.string(), + path: z.string().nullable(), + userRole: z.enum(USER_ROLES).nullable(), + }) + .nullable(), + ) + .query(async ({ ctx }) => { + const project = await projectQueries.getProjectByUserId(ctx.user.id, ctx.selectedProjectId); + if (!project) { + return null; + } + const userRole = await projectQueries.getUserRoleInProject(project.id, ctx.user.id); + return toCurrentProjectDto(project, userRole); + }), getDatabaseObjects: projectProtectedProcedure .output( diff --git a/apps/backend/src/utils/password-reset.ts b/apps/backend/src/utils/password-reset.ts new file mode 100644 index 000000000..8d9df336b --- /dev/null +++ b/apps/backend/src/utils/password-reset.ts @@ -0,0 +1,13 @@ +import { TRPCError } from '@trpc/server'; + +const CLOUD_PASSWORD_RESET_MESSAGE = + 'Administrator password resets are unavailable in nao cloud. Use self-service password recovery instead.'; + +export function assertAdminPasswordResetAllowed(isCloud: boolean): void { + if (isCloud) { + throw new TRPCError({ + code: 'FORBIDDEN', + message: CLOUD_PASSWORD_RESET_MESSAGE, + }); + } +} diff --git a/apps/backend/src/utils/project-dto.ts b/apps/backend/src/utils/project-dto.ts new file mode 100644 index 000000000..38e0a7c7e --- /dev/null +++ b/apps/backend/src/utils/project-dto.ts @@ -0,0 +1,22 @@ +import type { UserRole } from '@nao/shared/types'; + +import type { DBProject } from '../db/abstractSchema'; + +export interface CurrentProjectDto { + id: string; + name: string; + path: string | null; + userRole: UserRole | null; +} + +export function toCurrentProjectDto( + project: Pick, + userRole: UserRole | null, +): CurrentProjectDto { + return { + id: project.id, + name: project.name, + path: project.path, + userRole, + }; +} diff --git a/apps/backend/tests/cloud-password-reset.test.ts b/apps/backend/tests/cloud-password-reset.test.ts new file mode 100644 index 000000000..46feb0a7b --- /dev/null +++ b/apps/backend/tests/cloud-password-reset.test.ts @@ -0,0 +1,150 @@ +import { TRPCError } from '@trpc/server'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { assertAdminPasswordResetAllowed } from '../src/utils/password-reset'; + +const routeMocks = vi.hoisted(() => ({ + isCloud: true, + getAccountById: vi.fn(), + getProjectByUserId: vi.fn(), + getUserRoleInProject: vi.fn(), + getUserOrgMembership: vi.fn(), + getUserRoleInOrg: vi.fn(), +})); + +vi.mock('../src/env', () => ({ + env: { DEFAULT_USER_ROLE: 'user' }, + get isCloud() { + return routeMocks.isCloud; + }, +})); + +vi.mock('../src/auth', () => ({ + getAuth: vi.fn(), +})); + +vi.mock('../src/queries/account.queries', () => ({ + getAccountById: routeMocks.getAccountById, +})); + +vi.mock('../src/queries/project.queries', () => ({ + getProjectByUserId: routeMocks.getProjectByUserId, + getUserRoleInProject: routeMocks.getUserRoleInProject, +})); + +vi.mock('../src/queries/organization.queries', () => ({ + getUserOrgMembership: routeMocks.getUserOrgMembership, + getUserRoleInOrg: routeMocks.getUserRoleInOrg, +})); + +vi.mock('../src/queries/user.queries', () => ({})); + +vi.mock('../src/services/email', () => ({ + emailService: { sendEmail: vi.fn() }, +})); + +vi.mock('../src/services/team-member', () => ({ + addTeamMember: vi.fn(), +})); + +describe('admin password reset boundary', () => { + beforeEach(() => { + routeMocks.isCloud = true; + routeMocks.getAccountById.mockReset(); + routeMocks.getProjectByUserId.mockReset(); + routeMocks.getUserRoleInProject.mockReset(); + routeMocks.getUserOrgMembership.mockReset(); + routeMocks.getUserRoleInOrg.mockReset(); + }); + + it('blocks administrator password resets in cloud mode', () => { + expect(() => assertAdminPasswordResetAllowed(true)).toThrowError( + expect.objectContaining({ + code: 'FORBIDDEN', + message: + 'Administrator password resets are unavailable in nao cloud. Use self-service password recovery instead.', + }), + ); + }); + + it('preserves administrator password resets in self-hosted mode', () => { + expect(() => assertAdminPasswordResetAllowed(false)).not.toThrow(); + }); + + it('uses a tRPC error for cloud rejections', () => { + expect(() => assertAdminPasswordResetAllowed(true)).toThrowError(TRPCError); + }); + + it('blocks the project administrator procedure before account access', async () => { + routeMocks.getProjectByUserId.mockResolvedValue({ id: 'project-1' }); + routeMocks.getUserRoleInProject.mockResolvedValue('admin'); + const caller = await createAccountCaller(); + + await expect(caller.resetPassword({ userId: 'target-user' })).rejects.toMatchObject({ + code: 'FORBIDDEN', + }); + expect(routeMocks.getAccountById).not.toHaveBeenCalled(); + }); + + it('blocks the organization administrator procedure before account access', async () => { + routeMocks.getUserOrgMembership.mockResolvedValue({ + organization: { id: 'org-1', name: 'Finance' }, + role: 'admin', + }); + const caller = await createOrganizationCaller(); + + await expect(caller.resetMemberPassword({ userId: 'target-user' })).rejects.toMatchObject({ + code: 'FORBIDDEN', + }); + expect(routeMocks.getUserRoleInOrg).not.toHaveBeenCalled(); + expect(routeMocks.getAccountById).not.toHaveBeenCalled(); + }); + + it('keeps both procedures available to self-hosted administrators', async () => { + routeMocks.isCloud = false; + routeMocks.getProjectByUserId.mockResolvedValue({ id: 'project-1' }); + routeMocks.getUserRoleInProject.mockResolvedValue('admin'); + routeMocks.getUserOrgMembership.mockResolvedValue({ + organization: { id: 'org-1', name: 'Finance' }, + role: 'admin', + }); + routeMocks.getUserRoleInOrg.mockResolvedValue('user'); + routeMocks.getAccountById.mockResolvedValue(null); + + const accountCaller = await createAccountCaller(); + const organizationCaller = await createOrganizationCaller(); + + await expect(accountCaller.resetPassword({ userId: 'target-user' })).rejects.toMatchObject({ + code: 'NOT_FOUND', + }); + await expect(organizationCaller.resetMemberPassword({ userId: 'target-user' })).rejects.toMatchObject({ + code: 'NOT_FOUND', + }); + expect(routeMocks.getAccountById).toHaveBeenCalledTimes(2); + }); +}); + +async function createAccountCaller() { + const [{ accountRoutes }, { router }] = await Promise.all([ + import('../src/trpc/account.routes'), + import('../src/trpc/trpc'), + ]); + return router(accountRoutes).createCaller(createCallerContext()); +} + +async function createOrganizationCaller() { + const [{ organizationRoutes }, { router }] = await Promise.all([ + import('../src/trpc/organization.routes'), + import('../src/trpc/trpc'), + ]); + return router(organizationRoutes).createCaller(createCallerContext()); +} + +function createCallerContext() { + return { + session: { + user: { id: 'admin-user' }, + }, + selectedProjectId: 'project-1', + } as never; +} diff --git a/apps/backend/tests/context-explorer.service.test.ts b/apps/backend/tests/context-explorer.service.test.ts new file mode 100644 index 000000000..93d889ecc --- /dev/null +++ b/apps/backend/tests/context-explorer.service.test.ts @@ -0,0 +1,56 @@ +import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'fs'; +import { tmpdir } from 'os'; +import { join } from 'path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { readFileContent } from '../src/services/context-explorer.service'; + +describe('readFileContent', () => { + let projectFolder: string; + let testFolder: string; + + beforeEach(() => { + testFolder = mkdtempSync(join(tmpdir(), 'nao-context-explorer-')); + projectFolder = join(testFolder, 'project'); + mkdirSync(projectFolder); + }); + + afterEach(() => { + rmSync(testFolder, { recursive: true, force: true }); + }); + + it('reads a file using its virtual absolute path', async () => { + writeFileSync(join(projectFolder, 'context.md'), 'project context', 'utf-8'); + + await expect(readFileContent('/context.md', projectFolder)).resolves.toBe('project context'); + }); + + it('rejects a lexical parent-directory escape', async () => { + writeFileSync(join(testFolder, 'secret.txt'), 'outside', 'utf-8'); + + await expect(readFileContent('../secret.txt', projectFolder)).rejects.toThrow( + 'Access denied: path is outside the project folder', + ); + }); + + it('rejects a final symlink that resolves outside the project folder', async () => { + const secretPath = join(testFolder, 'secret.txt'); + writeFileSync(secretPath, 'outside', 'utf-8'); + symlinkSync(secretPath, join(projectFolder, 'secret.txt')); + + await expect(readFileContent('/secret.txt', projectFolder)).rejects.toThrow( + 'Access denied: path is outside the project folder', + ); + }); + + it('rejects a nested-directory symlink that resolves outside the project folder', async () => { + const outsideFolder = join(testFolder, 'outside'); + mkdirSync(outsideFolder); + writeFileSync(join(outsideFolder, 'secret.txt'), 'outside', 'utf-8'); + symlinkSync(outsideFolder, join(projectFolder, 'linked-directory')); + + await expect(readFileContent('/linked-directory/secret.txt', projectFolder)).rejects.toThrow( + 'Access denied: path is outside the project folder', + ); + }); +}); diff --git a/apps/backend/tests/execute-sql.test.ts b/apps/backend/tests/execute-sql.test.ts new file mode 100644 index 000000000..25d5e82a6 --- /dev/null +++ b/apps/backend/tests/execute-sql.test.ts @@ -0,0 +1,74 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { executeQuery } from '../src/agents/tools/execute-sql'; +import { env } from '../src/env'; +import type { ToolContext } from '../src/types/tools'; + +vi.mock('../src/agents/tools/query-app-db', () => ({ + queryAppDb: vi.fn(), +})); + +const fetchMock = vi.fn(); + +function createContext(dangerouslyWritePermEnabled = false): ToolContext { + return { + projectFolder: '/tmp/project', + chatId: 'chat-1', + userId: 'user-1', + projectId: 'project-1', + agentSettings: { sql: { dangerouslyWritePermEnabled } }, + envVars: {}, + azureAccessToken: null, + queryResults: new Map(), + generatedArtifacts: { charts: [], stories: [] }, + }; +} + +describe('executeQuery FastAPI request', () => { + beforeEach(() => { + fetchMock.mockResolvedValue( + new Response( + JSON.stringify({ + data: [{ value: 1 }], + row_count: 1, + columns: ['value'], + dialect: 'duckdb', + }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ), + ); + vi.stubGlobal('fetch', fetchMock); + }); + + afterEach(() => { + fetchMock.mockReset(); + vi.unstubAllGlobals(); + }); + + it('authenticates read-only sidecar requests without write permission', async () => { + await executeQuery({ sql_query: 'SELECT 1 AS value' }, createContext()); + + const [, request] = fetchMock.mock.calls[0]; + expect(request?.headers).toEqual({ + 'Content-Type': 'application/json', + 'X-Nao-Internal-Secret': env.BETTER_AUTH_SECRET, + }); + expect(JSON.parse(request?.body as string)).toMatchObject({ + sql: 'SELECT 1 AS value', + nao_project_folder: '/tmp/project', + dangerously_write_permission_enabled: false, + }); + }); + + it('conveys enabled dangerous write permission to the authenticated sidecar', async () => { + await executeQuery({ sql_query: 'DELETE FROM users' }, createContext(true)); + + const [, request] = fetchMock.mock.calls[0]; + expect(request?.headers).toMatchObject({ + 'X-Nao-Internal-Secret': env.BETTER_AUTH_SECRET, + }); + expect(JSON.parse(request?.body as string)).toMatchObject({ + dangerously_write_permission_enabled: true, + }); + }); +}); diff --git a/apps/backend/tests/project-dto.test.ts b/apps/backend/tests/project-dto.test.ts new file mode 100644 index 000000000..201754830 --- /dev/null +++ b/apps/backend/tests/project-dto.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from 'vitest'; + +import { toCurrentProjectDto } from '../src/utils/project-dto'; + +describe('current project DTO', () => { + it('returns only fields used by frontend callers', () => { + const project = { + id: 'project-1', + name: 'Finance', + path: '/srv/nao/finance', + envVars: { DATABASE_PASSWORD: 'secret' }, + slackSettings: { botToken: 'secret' }, + }; + + expect(toCurrentProjectDto(project, 'viewer')).toEqual({ + id: 'project-1', + name: 'Finance', + path: '/srv/nao/finance', + userRole: 'viewer', + }); + }); +}); diff --git a/apps/frontend/src/routes/_sidebar-layout.settings.organization.tsx b/apps/frontend/src/routes/_sidebar-layout.settings.organization.tsx index 2fb8c8833..a235777aa 100644 --- a/apps/frontend/src/routes/_sidebar-layout.settings.organization.tsx +++ b/apps/frontend/src/routes/_sidebar-layout.settings.organization.tsx @@ -190,9 +190,11 @@ function OrganizationPage() { isAdmin={isOrgAdmin} onEdit={setEditMember} onRemove={setRemoveMember} - extraActions={(member) => ( - setResetPasswordMember(member)} /> - )} + extraActions={ + isCloud + ? undefined + : (member) => setResetPasswordMember(member)} /> + } /> )} diff --git a/apps/frontend/src/routes/_sidebar-layout.settings.project.team.tsx b/apps/frontend/src/routes/_sidebar-layout.settings.project.team.tsx index a5326886d..b82442625 100644 --- a/apps/frontend/src/routes/_sidebar-layout.settings.project.team.tsx +++ b/apps/frontend/src/routes/_sidebar-layout.settings.project.team.tsx @@ -29,7 +29,9 @@ function ProjectTeamTabPage() { const { data: session } = useSession(); const queryClient = useQueryClient(); const usersWithRoles = useQuery(trpc.project.listAllUsersWithRoles.queryOptions()); + const systemConfig = useQuery(trpc.system.getPublicConfig.queryOptions()); const { isAdmin } = usePermissions(); + const isCloud = systemConfig.data?.naoMode === 'cloud'; const [isAddOpen, setIsAddOpen] = useState(false); const [editMember, setEditMember] = useState(null); @@ -122,9 +124,11 @@ function ProjectTeamTabPage() { isAdmin={isAdmin} onEdit={setEditMember} onRemove={setRemoveMember} - extraActions={(member) => ( - setResetPasswordMember(member)} /> - )} + extraActions={ + isCloud + ? undefined + : (member) => setResetPasswordMember(member)} /> + } /> )}