resolve secret references pasted with surrounding quotes
This commit is contained in:
7 files changed
+129
-7
No files matched your search
@@ -13,7 +13,7 @@
|
||||
|
||||
import { request } from 'undici';
|
||||
import type { AzureKvConfig, SecretProvider, TestConnectionResult } from './shared';
|
||||
import { assertSafeProviderHost, isJsonResponse } from './shared';
|
||||
import { assertSafeProviderHost, isJsonResponse, stripSurroundingQuotes } from './shared';
|
||||
|
||||
const API_VERSION = '7.4';
|
||||
const AZURE_REF_PREFIX = 'azurekv://';
|
||||
@@ -140,7 +140,7 @@ export const azureKvProvider: SecretProvider<AzureKvConfig> = {
|
||||
supportsBulk: true,
|
||||
|
||||
isReference(value: unknown): value is string {
|
||||
return typeof value === 'string' && AZURE_REF_RE.test(value.trim());
|
||||
return typeof value === 'string' && AZURE_REF_RE.test(stripSurroundingQuotes(value));
|
||||
},
|
||||
|
||||
async testConnection(config: AzureKvConfig): Promise<TestConnectionResult> {
|
||||
|
||||
@@ -15,7 +15,7 @@
|
||||
|
||||
import { request } from 'undici';
|
||||
import type { ConnectConfig, SecretProvider, TestConnectionResult } from './shared';
|
||||
import { assertSafeProviderHost, parseProviderError, isJsonResponse } from './shared';
|
||||
import { assertSafeProviderHost, parseProviderError, isJsonResponse, stripSurroundingQuotes } from './shared';
|
||||
import { UnsupportedOperationError } from './shared';
|
||||
|
||||
/** A vault as returned by GET /v1/vaults. */
|
||||
@@ -228,7 +228,7 @@ export const connectProvider: SecretProvider<ConnectConfig> = {
|
||||
supportsBulk: false,
|
||||
|
||||
isReference(value: unknown): value is string {
|
||||
return typeof value === 'string' && value.trim().startsWith('op://');
|
||||
return typeof value === 'string' && stripSurroundingQuotes(value).startsWith('op://');
|
||||
},
|
||||
|
||||
async testConnection(config: ConnectConfig): Promise<TestConnectionResult> {
|
||||
|
||||
@@ -20,6 +20,7 @@
|
||||
import { spawn } from 'node:child_process';
|
||||
import { isAbsolute } from 'node:path';
|
||||
import type { KeePassConfig, SecretProvider, TestConnectionResult } from './shared';
|
||||
import { stripSurroundingQuotes } from './shared';
|
||||
|
||||
const DEFAULT_CLI_PATH = '/usr/bin/keepassxc-cli';
|
||||
const COMMAND_TIMEOUT_MS = 30_000;
|
||||
@@ -260,7 +261,7 @@ export const keepassProvider: SecretProvider<KeePassConfig> = {
|
||||
supportsBulk: true,
|
||||
|
||||
isReference(value: unknown): value is string {
|
||||
return typeof value === 'string' && value.trim().startsWith(KEEPASS_PREFIX);
|
||||
return typeof value === 'string' && stripSurroundingQuotes(value).startsWith(KEEPASS_PREFIX);
|
||||
},
|
||||
|
||||
async testConnection(config: KeePassConfig): Promise<TestConnectionResult> {
|
||||
|
||||
@@ -28,6 +28,7 @@ import { chmod, mkdtemp, rm } from 'node:fs/promises';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { isAbsolute, join } from 'node:path';
|
||||
import type { ProtonConfig, SecretProvider, TestConnectionResult } from './shared';
|
||||
import { stripSurroundingQuotes } from './shared';
|
||||
|
||||
const DEFAULT_PASS_CLI_PATH = '/usr/local/bin/pass-cli';
|
||||
const LOGIN_TIMEOUT_MS = 30_000;
|
||||
@@ -368,7 +369,7 @@ export const protonProvider: SecretProvider<ProtonConfig> = {
|
||||
supportsBulk: true,
|
||||
|
||||
isReference(value: unknown): value is string {
|
||||
return typeof value === 'string' && value.trim().startsWith('pass://');
|
||||
return typeof value === 'string' && stripSurroundingQuotes(value).startsWith('pass://');
|
||||
},
|
||||
|
||||
async testConnection(config: ProtonConfig): Promise<TestConnectionResult> {
|
||||
|
||||
@@ -15,6 +15,7 @@
|
||||
|
||||
import { createClient } from '@1password/sdk';
|
||||
import type { SecretProvider, ServiceAccountConfig, TestConnectionResult } from './shared';
|
||||
import { stripSurroundingQuotes } from './shared';
|
||||
|
||||
const INTEGRATION_NAME = 'Dockhand';
|
||||
const INTEGRATION_VERSION = '1.0.0';
|
||||
@@ -49,7 +50,7 @@ export const serviceAccountProvider: SecretProvider<ServiceAccountConfig> = {
|
||||
supportsBulk: true,
|
||||
|
||||
isReference(value: unknown): value is string {
|
||||
return typeof value === 'string' && value.trim().startsWith('op://');
|
||||
return typeof value === 'string' && stripSurroundingQuotes(value).startsWith('op://');
|
||||
},
|
||||
|
||||
async testConnection({ token }: ServiceAccountConfig): Promise<TestConnectionResult> {
|
||||
|
||||
@@ -48,6 +48,20 @@ export type SecretProviderType =
|
||||
* self-hosted Vault/Infisical/Connect on the local network still works. Throws on
|
||||
* an unsafe host; call it before the first request in every REST provider.
|
||||
*/
|
||||
/**
|
||||
* Strip ONE layer of matching surrounding quotes for reference DETECTION only.
|
||||
* 1Password's "Copy Secret Reference" puts `"op://Vault/Item/field"` on the clipboard
|
||||
* (quotes included), so a pasted value fails a bare `startsWith('op://')` test and the
|
||||
* reference is silently skipped (#1521). Providers normalize with this before the prefix
|
||||
* test, and the resolver uses it to key the reference - the STORED value is left untouched
|
||||
* (so #1086's "stop stripping quotes on save" still holds). Only strips when both ends are
|
||||
* the same quote char; leaves a value with mismatched/one-sided quotes as-is.
|
||||
*/
|
||||
export function stripSurroundingQuotes(value: string): string {
|
||||
// \1 backreference: strip only when the SAME quote char wraps both ends.
|
||||
return value.trim().replace(/^(["'])(.*)\1$/s, '$2');
|
||||
}
|
||||
|
||||
export function assertSafeProviderHost(rawUrl: string, label: string): void {
|
||||
const safe = isSafeNotificationUrl(rawUrl);
|
||||
if (!safe.ok) {
|
||||
|
||||
@@ -0,0 +1,105 @@
|
||||
/**
|
||||
* #1521: op:// / pass:// references pasted with surrounding quotes (1Password's
|
||||
* "Copy Secret Reference" clipboard format) must still be detected as references.
|
||||
* stripSurroundingQuotes normalizes for the DETECTION/lookup test only - the stored
|
||||
* value is left untouched (keeps #1086's "don't strip quotes on save").
|
||||
*/
|
||||
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { stripSurroundingQuotes } from '../src/lib/server/secretproviders/shared';
|
||||
// NOTE: do NOT import service-account.ts here - it loads @1password/sdk (a native
|
||||
// module) at import time, which the unit-test env cannot resolve. Its isReference is
|
||||
// the identical `stripSurroundingQuotes(value).startsWith('op://')` as connectProvider,
|
||||
// which IS tested below; the pure helper carries the real quote logic.
|
||||
import { connectProvider } from '../src/lib/server/secretproviders/connect';
|
||||
import { protonProvider } from '../src/lib/server/secretproviders/proton';
|
||||
import { keepassProvider } from '../src/lib/server/secretproviders/keepass';
|
||||
import { azureKvProvider } from '../src/lib/server/secretproviders/azure-kv';
|
||||
|
||||
describe('stripSurroundingQuotes', () => {
|
||||
test('strips matching double quotes', () => {
|
||||
expect(stripSurroundingQuotes('"op://Vault/Item/field"')).toBe('op://Vault/Item/field');
|
||||
});
|
||||
test('strips matching single quotes', () => {
|
||||
expect(stripSurroundingQuotes("'op://Vault/Item/field'")).toBe('op://Vault/Item/field');
|
||||
});
|
||||
test('trims whitespace before stripping', () => {
|
||||
expect(stripSurroundingQuotes(' "op://v/i/f" ')).toBe('op://v/i/f');
|
||||
});
|
||||
test('leaves an unquoted value unchanged (just trimmed)', () => {
|
||||
expect(stripSurroundingQuotes('op://v/i/f')).toBe('op://v/i/f');
|
||||
});
|
||||
test('leaves mismatched / one-sided quotes unchanged', () => {
|
||||
expect(stripSurroundingQuotes(`"op://v/i/f'`)).toBe(`"op://v/i/f'`);
|
||||
expect(stripSurroundingQuotes('"op://v/i/f')).toBe('"op://v/i/f');
|
||||
});
|
||||
test('does not strip quotes that are not surrounding', () => {
|
||||
expect(stripSurroundingQuotes('no"quote"inside')).toBe('no"quote"inside');
|
||||
});
|
||||
test('empty quoted string collapses to empty', () => {
|
||||
expect(stripSurroundingQuotes('""')).toBe('');
|
||||
});
|
||||
test('a lone quote char is not a pair', () => {
|
||||
expect(stripSurroundingQuotes('"')).toBe('"');
|
||||
});
|
||||
test('strips quotes around a MULTILINE value (pins the /s dotall flag)', () => {
|
||||
// Without the `s` flag, `.` would not match the newline and the quotes would
|
||||
// survive - a multi-line secret (e.g. a PEM) pasted with quotes must still strip.
|
||||
expect(stripSurroundingQuotes('"line1\nline2"')).toBe('line1\nline2');
|
||||
});
|
||||
});
|
||||
|
||||
describe('op:// isReference tolerates surrounding quotes (#1521)', () => {
|
||||
for (const [name, p] of [
|
||||
['connect', connectProvider],
|
||||
] as const) {
|
||||
test(`${name}: detects a bare op:// reference`, () => {
|
||||
expect(p.isReference('op://Vault/Item/field')).toBe(true);
|
||||
});
|
||||
test(`${name}: detects a double-quoted op:// reference`, () => {
|
||||
expect(p.isReference('"op://Vault/Item/field"')).toBe(true);
|
||||
});
|
||||
test(`${name}: detects a single-quoted op:// reference`, () => {
|
||||
expect(p.isReference("'op://Vault/Item/field'")).toBe(true);
|
||||
});
|
||||
test(`${name}: a plain value is not a reference`, () => {
|
||||
expect(p.isReference('just-a-token')).toBe(false);
|
||||
expect(p.isReference('"just-a-token"')).toBe(false);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
describe('proton pass:// isReference tolerates surrounding quotes (#1521)', () => {
|
||||
test('detects bare + quoted pass:// references', () => {
|
||||
expect(protonProvider.isReference('pass://Vault/Item/field')).toBe(true);
|
||||
expect(protonProvider.isReference('"pass://Vault/Item/field"')).toBe(true);
|
||||
expect(protonProvider.isReference("'pass://Vault/Item/field'")).toBe(true);
|
||||
});
|
||||
test('a plain value is not a reference', () => {
|
||||
expect(protonProvider.isReference('"nope"')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('keepass:// isReference tolerates surrounding quotes (#1521)', () => {
|
||||
test('detects bare + quoted keepass:// references', () => {
|
||||
expect(keepassProvider.isReference('keepass://Database/Entry/Password')).toBe(true);
|
||||
expect(keepassProvider.isReference('"keepass://Database/Entry/Password"')).toBe(true);
|
||||
expect(keepassProvider.isReference("'keepass://Database/Entry/Password'")).toBe(true);
|
||||
});
|
||||
test('a plain value is not a reference', () => {
|
||||
expect(keepassProvider.isReference('"nope"')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('azurekv:// isReference tolerates surrounding quotes (#1521)', () => {
|
||||
// azure-kv refs are a single segment: azurekv://<name>. The anchored regex broke
|
||||
// on the leading quote before the fix.
|
||||
test('detects bare + quoted azurekv:// references', () => {
|
||||
expect(azureKvProvider.isReference('azurekv://db-password')).toBe(true);
|
||||
expect(azureKvProvider.isReference('"azurekv://db-password"')).toBe(true);
|
||||
expect(azureKvProvider.isReference("'azurekv://db-password'")).toBe(true);
|
||||
});
|
||||
test('a plain value is not a reference', () => {
|
||||
expect(azureKvProvider.isReference('"nope"')).toBe(false);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user