deleting a secret provider warns which stacks it unbinds
This commit is contained in:
9 files changed
+186
-10
No files matched your search
@@ -26,6 +26,13 @@
|
||||
/** Bound provider type/name, for the injected banner + pills. */
|
||||
providerType?: string | null;
|
||||
providerName?: string | null;
|
||||
/**
|
||||
* Whether a secret provider is CURRENTLY bound to this stack. When false but
|
||||
* injectedSecretKeys is non-empty, the keys are historical (from the last deploy)
|
||||
* and the next deploy will drop them - the banner says so instead of implying they
|
||||
* are still active (#1522).
|
||||
*/
|
||||
providerBound?: boolean;
|
||||
/** Set when the live provider probe failed - shown as an amber line. */
|
||||
probeError?: string | null;
|
||||
/** Key NAMES the live probe found in the bound provider (present RIGHT NOW).
|
||||
@@ -52,6 +59,7 @@
|
||||
injectedSecretKeys = [],
|
||||
providerType = null,
|
||||
providerName = null,
|
||||
providerBound = true,
|
||||
probeError = null,
|
||||
providerKeySet = new Set<string>(),
|
||||
showInterpolationHint = false,
|
||||
@@ -524,7 +532,7 @@
|
||||
</div>
|
||||
{/if}
|
||||
<!-- Provider-injected secrets loaded at the last deploy -->
|
||||
{#if injectedSecretKeys.length > 0}
|
||||
{#if injectedSecretKeys.length > 0 && providerBound}
|
||||
<div class="flex items-start gap-2 px-2.5 py-2 rounded bg-emerald-50 dark:bg-emerald-900/20 border border-emerald-200 dark:border-emerald-800/50">
|
||||
<Check class="w-4 h-4 text-emerald-500 shrink-0 mt-0.5" />
|
||||
<div class="text-xs text-emerald-700 dark:text-emerald-300 min-w-0">
|
||||
@@ -552,6 +560,24 @@
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
{:else if injectedSecretKeys.length > 0 && !providerBound}
|
||||
<!-- Historical: keys were injected on a previous deploy but no provider is bound now (#1522). -->
|
||||
<div class="flex items-start gap-2 px-2.5 py-2 rounded bg-amber-50 dark:bg-amber-900/20 border border-amber-200 dark:border-amber-800/50">
|
||||
<AlertTriangle class="w-4 h-4 text-amber-500 shrink-0 mt-0.5" />
|
||||
<div class="text-xs text-amber-700 dark:text-amber-300 min-w-0">
|
||||
<div class="font-semibold">No secret provider is bound</div>
|
||||
<p class="text-amber-600 dark:text-amber-400 mt-0.5">
|
||||
These {injectedSecretKeys.length} secret{injectedSecretKeys.length === 1 ? ' was' : 's were'} injected on the last deploy but the stack is no longer bound to a provider. The next deploy will drop {injectedSecretKeys.length === 1 ? 'it' : 'them'}. Reselect a provider to keep them.
|
||||
</p>
|
||||
<div class="flex flex-wrap gap-1.5 mt-1.5">
|
||||
{#each injectedSecretKeys as key}
|
||||
<span class="inline-flex items-center gap-1 font-mono text-2xs font-semibold px-2 py-0.5 rounded-full bg-amber-100 dark:bg-amber-800/40 border border-amber-300 dark:border-amber-700 line-through decoration-amber-500/60">
|
||||
<KeyRound class="w-2.5 h-2.5" />{key}
|
||||
</span>
|
||||
{/each}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
{/if}
|
||||
<!-- Live provider probe couldn't reach the provider: keys fall back to MISSING -->
|
||||
{#if probeError}
|
||||
|
||||
@@ -354,6 +354,20 @@ export async function getSecretProviderById(id: number): Promise<SecretProviderW
|
||||
};
|
||||
}
|
||||
|
||||
/** Lightweight existence check (no config decrypt), for validating a bound provider id. */
|
||||
export async function secretProviderExists(id: number): Promise<boolean> {
|
||||
const rows = await db.select({ id: secretProviders.id }).from(secretProviders).where(eq(secretProviders.id, id));
|
||||
return rows.length > 0;
|
||||
}
|
||||
|
||||
/** Stacks currently bound to a secret provider, so a delete can warn which ones lose it. */
|
||||
export async function getStacksUsingSecretProvider(id: number): Promise<Array<{ stackName: string; environmentId: number | null }>> {
|
||||
return db
|
||||
.select({ stackName: stackSources.stackName, environmentId: stackSources.environmentId })
|
||||
.from(stackSources)
|
||||
.where(eq(stackSources.secretProviderId, id));
|
||||
}
|
||||
|
||||
export async function createSecretProvider(data: {
|
||||
type: SecretProviderType;
|
||||
name: string;
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
/**
|
||||
* Pure git-stack deploy decisions, extracted so they are unit-testable without the
|
||||
* I/O-heavy deployGitStack. A manual/one-off deploy (`force`) deploys but does NOT
|
||||
* force-recreate, so only shouldDeployGitStack takes it.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Whether to deploy at all. A manual/one-off deploy (`force`) or a real git change
|
||||
* always deploys; the "always redeploy on webhook/scheduled sync" setting forces a
|
||||
* deploy even when git reported no changes.
|
||||
*/
|
||||
export function shouldDeployGitStack(input: {
|
||||
force: boolean;
|
||||
forceRedeploy: boolean;
|
||||
gitUpdated: boolean;
|
||||
}): boolean {
|
||||
return input.force || input.forceRedeploy || input.gitUpdated;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether to pass `--force-recreate`. Recreate on a real git change OR when the user
|
||||
* enabled "always redeploy": that setting exists to force the container back in step
|
||||
* with its configuration (a changed env var, secret, or rebound provider leaves no git
|
||||
* diff), and a plain `up` without --force-recreate would no-op and never re-inject the
|
||||
* shell-env secrets (#1523). Default (setting off, no git change) does NOT recreate, so
|
||||
* an ordinary sync never causes a surprise recreate.
|
||||
*/
|
||||
export function shouldForceRecreateGitStack(input: {
|
||||
forceRedeploy: boolean;
|
||||
gitUpdated: boolean;
|
||||
}): boolean {
|
||||
return input.gitUpdated || input.forceRedeploy;
|
||||
}
|
||||
@@ -8,7 +8,8 @@ import {
|
||||
createGitRepository,
|
||||
upsertStackSource,
|
||||
setStackEnvVars,
|
||||
getStackSource
|
||||
getStackSource,
|
||||
secretProviderExists
|
||||
} from '$lib/server/db';
|
||||
import { deployGitStack } from '$lib/server/git';
|
||||
import { authorize } from '$lib/server/authorize';
|
||||
@@ -98,6 +99,13 @@ export const POST: RequestHandler = async (event) => {
|
||||
return json({ error: 'Permission denied: binding a secret provider requires the secrets permission' }, { status: 403 });
|
||||
}
|
||||
|
||||
// A stale provider id (e.g. the provider was deleted/recreated while the editor
|
||||
// held the old list) would otherwise hit a raw foreign-key error on save. Reject
|
||||
// it cleanly so the user knows to reselect a provider (#1522).
|
||||
if (typeof data.secretProviderId === 'number' && !(await secretProviderExists(data.secretProviderId))) {
|
||||
return json({ error: 'The selected secret provider no longer exists. Reopen the stack and pick a current provider.' }, { status: 400 });
|
||||
}
|
||||
|
||||
// Check for name conflicts with existing stacks (regular/external/git)
|
||||
const existing = await getStackSource(trimmedStackName, data.environmentId || null);
|
||||
if (existing) {
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { json } from '@sveltejs/kit';
|
||||
import type { RequestHandler } from './$types';
|
||||
import { getGitStack, updateGitStack, deleteGitStack, deleteStackSource, updateStackSourceName, updateStackEnvVarsName, setStackEnvVars, getStackEnvVars, deleteStackEnvVars, updateStackSource } from '$lib/server/db';
|
||||
import { getGitStack, updateGitStack, deleteGitStack, deleteStackSource, updateStackSourceName, updateStackEnvVarsName, setStackEnvVars, getStackEnvVars, deleteStackEnvVars, updateStackSource, secretProviderExists } from '$lib/server/db';
|
||||
import { deleteGitStackFiles, deployGitStack } from '$lib/server/git';
|
||||
import { normalizeStackBranchUpdate } from '$lib/git-stack-branch';
|
||||
import { authorize } from '$lib/server/authorize';
|
||||
@@ -90,6 +90,12 @@ export const PUT: RequestHandler = async (event) => {
|
||||
return json({ error: 'Permission denied: binding a secret provider requires the secrets permission' }, { status: 403 });
|
||||
}
|
||||
|
||||
// A stale provider id (provider deleted/recreated while the editor held the old
|
||||
// list) would otherwise hit a raw foreign-key error on save (#1522).
|
||||
if (typeof data.secretProviderId === 'number' && !(await secretProviderExists(data.secretProviderId))) {
|
||||
return json({ error: 'The selected secret provider no longer exists. Reopen the stack and pick a current provider.' }, { status: 400 });
|
||||
}
|
||||
|
||||
// Validate stack name if it's being changed
|
||||
if (data.stackName !== undefined) {
|
||||
const trimmedStackName = data.stackName.trim();
|
||||
|
||||
@@ -4,7 +4,8 @@ import {
|
||||
getSecretProviders,
|
||||
getSecretProviderById,
|
||||
updateSecretProvider,
|
||||
deleteSecretProvider
|
||||
deleteSecretProvider,
|
||||
getStacksUsingSecretProvider
|
||||
} from '$lib/server/db';
|
||||
import { hasProvider } from '$lib/server/secretproviders';
|
||||
import { redactProviderConfig } from '$lib/server/secretproviders/shared';
|
||||
@@ -15,7 +16,7 @@ import { auditSecretProvider } from '$lib/server/audit';
|
||||
* @openapi
|
||||
* summary: Get one secret provider with its non-secret config coordinates (the token is redacted out)
|
||||
* path: id:integer The secret provider id
|
||||
* resp-200: {id:integer!, name:string!, type:string!, config:object!}
|
||||
* resp-200: {id:integer!, name:string!, type:string!, config:object!, stacksUsing:array<{stackName:string!, environmentId:integer}>!}
|
||||
* resp-400: Invalid secret provider ID
|
||||
* resp-403: Permission denied (needs secrets:view)
|
||||
* resp-404: Secret provider not found
|
||||
@@ -39,7 +40,17 @@ export const GET: RequestHandler = async ({ params, cookies }) => {
|
||||
return json({ error: 'Secret provider not found' }, { status: 404 });
|
||||
}
|
||||
const { config, ...summary } = full;
|
||||
return json({ ...summary, config: redactProviderConfig(config) });
|
||||
// Name the stacks currently bound to this provider so the delete confirmation can
|
||||
// warn the user that deleting it unbinds them (#1522). A provider is global, but its
|
||||
// bound stacks live in environments - so env-scope the list to what the caller may
|
||||
// see (Enterprise RBAC), or a restricted user would learn stack names in envs they
|
||||
// cannot reach. null = all-environments access (admin/free/disabled) -> no filter.
|
||||
const allStacksUsing = await getStacksUsingSecretProvider(id);
|
||||
const accessibleEnvIds = await auth.getAccessibleEnvironmentIds();
|
||||
const stacksUsing = accessibleEnvIds === null
|
||||
? allStacksUsing
|
||||
: allStacksUsing.filter((s) => s.environmentId === null || accessibleEnvIds.includes(s.environmentId));
|
||||
return json({ ...summary, config: redactProviderConfig(config), stacksUsing });
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
@@ -30,6 +30,11 @@
|
||||
let showModal = $state(false);
|
||||
let editing = $state<SecretProvider | null>(null);
|
||||
let confirmDeleteId = $state<number | null>(null);
|
||||
// Stacks bound to the provider whose delete popover is open, so we can warn the
|
||||
// user that deleting it unbinds them (#1522). Keyed by nothing - only one popover is
|
||||
// open at a time.
|
||||
let deleteAffectedStacks = $state<Array<{ stackName: string; environmentId: number | null }>>([]);
|
||||
let loadingAffected = $state(false);
|
||||
let testingId = $state<number | null>(null);
|
||||
// Brief green tick on the tile's Test button right after a successful test.
|
||||
let testOkId = $state<number | null>(null);
|
||||
@@ -53,6 +58,22 @@
|
||||
showModal = true;
|
||||
}
|
||||
|
||||
async function loadAffectedStacks(id: number) {
|
||||
loadingAffected = true;
|
||||
deleteAffectedStacks = [];
|
||||
try {
|
||||
const res = await fetch(`/api/secret-providers/${id}`);
|
||||
if (res.ok) {
|
||||
const data = await res.json();
|
||||
deleteAffectedStacks = data.stacksUsing ?? [];
|
||||
}
|
||||
} catch {
|
||||
// Non-fatal: the confirmation still works, just without the stack list.
|
||||
} finally {
|
||||
loadingAffected = false;
|
||||
}
|
||||
}
|
||||
|
||||
async function deleteProvider(id: number) {
|
||||
try {
|
||||
const response = await fetch(`/api/secret-providers/${id}`, {
|
||||
@@ -195,10 +216,11 @@
|
||||
position="left"
|
||||
onConfirm={() =>
|
||||
deleteProvider(provider.id)}
|
||||
onOpenChange={(open) =>
|
||||
(confirmDeleteId = open
|
||||
? provider.id
|
||||
: null)}
|
||||
onOpenChange={(open) => {
|
||||
confirmDeleteId = open ? provider.id : null;
|
||||
if (open) loadAffectedStacks(provider.id);
|
||||
else deleteAffectedStacks = [];
|
||||
}}
|
||||
>
|
||||
{#snippet children({ open })}
|
||||
<Trash2
|
||||
@@ -207,6 +229,20 @@
|
||||
: 'text-muted-foreground hover:text-destructive'}"
|
||||
/>
|
||||
{/snippet}
|
||||
{#snippet extraContent()}
|
||||
{#if loadingAffected}
|
||||
<p class="text-xs text-muted-foreground">Checking which stacks use this provider...</p>
|
||||
{:else if deleteAffectedStacks.length > 0}
|
||||
<p class="text-xs text-amber-600 dark:text-amber-500">
|
||||
This unbinds {deleteAffectedStacks.length} stack{deleteAffectedStacks.length === 1 ? '' : 's'}; their next deploy drops the injected secrets:
|
||||
</p>
|
||||
<ul class="mt-1 text-xs text-muted-foreground list-disc list-inside max-h-24 overflow-y-auto">
|
||||
{#each deleteAffectedStacks as s}
|
||||
<li>{s.stackName}</li>
|
||||
{/each}
|
||||
</ul>
|
||||
{/if}
|
||||
{/snippet}
|
||||
</ConfirmPopover>
|
||||
{/if}
|
||||
</div>
|
||||
|
||||
@@ -1365,6 +1365,7 @@
|
||||
injectedSecretKeys={gitStack !== null ? injectedSecretKeys : []}
|
||||
providerType={secretProviders.find((p) => p.id === formSecretProviderId)?.type ?? null}
|
||||
providerName={secretProviders.find((p) => p.id === formSecretProviderId)?.name ?? null}
|
||||
providerBound={formSecretProviderId != null && secretProviders.some((p) => p.id === formSecretProviderId)}
|
||||
placeholder={{ key: 'MY_VAR', value: 'value' }}
|
||||
infoText="Override variables from your repository env files. Non-secrets are saved to <code class='bg-muted px-1 rounded'>.env.dockhand</code> in the stack directory. Secrets are stored in the database and injected via shell environment at deploy time.<br/><br/>Variables are available for <strong>compose file interpolation</strong> using <code class='bg-muted px-1 rounded'>${'{VAR_NAME}'}</code> syntax. They are not automatically injected into containers — use <code class='bg-muted px-1 rounded'>environment:</code> or reference <code class='bg-muted px-1 rounded'>.env.dockhand</code> in <code class='bg-muted px-1 rounded'>env_file:</code> to pass them through."
|
||||
existingSecretKeys={gitStack !== null ? existingSecretKeys : new Set()}
|
||||
|
||||
@@ -0,0 +1,41 @@
|
||||
/**
|
||||
* #1523: the "always redeploy" (forceRedeploy) setting must drive --force-recreate,
|
||||
* not just whether we deploy. Previously forceRecreate was computed from the git-diff
|
||||
* flag alone, so the setting was read and discarded and the container never re-received
|
||||
* its shell-env secrets on a config-only change.
|
||||
*/
|
||||
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { shouldDeployGitStack, shouldForceRecreateGitStack } from '../src/lib/server/git-deploy-policy';
|
||||
|
||||
describe('shouldDeployGitStack', () => {
|
||||
test('deploys on a real git change', () => {
|
||||
expect(shouldDeployGitStack({ force: false, forceRedeploy: false, gitUpdated: true })).toBe(true);
|
||||
});
|
||||
test('deploys on a manual/forced deploy', () => {
|
||||
expect(shouldDeployGitStack({ force: true, forceRedeploy: false, gitUpdated: false })).toBe(true);
|
||||
});
|
||||
test('deploys when forceRedeploy is on even with no git change', () => {
|
||||
expect(shouldDeployGitStack({ force: false, forceRedeploy: true, gitUpdated: false })).toBe(true);
|
||||
});
|
||||
test('skips when nothing changed and nothing forced', () => {
|
||||
expect(shouldDeployGitStack({ force: false, forceRedeploy: false, gitUpdated: false })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('shouldForceRecreateGitStack', () => {
|
||||
test('recreates on a real git change', () => {
|
||||
expect(shouldForceRecreateGitStack({ forceRedeploy: false, gitUpdated: true })).toBe(true);
|
||||
});
|
||||
test('#1523: recreates when forceRedeploy is on even with NO git change', () => {
|
||||
// The core bug: this was false before the fix, so the container never recreated
|
||||
// and the shell-env secrets were never re-injected.
|
||||
expect(shouldForceRecreateGitStack({ forceRedeploy: true, gitUpdated: false })).toBe(true);
|
||||
});
|
||||
test('recreates when both are set', () => {
|
||||
expect(shouldForceRecreateGitStack({ forceRedeploy: true, gitUpdated: true })).toBe(true);
|
||||
});
|
||||
test('default (setting off, no git change) does NOT recreate - no surprise recreates', () => {
|
||||
expect(shouldForceRecreateGitStack({ forceRedeploy: false, gitUpdated: false })).toBe(false);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user