Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .bumpy/dashlane-locked-vault.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@varlock/dashlane-plugin": minor
---

dashlane() no longer hangs forever on a locked vault: dcli calls run with stdin closed and a timeout (default 30s, configurable via @initDashlane(timeoutMs=...)). New allowMissing option (per item or in @initDashlane) resolves missing vault entries as empty instead of failing
7 changes: 7 additions & 0 deletions packages/plugins/dashlane/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,8 @@ Initialize a Dashlane plugin instance.
- `id?: string` - Instance identifier for multiple instances (defaults to `_default`)
- `autoSync?: boolean` - If `true`, runs `dcli sync` once before the first read (default `false`)
- `lockOnExit?: boolean` - Lock the vault on process exit. Defaults to `true` in headless mode, `false` in interactive mode.
- `allowMissing?: boolean` - If `true`, entries that do not exist in the vault resolve as empty instead of failing (default `false`). Only applies to missing entries: a locked vault or timeout still fails. Can be overridden per item.
- `timeoutMs?: number` - Maximum time in milliseconds to wait for each `dcli` call (default `30000`)

### Resolver functions

Expand All @@ -189,6 +191,9 @@ Fetch a secret from Dashlane via a `dl://` reference.
- `dashlane(dlRef)` - Fetch by `dl://` reference
- `dashlane(instanceId, dlRef)` - Fetch from a specific instance

**Named parameters:**
- `allowMissing?: boolean` - If `true`, resolves as empty when the entry does not exist in the vault, instead of failing. Overrides the instance-level `allowMissing` setting. A locked vault or timeout still fails.

**Returns:** The resolved secret value as a string.

### Data types
Expand Down Expand Up @@ -245,6 +250,8 @@ Install the Dashlane CLI following the [installation docs](https://cli.dashlane.
- Run `dcli sync` to sync and unlock your vault
- If the vault is locked, you may need to enter your master password

The plugin never waits on a locked vault: `dcli` calls run with stdin closed (so interactive prompts fail immediately) and are killed after `timeoutMs` (default 30 seconds) as a backstop. A locked vault always fails the affected items, regardless of `allowMissing` or whether they are optional.

### Stale secrets

If a recently changed secret isn't reflected:
Expand Down
145 changes: 125 additions & 20 deletions packages/plugins/dashlane/src/dashlane-instance.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { spawnSync } from 'node:child_process';
import { spawnAsync, ExecError } from '@env-spec/utils/exec-helpers';
import { spawn, spawnSync } from 'node:child_process';
import { ExecError } from '@env-spec/utils/exec-helpers';

type ErrorCtor = new (msg: string, opts?: { tip?: string }) => Error;

Expand All @@ -9,6 +9,21 @@ const FIX_INSTALL_TIP = [
' https://cli.dashlane.com/installation',
].join('\n');

export const DEFAULT_DCLI_TIMEOUT_MS = 30_000;

/** dcli output that means the requested entry does not exist in the vault */
const DCLI_NOT_FOUND_RE = /not found|does not exist|no matching/i;

/** How long to wait after SIGTERM before hard-killing and giving up on the child */
const KILL_GRACE_MS = 2000;

/** Thrown when a dcli call is killed by our timeout (or an external signal) */
class DcliTimeoutError extends Error {
constructor(detail: string) {
super(`dcli did not complete: ${detail}`);
}
}

export class DashlanePluginInstance {
private serviceDeviceKeys?: string;
private cache = new Map<string, string>();
Expand All @@ -18,16 +33,25 @@ export class DashlanePluginInstance {
private syncPromise?: Promise<void>;
private synced = false;
private lockAfter = false;
private allowMissing = false;
private timeoutMs = DEFAULT_DCLI_TIMEOUT_MS;

constructor(
readonly id: string,
private ResolutionError: ErrorCtor,
) {}

configure(serviceDeviceKeys?: string, opts?: { autoSync?: boolean; lockOnExit?: boolean }) {
configure(serviceDeviceKeys?: string, opts?: {
autoSync?: boolean;
lockOnExit?: boolean;
allowMissing?: boolean;
timeoutMs?: number;
}) {
this.serviceDeviceKeys = serviceDeviceKeys;
if (opts?.autoSync !== undefined) this.autoSync = opts.autoSync;
this.lockAfter = opts?.lockOnExit ?? !!serviceDeviceKeys;
if (opts?.allowMissing !== undefined) this.allowMissing = opts.allowMissing;
if (opts?.timeoutMs !== undefined) this.timeoutMs = opts.timeoutMs;
}

/** @internal telemetry: whether this instance auto-syncs the vault before reads */
Expand All @@ -44,9 +68,65 @@ export class DashlanePluginInstance {
};
}

private get spawnOpts(): { env: Record<string, string> } | undefined {
const env = this.spawnEnv;
return env ? { env } : undefined;
/**
* Run a dcli subcommand with stdin closed and a hard timeout.
*
* On a locked vault, dcli prompts for the master password and waits on stdin.
* With piped stdio and no timeout that call never returns, hanging the whole
* load. Ignoring stdin makes the prompt hit EOF and fail immediately; the
* timeout is a backstop for any other way dcli can stall.
*/
private execDcli(args: Array<string>): Promise<string> {
return new Promise<string>((resolve, reject) => {
const child = spawn('dcli', args, {
...this.spawnEnv && { env: this.spawnEnv },
stdio: ['ignore', 'pipe', 'pipe'],
});
let stdout = '';
let stderr = '';
child.stdout!.on('data', (data) => {
stdout += data.toString();
});
child.stderr!.on('data', (data) => {
stderr += data.toString();
});

let settled = false;
const timers: Array<NodeJS.Timeout> = [];
const settle = (fn: () => void) => {
if (settled) return;
settled = true;
for (const t of timers) clearTimeout(t);
fn();
};

// own deadline rather than spawn's `timeout` option: that option only
// sends the kill signal, so a dcli that ignores SIGTERM would still
// leave this promise pending forever
timers.push(setTimeout(() => {
child.kill('SIGTERM');
timers.push(setTimeout(() => {
// child ignored SIGTERM: hard-kill, release our ends of the pipes,
// and reject without waiting on `exit`
child.kill('SIGKILL');
child.stdout?.destroy();
child.stderr?.destroy();
child.unref();
settle(() => reject(new DcliTimeoutError(`still running ${KILL_GRACE_MS}ms after SIGTERM`)));
}, KILL_GRACE_MS));
}, this.timeoutMs));

child.on('error', (err) => settle(() => reject(err)));
child.on('exit', (exitCode, signal) => settle(() => {
if (exitCode === 0) {
resolve(stdout);
} else if (signal) {
reject(new DcliTimeoutError(`killed by ${signal}`));
} else {
reject(new ExecError(exitCode ?? 1, signal, stderr));
}
}));
});
}

async ensureDcliInstalled(): Promise<void> {
Expand All @@ -57,7 +137,7 @@ export class DashlanePluginInstance {

private async doDcliCheck(): Promise<void> {
try {
await spawnAsync('dcli', ['--version'], this.spawnOpts);
await this.execDcli(['--version']);
this.dcliChecked = true;
} catch (err) {
if ((err as NodeJS.ErrnoException).code === 'ENOENT') {
Expand All @@ -81,7 +161,7 @@ export class DashlanePluginInstance {
private async doSync(): Promise<void> {
await this.ensureDcliInstalled();
try {
await spawnAsync('dcli', ['sync'], this.spawnOpts);
await this.execDcli(['sync']);
} catch {
// Sync failure should not block reads - vault may still have recent data
}
Expand Down Expand Up @@ -109,8 +189,13 @@ export class DashlanePluginInstance {
* Read a secret by dl:// reference.
* Supports both dl://<id>/field (fast, skips vault decryption)
* and dl://<title>/field (slower, requires full vault sync).
*
* With allowMissing (per-call arg, falling back to the instance default),
* a reference that does not exist in the vault resolves to undefined
* instead of throwing. Any other failure (locked vault, timeout, auth)
* still throws.
*/
async readReference(dlUri: string): Promise<string> {
async readReference(dlUri: string, opts?: { allowMissing?: boolean }): Promise<string | undefined> {
if (!dlUri.startsWith('dl://') || dlUri === 'dl://') {
throw new this.ResolutionError(`Invalid Dashlane reference: "${dlUri}"`, {
tip: 'References must start with dl:// and include a path — e.g. dashlane("dl://<id>/password")',
Expand All @@ -125,28 +210,57 @@ export class DashlanePluginInstance {
}

try {
const result = await spawnAsync('dcli', ['read', dlUri], this.spawnOpts);
const result = await this.execDcli(['read', dlUri]);
const value = result.replace(/\n$/, '');
this.cache.set(dlUri, value);
return value;
} catch (err) {
const allowMissing = opts?.allowMissing ?? this.allowMissing;
if (
allowMissing
&& err instanceof ExecError
&& DCLI_NOT_FOUND_RE.test(err.data || err.message)
) {
return undefined;
}
return this.handleDcliError(err, dlUri);
}
}

private throwLockedVaultError(details?: string): never {
throw new this.ResolutionError(
`Dashlane vault appears locked or not synced${details ? ` (${details})` : ''}`,
{
tip: [
'Run `dcli sync` to sync and unlock your vault.',
'Or set autoSync=true in @initDashlane to sync automatically.',
].join('\n'),
},
);
}

private handleDcliError(err: unknown, context: string): never {
if (err instanceof DcliTimeoutError) {
this.throwLockedVaultError(`dcli did not respond within ${this.timeoutMs}ms`);
}

if (err instanceof ExecError) {
const msg = err.data || err.message;

if (msg.match(/not found/i) || msg.match(/does not exist/i) || msg.match(/no matching/i)) {
if (DCLI_NOT_FOUND_RE.test(msg)) {
throw new this.ResolutionError(`Entry "${context}" not found in Dashlane vault`, {
tip: [
'Verify the entry exists: dcli password -o json | jq \'.[].title\'',
'Use the entry ID for reliable lookups: dashlane("dl://<id>/password")',
'Or set allowMissing=true (per item or in @initDashlane) to resolve missing entries as empty',
].join('\n'),
});
}

if (msg.match(/locked/i) || msg.match(/sync/i) || msg.match(/master password/i)) {
this.throwLockedVaultError();
}

if (msg.match(/auth/i) || msg.match(/credential/i) || msg.match(/login/i)) {
throw new this.ResolutionError('Dashlane authentication failed', {
tip: [
Expand All @@ -157,15 +271,6 @@ export class DashlanePluginInstance {
});
}

if (msg.match(/locked/i) || msg.match(/sync/i)) {
throw new this.ResolutionError('Dashlane vault appears locked or not synced', {
tip: [
'Run `dcli sync` to sync your vault.',
'Or set autoSync=true in @initDashlane to sync automatically.',
].join('\n'),
});
}

throw new this.ResolutionError(`Failed to fetch "${context}" from Dashlane: ${msg}`);
}

Expand Down
28 changes: 26 additions & 2 deletions packages/plugins/dashlane/src/dashlane-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,32 @@ export class DashlaneManager {
? (objArgs.lockOnExit.staticValue === true || objArgs.lockOnExit.staticValue === 'true')
: undefined;

if (objArgs?.allowMissing && !objArgs.allowMissing.isStatic) {
throw new SchemaError('Expected allowMissing to be static');
}
const allowMissing = objArgs?.allowMissing
? (objArgs.allowMissing.staticValue === true || objArgs.allowMissing.staticValue === 'true')
: undefined;

if (objArgs?.timeoutMs && !objArgs.timeoutMs.isStatic) {
throw new SchemaError('Expected timeoutMs to be static');
}
let timeoutMs: number | undefined;
if (objArgs?.timeoutMs) {
timeoutMs = Number(objArgs.timeoutMs.staticValue);
if (!Number.isInteger(timeoutMs) || timeoutMs <= 0) {
throw new SchemaError('Expected timeoutMs to be a positive integer (milliseconds)');
}
}

this.instances[id] = new DashlanePluginInstance(id, this.errors.ResolutionError);

return {
id,
autoSync,
lockOnExit,
allowMissing,
timeoutMs,
serviceDeviceKeysResolver: objArgs?.serviceDeviceKeys,
};
}
Expand All @@ -63,11 +83,13 @@ export class DashlaneManager {
* Called at resolution time.
*/
async executeInit({
id, autoSync, lockOnExit, serviceDeviceKeysResolver,
id, autoSync, lockOnExit, allowMissing, timeoutMs, serviceDeviceKeysResolver,
}: {
id: string;
autoSync?: boolean;
lockOnExit?: boolean;
allowMissing?: boolean;
timeoutMs?: number;
serviceDeviceKeysResolver?: ArgValue;
}) {
const serviceDeviceKeys = serviceDeviceKeysResolver
Expand All @@ -78,7 +100,9 @@ export class DashlaneManager {
serviceDeviceKeys && typeof serviceDeviceKeys === 'string'
? serviceDeviceKeys
: undefined,
{ autoSync, lockOnExit },
{
autoSync, lockOnExit, allowMissing, timeoutMs,
},
);
}

Expand Down
14 changes: 10 additions & 4 deletions packages/plugins/dashlane/src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ plugin.registerResolverFunction({
label: 'Fetch secret from Dashlane by dl:// reference',
icon: DASHLANE_ICON,
argsSchema: {
type: 'array',
type: 'mixed',
arrayMinLength: 1,
arrayMaxLength: 2,
},
Expand All @@ -58,9 +58,11 @@ plugin.registerResolverFunction({

manager.getInstance(instanceId);

return { instanceId, refResolver };
const allowMissingResolver = this.objArgs?.allowMissing;

return { instanceId, refResolver, allowMissingResolver };
},
async resolve({ instanceId, refResolver }) {
async resolve({ instanceId, refResolver, allowMissingResolver }) {
manager.registerExitHandler();
const instance = manager.getInstance(instanceId);
if (!refResolver) {
Expand All @@ -70,7 +72,11 @@ plugin.registerResolverFunction({
if (typeof dlUri !== 'string') {
throw new SchemaError('Expected dl:// reference to resolve to a string');
}
return await instance.readReference(dlUri);
// per-call allowMissing overrides the instance-level default
const allowMissing = allowMissingResolver
? !!(await allowMissingResolver.resolve())
: undefined;
return await instance.readReference(dlUri, { allowMissing });
},
});

Expand Down
Loading
Loading