From 3bda4002fe0224707567fe3ad1599feed55f52b5 Mon Sep 17 00:00:00 2001 From: Daniel Dolezal Date: Wed, 24 Jun 2026 11:41:18 +0200 Subject: [PATCH] fix: allow vault info under path scope and clarify API errors - Allow vault and vaults commands when a token has path restrictions - Treat vault/vaults as pathless metadata commands in policy checks - Add allowed commands, vaults and paths to 403 permission errors - Surface real JWT failure reason in 401 instead of generic Unauthorized - Report missing, malformed and empty Bearer tokens distinctly - Catch httpRequest failures in the n8n node to avoid circular JSON - Map API errors to clean NodeApiError with status code and detail - Support continueOnFail with per-item error output in the n8n node - Push a versioned image tag from pyproject in the CI workflow - Bump server to 1.0.2 and n8n node package to 0.2.1 - Add tests for vault info access and richer auth error messages --- .gitea/workflows/prod-docker-images.yml | 7 ++ app/auth.py | 13 +++- app/policy.py | 27 +++++-- .../nodes/ObsidianApi/ObsidianApi.node.ts | 77 +++++++++++-------- .../n8n-nodes-obsidian-api/package-lock.json | 4 +- packages/n8n-nodes-obsidian-api/package.json | 2 +- .../shared/ObsidianApiClient.ts | 65 +++++++++++++++- pyproject.toml | 2 +- tests/test_api.py | 40 +++++++++- 9 files changed, 184 insertions(+), 53 deletions(-) diff --git a/.gitea/workflows/prod-docker-images.yml b/.gitea/workflows/prod-docker-images.yml index 7df8205..e9607cc 100644 --- a/.gitea/workflows/prod-docker-images.yml +++ b/.gitea/workflows/prod-docker-images.yml @@ -27,6 +27,12 @@ jobs: token: "${{ secrets.ACTION_ACCESS_TOKEN }}" submodules: recursive + - name: Read app version + id: version + run: | + VERSION=$(grep -m1 '^version' pyproject.toml | sed -E 's/.*"(.*)".*/\1/') + echo "version=${VERSION}" >> "$GITHUB_OUTPUT" + - name: Set up Docker Buildx uses: docker/setup-buildx-action@v4 @@ -48,3 +54,4 @@ jobs: OBSIDIAN_VERSION=${{ env.OBSIDIAN_VERSION }} tags: | ${{ vars.DOCKER_REGISTRY_URL }}/docker/${{ env.IMAGE_NAME }}:latest + ${{ vars.DOCKER_REGISTRY_URL }}/docker/${{ env.IMAGE_NAME }}:${{ steps.version.outputs.version }} diff --git a/app/auth.py b/app/auth.py index 01ae2d4..c386826 100644 --- a/app/auth.py +++ b/app/auth.py @@ -73,12 +73,17 @@ def require_auth(request:Request) -> AuthContext|JSONResponse: return error(500, "OBSIDIAN_JWT_SECRET is required") header = request.headers.get("authorization", "") + if not header: + return error(401, "missing Authorization header; expected 'Bearer '") if not header.startswith("Bearer "): - return error(401, "Unauthorized") + return error(401, "invalid Authorization header; expected 'Bearer '") + + supplied = header.removeprefix("Bearer ").strip() + if not supplied: + return error(401, "empty Bearer token") - supplied = header.removeprefix("Bearer ") try: claims = decode_hs256_jwt(supplied, JWT_SECRET.encode()) return AuthContext(policy=policy_from_jwt_claims(supplied, claims)) - except ValueError: - return error(401, "Unauthorized") + except ValueError as exc: + return error(401, f"invalid token: {exc}") diff --git a/app/policy.py b/app/policy.py index 10b4824..78531f7 100644 --- a/app/policy.py +++ b/app/policy.py @@ -36,7 +36,7 @@ PATH_REQUIRED_WHEN_RESTRICTED = { "property:set", "property:remove", "property:read", "properties", "aliases", "backlinks", "links", "outlinks", "tags", "tasks", "task", } -PATHLESS_ALLOWED_WHEN_RESTRICTED = {"help", "version"} +PATHLESS_ALLOWED_WHEN_RESTRICTED = {"help", "version", "vault", "vaults"} DAILY_NOTE_COMMANDS = {"daily:path", "daily:read", "daily:append", "daily:prepend"} def tuple_claim(value:Any, default:tuple[str, ...]) -> tuple[str, ...]: @@ -178,12 +178,18 @@ def authorize_command(policy:TokenPolicy|None, args:list[str], cwd:Path) -> None command = target.command or "" if not policy.allows_all_commands() and command not in policy.commands: - raise PermissionError(f"token is not allowed to run command: {command}") + allowed = ", ".join(sorted(policy.commands)) or "(none)" + raise PermissionError( + f"token is not allowed to run command: {command or '(empty)'}. allowed commands: {allowed}" + ) if not policy.allows_all_vaults(): vault = target.vault or vault_for_path(cwd) or DEFAULT_VAULT_NAME if vault not in policy.vaults: - raise PermissionError(f"token is not allowed to access vault: {vault}") + allowed = ", ".join(sorted(policy.vaults)) or "(none)" + raise PermissionError( + f"token is not allowed to access vault: {vault}. allowed vaults: {allowed}" + ) if policy.allows_all_paths(): return @@ -192,13 +198,22 @@ def authorize_command(policy:TokenPolicy|None, args:list[str], cwd:Path) -> None if not paths and command in DAILY_NOTE_COMMANDS: paths.append(daily_note_path_for_vault(vault_root_for_target(target, cwd))) + allowed_paths = ", ".join(policy.paths) or "(none)" if not paths: if command in PATHLESS_ALLOWED_WHEN_RESTRICTED: return if command in PATH_REQUIRED_WHEN_RESTRICTED: - raise PermissionError("token has path restrictions; command must include an allowed path/file/folder/name argument") - raise PermissionError(f"token has path restrictions; command is not allowed without an explicit path: {command}") + raise PermissionError( + f"token has path restrictions; command '{command}' must include an allowed " + f"path/file/folder/name argument. allowed paths: {allowed_paths}" + ) + raise PermissionError( + f"token has path restrictions; command '{command}' is not allowed without an " + f"explicit path. allowed paths: {allowed_paths}" + ) denied = [path for path in paths if not is_path_allowed(path, policy.paths)] if denied: - raise PermissionError(f"token is not allowed to access path: {denied[0]}") + raise PermissionError( + f"token is not allowed to access path: {denied[0]}. allowed paths: {allowed_paths}" + ) diff --git a/packages/n8n-nodes-obsidian-api/nodes/ObsidianApi/ObsidianApi.node.ts b/packages/n8n-nodes-obsidian-api/nodes/ObsidianApi/ObsidianApi.node.ts index 0ec6fe1..95337d7 100644 --- a/packages/n8n-nodes-obsidian-api/nodes/ObsidianApi/ObsidianApi.node.ts +++ b/packages/n8n-nodes-obsidian-api/nodes/ObsidianApi/ObsidianApi.node.ts @@ -225,41 +225,50 @@ export class ObsidianApi implements INodeType { const returnData: INodeExecutionData[] = []; for (let itemIndex = 0; itemIndex < items.length; itemIndex++) { - const resource = this.getNodeParameter('resource', itemIndex) as string; - const operation = this.getNodeParameter('operation', itemIndex) as string; - const additionalFields = this.getNodeParameter('additionalFields', itemIndex, {}) as Record; - const vault = String(additionalFields.vault || ''); - const timeout = Number(additionalFields.timeout ?? 60); + try { + const resource = this.getNodeParameter('resource', itemIndex) as string; + const operation = this.getNodeParameter('operation', itemIndex) as string; + const additionalFields = this.getNodeParameter('additionalFields', itemIndex, {}) as Record; + const vault = String(additionalFields.vault || ''); + const timeout = Number(additionalFields.timeout ?? 60); - let args: string[]; + let args: string[]; - switch (resource) { - case 'note': { - args = buildNoteArgs(this, operation, itemIndex); - break; + switch (resource) { + case 'note': { + args = buildNoteArgs(this, operation, itemIndex); + break; + } + case 'daily': { + args = buildDailyArgs(this, operation, itemIndex); + break; + } + case 'search': { + args = buildSearchArgs(this, operation, itemIndex); + break; + } + case 'vault': { + args = buildVaultArgs(this, operation, itemIndex, additionalFields); + break; + } + case 'raw': { + args = parseRawArgs(this, itemIndex); + break; + } + default: + throw new NodeOperationError(this.getNode(), `Unsupported resource: ${resource}`, { itemIndex }); } - case 'daily': { - args = buildDailyArgs(this, operation, itemIndex); - break; + + const result = await executeObsidianCommand(this, itemIndex, args, { vault, timeout }); + returnData.push({ json: result, pairedItem: { item: itemIndex } }); + } catch (error) { + if (this.continueOnFail()) { + const message = error instanceof Error ? error.message : String(error); + returnData.push({ json: { error: message }, pairedItem: { item: itemIndex } }); + continue; } - case 'search': { - args = buildSearchArgs(this, operation, itemIndex); - break; - } - case 'vault': { - args = buildVaultArgs(this, operation, itemIndex, additionalFields); - break; - } - case 'raw': { - args = parseRawArgs(this, itemIndex); - break; - } - default: - throw new NodeOperationError(this.getNode(), `Unsupported resource: ${resource}`, { itemIndex }); + throw error; } - - const result = await executeObsidianCommand(this, itemIndex, args, { vault, timeout }); - returnData.push({ json: result, pairedItem: { item: itemIndex } }); } return [returnData]; @@ -285,7 +294,7 @@ function buildNoteArgs(context: IExecuteFunctions, operation: string, itemIndex: return compactArgs(['files', kv('folder', context.getNodeParameter('folder', itemIndex, '') as string)]); default: throw new NodeOperationError(context.getNode(), `Unsupported note operation: ${operation}`, { itemIndex }); -} + } } function buildDailyArgs(context: IExecuteFunctions, operation: string, itemIndex: number): string[] { @@ -300,7 +309,7 @@ function buildDailyArgs(context: IExecuteFunctions, operation: string, itemIndex return ['daily:path']; default: throw new NodeOperationError(context.getNode(), `Unsupported daily operation: ${operation}`, { itemIndex }); -} + } } function buildSearchArgs(context: IExecuteFunctions, operation: string, itemIndex: number): string[] { @@ -322,7 +331,7 @@ function buildVaultArgs(context: IExecuteFunctions, operation: string, itemIndex } default: throw new NodeOperationError(context.getNode(), `Unsupported vault operation: ${operation}`, { itemIndex }); -} + } } function parseRawArgs(context: IExecuteFunctions, itemIndex: number): string[] { @@ -330,7 +339,7 @@ function parseRawArgs(context: IExecuteFunctions, itemIndex: number): string[] { const parsed = Array.isArray(raw) ? raw : JSON.parse(raw); if (!Array.isArray(parsed) || !parsed.every((item) => typeof item === 'string')) { throw new NodeOperationError(context.getNode(), 'Args JSON must be an array of strings', { itemIndex }); -} + } return parsed; } diff --git a/packages/n8n-nodes-obsidian-api/package-lock.json b/packages/n8n-nodes-obsidian-api/package-lock.json index 6842d57..b0b1a61 100644 --- a/packages/n8n-nodes-obsidian-api/package-lock.json +++ b/packages/n8n-nodes-obsidian-api/package-lock.json @@ -1,12 +1,12 @@ { "name": "n8n-nodes-obsidian-cli-api", - "version": "0.2.0", + "version": "0.2.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "n8n-nodes-obsidian-cli-api", - "version": "0.2.0", + "version": "0.2.1", "license": "MIT", "devDependencies": { "@types/node": "^24.0.0", diff --git a/packages/n8n-nodes-obsidian-api/package.json b/packages/n8n-nodes-obsidian-api/package.json index 778c77e..1681f98 100644 --- a/packages/n8n-nodes-obsidian-api/package.json +++ b/packages/n8n-nodes-obsidian-api/package.json @@ -1,6 +1,6 @@ { "name": "n8n-nodes-obsidian-cli-api", - "version": "0.2.0", + "version": "0.2.1", "description": "n8n community nodes for the Obsidian API Docker service backed by the official Obsidian CLI.", "license": "MIT", "homepage": "https://git.yiprawr.dev/Docker/obsidian-api", diff --git a/packages/n8n-nodes-obsidian-api/shared/ObsidianApiClient.ts b/packages/n8n-nodes-obsidian-api/shared/ObsidianApiClient.ts index 5a37672..c129994 100644 --- a/packages/n8n-nodes-obsidian-api/shared/ObsidianApiClient.ts +++ b/packages/n8n-nodes-obsidian-api/shared/ObsidianApiClient.ts @@ -3,7 +3,7 @@ import type { IDataObject, IHttpRequestOptions, } from 'n8n-workflow'; -import { NodeOperationError } from 'n8n-workflow'; +import { NodeApiError, NodeOperationError } from 'n8n-workflow'; export interface ObsidianCommandResult extends IDataObject { id: string; @@ -64,7 +64,12 @@ export async function executeObsidianCommand( returnFullResponse: false, }; - const response = await context.helpers.httpRequest(request) as unknown; + let response: unknown; + try { + response = await context.helpers.httpRequest(request) as unknown; + } catch (error) { + throw toCleanApiError(context, error, itemIndex); + } const result = normalizeCommandResult(response); if (result.exit_code !== 0) { @@ -78,6 +83,62 @@ export async function executeObsidianCommand( return result; } +function toCleanApiError( + context: IExecuteFunctions, + error: unknown, + itemIndex: number, +): Error { + const err = isObject(error) ? error : {}; + const statusCode = Number( + err.statusCode ?? err.status ?? (isObject(err.response) ? err.response.statusCode : undefined) ?? 0, + ); + const body = extractErrorBody(err); + const detail = extractErrorMessage(body) || (typeof err.message === 'string' ? err.message : ''); + + const statusText = statusCode ? `HTTP ${statusCode}` : 'Request failed'; + const message = detail + ? `Obsidian API error (${statusText}): ${detail}` + : `Obsidian API error (${statusText})`; + + // Build a plain-object error so n8n never tries to serialize the raw HTTP + // response (it contains circular fields like res.socket._httpMessage). + return new NodeApiError( + context.getNode(), + { message, ...(statusCode ? { statusCode } : {}), ...(detail ? { detail } : {}) }, + { message, description: detail || undefined, httpCode: statusCode ? String(statusCode) : undefined, itemIndex }, + ); +} + +function extractErrorBody(err: Record): unknown { + if ('body' in err && err.body !== undefined) return err.body; + const response = err.response; + if (isObject(response)) { + if ('body' in response && response.body !== undefined) return response.body; + if ('data' in response && response.data !== undefined) return response.data; + } + if ('error' in err && err.error !== undefined) return err.error; + return undefined; +} + +function extractErrorMessage(body: unknown): string { + let data: unknown = body; + if (typeof data === 'string') { + const trimmed = data.trim(); + if (!trimmed) return ''; + try { + data = JSON.parse(trimmed); + } catch { + return trimmed; + } + } + if (isObject(data)) { + const candidate = data.detail ?? data.message ?? data.error; + if (typeof candidate === 'string' && candidate) return candidate; + if (isObject(candidate) && typeof candidate.message === 'string') return candidate.message; + } + return ''; +} + function normalizeCommandResult(response: unknown): ObsidianCommandResult { const data = extractResponseBody(response); const result = typeof data === 'string' ? JSON.parse(data) : data; diff --git a/pyproject.toml b/pyproject.toml index 6743ea6..71da8a7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "obsidian-api" -version = "1.0.0" +version = "1.0.2" description = "HTTP API wrapper for running Obsidian CLI commands from n8n, SDKs, or agent harnesses." requires-python = ">=3.14" dependencies = [ diff --git a/tests/test_api.py b/tests/test_api.py index fdc8223..7257170 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -50,6 +50,19 @@ def test_command_requires_auth(): response = client.post("/commands", json={"args": ["--version"]}) assert response.status_code == 401 + assert response.json()["detail"] == "missing Authorization header; expected 'Bearer '" + +def test_invalid_token_reports_reason(monkeypatch): + monkeypatch.setattr("app.auth.JWT_SECRET", "secret") + + response = client.post( + "/commands", + headers={"Authorization": "Bearer not-a-jwt"}, + json={"args": ["--version"]}, + ) + + assert response.status_code == 401 + assert response.json()["detail"].startswith("invalid token:") def test_command_runs_configured_cli_only(): response = client.post( @@ -118,7 +131,7 @@ def test_token_policy_restricts_command(monkeypatch): ) assert response.status_code == 403 - assert response.json()["detail"] == "token is not allowed to run command: create" + assert response.json()["detail"].startswith("token is not allowed to run command: create") def test_token_policy_restricts_vault(monkeypatch): token = use_jwt_auth(monkeypatch, {"sub": "work", "commands": ["*"], "vaults": ["work"], "paths": ["*"]}) @@ -130,7 +143,7 @@ def test_token_policy_restricts_vault(monkeypatch): ) assert response.status_code == 403 - assert response.json()["detail"] == "token is not allowed to access vault: personal" + assert response.json()["detail"].startswith("token is not allowed to access vault: personal") def test_token_policy_restricts_path(monkeypatch): token = use_jwt_auth(monkeypatch, {"sub": "inbox", "commands": ["read"], "vaults": ["*"], "paths": ["Inbox/"]}) @@ -142,7 +155,7 @@ def test_token_policy_restricts_path(monkeypatch): ) assert response.status_code == 403 - assert response.json()["detail"] == "token is not allowed to access path: Private/Secret.md" + assert response.json()["detail"].startswith("token is not allowed to access path: Private/Secret.md") def test_token_policy_allows_allowed_path(monkeypatch): token = use_jwt_auth(monkeypatch, {"sub": "inbox", "commands": ["read"], "vaults": ["*"], "paths": ["Inbox/"]}) @@ -185,6 +198,27 @@ def test_path_restricted_token_allows_daily_command_from_obsidian_config(monkeyp assert response.status_code == 200 assert response.json()["command"] == ["python", "vault=work", "daily:append", "content=ok"] +def test_path_restricted_token_allows_vault_info_command(monkeypatch): + token = use_jwt_auth(monkeypatch, {"sub": "daily", "commands": ["daily:append", "vault", "vaults"], "vaults": ["*"], "paths": ["00. Journal/"]}) + + response = client.post( + "/commands", + headers={"Authorization": f"Bearer {token}"}, + json={"args": ["vault"]}, + ) + + assert response.status_code == 200 + assert response.json()["command"] == ["python", "vault"] + + response = client.post( + "/commands", + headers={"Authorization": f"Bearer {token}"}, + json={"args": ["vaults"]}, + ) + + assert response.status_code == 200 + assert response.json()["command"] == ["python", "vaults"] + def test_path_restricted_token_denies_daily_command_outside_obsidian_config(monkeypatch, tmp_path): vault = tmp_path / "vault" config = vault / ".obsidian"