fix(stage-6): dashboard hardening + audit Lows cherry-pick
Closes 4 dashboard hardening findings and 5 of the Lows from the audit. Dashboard hardening: - Subtabs no longer re-fetch the app via api.apps.get on every page load. users/files/dead-letters drop the fetch outright (the variable was set but never read); queues + queues/[name] now consume the layout's AppContext via getContext for the page title. The layout's reloadApp() owns the historical-slug redirect — subtab-local redirect blocks are removed so there's no race. - The global :global(details > summary::before) chevron is now scoped to details.chevron. The script editor's "Advanced sandbox" details and the inbound-email-shape help-text both opt in; the script exec-list logs no longer inherit a spurious chevron. - deriveTab now matches the path segment by anchored ===, so a future /apps/<slug>/queues-archived route wouldn't activate the queues tab. Lows cherry-pick: - ExecError gains Serialize/Deserialize derives + a snake_case tag so RemoteExecutorClient (cluster mode v1.3+) can round-trip the variant. - triggers_api rejects queue triggers whose visibility_timeout_secs is below the dispatcher's per-message executor budget; with no minimum the reclaim task races the handler and the queue silently double-delivers. Existing test using 5s updated to 30s. - New migration 0040: execution_logs.script_id cascade switched from ON DELETE CASCADE to ON DELETE SET NULL so deleting a script no longer wipes the forensic history that motivated the delete. - New migration 0041: dead_letters composite index on (app_id, created_at DESC) so the "list all" dashboard view stops falling back to seqscan + sort when unresolved=false. - Schema snapshot re-blessed. Deferred to v1.2: the ExecRequest principal serde(skip) marker (documented in-place; the cluster-mode PR will introduce the wire-safe snapshot at that point) and the `pic --help` mention of `picloud admin reset-password` (one-line follow-up). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,18 @@
|
||||
-- Audit Low finding: preserving forensic history.
|
||||
--
|
||||
-- The `execution_logs.script_id` foreign key was created with ON DELETE
|
||||
-- CASCADE in 0001_init. Deleting a script then wipes every log row that
|
||||
-- ever referenced it — including the rows that captured the failure
|
||||
-- that motivated the delete. Switch to ON DELETE SET NULL so the
|
||||
-- forensic history survives and operators can still look up "what did
|
||||
-- this dead script do before we removed it" by id.
|
||||
|
||||
ALTER TABLE execution_logs
|
||||
DROP CONSTRAINT IF EXISTS execution_logs_script_id_fkey;
|
||||
|
||||
ALTER TABLE execution_logs
|
||||
ALTER COLUMN script_id DROP NOT NULL;
|
||||
|
||||
ALTER TABLE execution_logs
|
||||
ADD CONSTRAINT execution_logs_script_id_fkey
|
||||
FOREIGN KEY (script_id) REFERENCES scripts(id) ON DELETE SET NULL;
|
||||
@@ -0,0 +1,12 @@
|
||||
-- Audit Low finding: speeding up the "list all DL for app" view.
|
||||
--
|
||||
-- The dashboard's dead-letters tab issues
|
||||
-- SELECT ... FROM dead_letters
|
||||
-- WHERE app_id = $1
|
||||
-- ORDER BY created_at DESC LIMIT $2
|
||||
-- which falls back to a seq-scan + sort when the unresolved=false
|
||||
-- filter is on (the partial idx_dead_letters_app_unresolved doesn't
|
||||
-- cover resolved rows). This composite covers both modes.
|
||||
|
||||
CREATE INDEX IF NOT EXISTS idx_dead_letters_app_created
|
||||
ON dead_letters (app_id, created_at DESC);
|
||||
@@ -34,6 +34,12 @@ use crate::trigger_repo::{
|
||||
TriggerRepo, TriggerRepoError,
|
||||
};
|
||||
|
||||
/// Minimum allowed queue visibility timeout. Anything shorter races the
|
||||
/// dispatcher's per-message executor budget; reclaim would re-deliver
|
||||
/// the message before the handler returned. Picked to outlast a slow
|
||||
/// handler by a comfortable margin.
|
||||
const MIN_QUEUE_VISIBILITY_TIMEOUT_SECS: u32 = 30;
|
||||
|
||||
#[derive(Clone)]
|
||||
pub struct TriggersState {
|
||||
pub triggers: Arc<dyn TriggerRepo>,
|
||||
@@ -567,6 +573,21 @@ async fn create_queue_trigger(
|
||||
"queue_name must not be empty".into(),
|
||||
));
|
||||
}
|
||||
// Reject obviously-too-short visibility timeouts. The dispatcher
|
||||
// budgets up to ~5 minutes of executor wall-clock per message
|
||||
// (DEFAULT_ASYNC_EXEC_TIMEOUT in dispatcher.rs); a 10s visibility
|
||||
// timeout would race the executor and cause the reclaim task to
|
||||
// re-deliver the message before the first handler returned, so the
|
||||
// queue would silently double-deliver. The audit's Low finding.
|
||||
if let Some(secs) = input.visibility_timeout_secs {
|
||||
if secs < MIN_QUEUE_VISIBILITY_TIMEOUT_SECS {
|
||||
return Err(TriggersApiError::Invalid(format!(
|
||||
"visibility_timeout_secs must be >= {MIN_QUEUE_VISIBILITY_TIMEOUT_SECS} \
|
||||
(shorter than the dispatcher's per-message executor budget; \
|
||||
reclaim would race the handler)"
|
||||
)));
|
||||
}
|
||||
}
|
||||
validate_trigger_target(&*s.scripts, app_id, input.script_id).await?;
|
||||
|
||||
let req = crate::trigger_repo::CreateQueueTrigger {
|
||||
|
||||
@@ -181,7 +181,7 @@ table: email_trigger_details
|
||||
|
||||
table: execution_logs
|
||||
id: uuid NOT NULL default=gen_random_uuid()
|
||||
script_id: uuid NOT NULL
|
||||
script_id: uuid NULL
|
||||
request_id: uuid NOT NULL
|
||||
request_path: text NULL
|
||||
request_headers: jsonb NOT NULL default='{}'::jsonb
|
||||
@@ -403,6 +403,7 @@ indexes on dead_letter_trigger_details:
|
||||
|
||||
indexes on dead_letters:
|
||||
dead_letters_pkey: public.dead_letters USING btree (id)
|
||||
idx_dead_letters_app_created: public.dead_letters USING btree (app_id, created_at DESC)
|
||||
idx_dead_letters_app_unresolved: public.dead_letters USING btree (app_id) WHERE (resolved_at IS NULL)
|
||||
idx_dead_letters_gc: public.dead_letters USING btree (created_at)
|
||||
|
||||
@@ -589,7 +590,7 @@ constraints on email_trigger_details:
|
||||
constraints on execution_logs:
|
||||
[CHECK] execution_logs_status_check: CHECK ((status = ANY (ARRAY['success'::text, 'error'::text, 'timeout'::text, 'budget_exceeded'::text])))
|
||||
[FOREIGN KEY] execution_logs_app_id_fk: FOREIGN KEY (app_id) REFERENCES apps(id) ON DELETE CASCADE
|
||||
[FOREIGN KEY] execution_logs_script_id_fkey: FOREIGN KEY (script_id) REFERENCES scripts(id) ON DELETE CASCADE
|
||||
[FOREIGN KEY] execution_logs_script_id_fkey: FOREIGN KEY (script_id) REFERENCES scripts(id) ON DELETE SET NULL
|
||||
[PRIMARY KEY] execution_logs_pkey: PRIMARY KEY (id)
|
||||
|
||||
constraints on files:
|
||||
@@ -705,3 +706,5 @@ constraints on triggers:
|
||||
0037: drop idx cron triggers due
|
||||
0038: email realtime secret check
|
||||
0039: app user invitations unique pending
|
||||
0040: execution logs keep history
|
||||
0041: dead letters composite idx
|
||||
|
||||
Reference in New Issue
Block a user