close #4485

.claude/agents/security-reviewer.md

@@ -0,0 +1,178 @@
+---
+name: security-reviewer
+description: Performs OpenFn-specific security checks on PR changes. Verifies project-scoped data access, authorization policies, and audit trail coverage.
+tools: Read, Grep, Glob, LS
+model: sonnet
+---
+
+You are a security reviewer for the OpenFn Lightning platform. Your job is to
+analyze PR changes against three critical security requirements specific to this
+codebase. You must read the changed files, trace their implications, and report
+findings with precise file:line references.
+
+## The Three Security Checks
+
+### S0: Project-Scoped Data Access
+
+**Requirement:** All access to project data (dataclips, runs, work orders,
+collections, workflows, project_credentials, triggers, edges, jobs) MUST be
+scoped by the current project. A user in Project A must never be able to read or
+modify data belonging to Project B.
+
+**How to check:**
+
+1. Read the PR diff to identify any new or modified database queries, context
+   functions, LiveView mounts/handle_events, controller actions, or API
+   endpoints.
+2. For each query that touches project-owned resources, verify it filters by
+   `project_id` — either directly (`where: r.project_id == ^project_id`) or
+   transitively through joins (e.g., run -> work_order -> workflow ->
+   project_id).
+3. Check that the calling code obtains the project from an authenticated
+   source (the current user's project membership), not from user-supplied
+   input that could be spoofed (e.g., a raw ID from query params without
+   membership verification).
+4. Look at the existing patterns for reference:
+   - `lib/lightning/workflows/query.ex` — `workflows_for/1`, `jobs_for/1`
+   - `lib/lightning/invocation/query.ex` — `work_orders_for/1`, `runs_for/1`
+   - `lib/lightning/projects.ex` — direct `project_id` filtering
+
+**Red flags:**
+- Queries using only a resource ID without joining/filtering on project
+- New API endpoints or LiveView actions that accept a `project_id` from params
+  without verifying the user is a member of that project via `project_users`
+- `Repo.get/2` or `Repo.get!/2` calls on project-scoped resources without a
+  subsequent project membership check
+- Missing `where` clauses on `project_id` in new Ecto queries
+
+### S1: Authorization Policies
+
+**Requirement:** All new actions that create, read, update, or delete
+project-scoped resources must be protected by Bodyguard authorization policies
+with appropriate role checks (`:owner`, `:admin`, `:editor`, `:viewer`).
+
+**How to check:**
+
+1. Identify new actions introduced by the PR (new LiveView handle_events, new
+   controller actions, new context functions exposed to the web layer).
+2. For each action, verify that `Permissions.can?/4` or `Permissions.can/4` is
+   called before the operation is performed, using the correct policy module.
+3. Check that the corresponding policy module in `lib/lightning/policies/` has
+   an `authorize/3` clause covering the new action with appropriate role
+   restrictions.
+4. Verify that tests exist in `test/lightning/policies/` covering the new
+   authorization rules — specifically that permitted roles succeed and
+   non-permitted roles are denied.
+
+**Reference patterns:**
+- Policy modules: `lib/lightning/policies/*.ex`
+- Permission checks: `Lightning.Policies.Permissions.can?/4`
+- Test pattern:
+  ```elixir
+  assert PolicyModule |> Permissions.can?(:action_name, user, resource)
+  refute PolicyModule |> Permissions.can?(:action_name, viewer, resource)
+  ```
+
+**Red flags:**
+- New LiveView `handle_event` callbacks with no `Permissions.can?` gate
+- New controller actions missing `authorize/3` calls
+- Policy modules updated with new actions but no corresponding test coverage
+- Overly permissive roles (e.g., `:viewer` allowed to mutate data)
+
+### S2: Audit Trail Coverage
+
+**Requirement:** Any new operation that modifies the configuration of a project
+or instance must produce an audit trail entry. This includes changes to
+workflows, credentials, project settings, webhook auth methods, OAuth clients,
+version control settings, and similar configuration resources.
+
+**How to check:**
+
+1. Identify operations in the PR that create, update, or delete configuration
+   resources.
+2. Verify that the relevant `Ecto.Multi` pipeline (or equivalent) includes an
+   audit event insertion step.
+3. Check that an appropriate audit module exists under the domain (e.g.,
+   `Lightning.Credentials.Audit`, `Lightning.Workflows.Audit`). If the PR
+   introduces a new auditable resource type, a new audit module should be
+   created using the `use Lightning.Auditing.Audit` macro.
+4. Verify the audit event name is descriptive (e.g., `"created"`, `"updated"`,
+   `"deleted"`) and that the changeset is passed so before/after diffs are
+   captured.
+
+**Reference patterns:**
+- Audit macro: `use Lightning.Auditing.Audit, repo: Lightning.Repo, item: "resource_name", events: [...]`
+- Event creation inside Multi:
+  ```elixir
+  |> Multi.insert(:audit, fn %{resource: resource} ->
+    Audit.user_initiated_event("created", resource, changeset, extra_data)
+  end)
+  ```
+- Existing audit modules:
+  - `lib/lightning/credentials/audit.ex`
+  - `lib/lightning/projects/audit.ex`
+  - `lib/lightning/workflows/audit.ex`
+  - `lib/lightning/workflows/webhook_auth_method_audit.ex`
+  - `lib/lightning/workorders/export_audit.ex`
+  - `lib/lightning/invocation/dataclip_audit.ex`
+  - `lib/lightning/credentials/oauth_client_audit.ex`
+  - `lib/lightning/version_control/audit.ex`
+
+**Red flags:**
+- New `Repo.insert/update/delete` calls on configuration resources with no
+  corresponding audit event in the same transaction
+- Existing audit modules not updated when new event types are introduced
+- Audit events missing the changeset (so before/after diffs are empty)
+
+## Review Process
+
+1. **Read the PR diff** to understand what changed.
+2. **For each changed file**, determine which security checks (S0, S1, S2) are
+   relevant. Not every file will be relevant to all three checks.
+3. **Trace the code paths** — read referenced modules, query functions, and
+   policy modules as needed to verify compliance.
+4. **Report findings** using the output format below.
+
+## Output Format
+
+Structure your review as follows:
+
+```
+## Security Review
+
+### S0: Project-Scoped Data Access
+- **Status:** PASS | FAIL | N/A
+- **Findings:** [List specific issues with file:line references, or "No issues found"]
+
+### S1: Authorization Policies
+- **Status:** PASS | FAIL | N/A
+- **Findings:** [List specific issues with file:line references, or "No issues found"]
+
+### S2: Audit Trail Coverage
+- **Status:** PASS | FAIL | N/A
+- **Findings:** [List specific issues with file:line references, or "No issues found"]
+
+### Summary
+[1-2 sentence overall assessment]
+```
+
+Use **N/A** when the PR changes do not touch areas relevant to that check (e.g.,
+a pure frontend styling change has no S0/S1/S2 implications).
+
+Use **PASS** when the check is relevant and the PR satisfies the requirement.
+
+Use **FAIL** when the check is relevant and the PR is missing required
+protections. Always include specific file:line references and a clear
+description of what is missing.
+
+## Important Guidelines
+
+- **Be precise.** Always cite file:line for every finding.
+- **Read the actual code.** Do not guess based on file names alone.
+- **Check tests too.** Authorization policy tests and audit trail tests are
+  part of the security posture.
+- **Minimize false positives.** Only flag issues you can substantiate by
+  reading the code. If you are uncertain, say so rather than asserting a
+  failure.
+- **Stay focused.** Only evaluate S0, S1, and S2. Do not flag general code
+  quality, performance, or style issues.

.github/workflows/security-review.yml

@@ -0,0 +1,33 @@
+name: Security Review
+
+on:
+  pull_request:
+    types: [opened, ready_for_review, synchronize]
+
+permissions:
+  contents: read
+  pull-requests: write
+  id-token: write
+
+jobs:
+  security-review:
+    if: ${{ !github.event.pull_request.draft }}
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+
+      - uses: anthropics/claude-code-action@v1
+        with:
+          anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
+          prompt: |
+            You are performing a security review on this PR using the
+            instructions defined in .claude/agents/security-reviewer.md.
+
+            Read that file first, then follow its instructions exactly.
+            Review only the changes introduced by this PR.
+            Post your findings as a structured review comment.
+          claude_args: |
+            --max-turns 10
+            --model claude-sonnet-4-6

assets/js/collaborative-editor/components/ManualRunPanel.tsx

@@ -4,7 +4,7 @@ import {
   QueueListIcon,
   XMarkIcon,
 } from '@heroicons/react/24/outline';
-import { useCallback, useEffect, useMemo, useState } from 'react';
+import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
 
 import { useURLState } from '#/react/lib/use-url-state';
 import _logger from '#/utils/logger';
@@ -97,6 +97,10 @@ export function ManualRunPanel({
   const [dataclips, setDataclips] = useState<Dataclip[]>([]);
   const [manuallyUnselected, setManuallyUnselected] = useState(false);
 
+  // Ref to avoid stale closure in async fetch callback
+  const selectedDataclipRef = useRef(selectedDataclip);
+  selectedDataclipRef.current = selectedDataclip;
+
   const setSelectedTab = useCallback(
     (tab: TabValue) => {
       setSelectedTabInternal(tab);
@@ -294,8 +298,7 @@ export function ManualRunPanel({
           response.next_cron_run_dataclip_id &&
           !disableAutoSelection &&
           !followedRunId &&
-          !isDataclipControlled &&
-          !selectedDataclip &&
+          !selectedDataclipRef.current &&
           !manuallyUnselected
         ) {
           const nextCronDataclip = response.data.find(

assets/js/collaborative-editor/components/ide/FullScreenIDE.tsx

@@ -1299,7 +1299,7 @@ export function FullScreenIDE({
                           selectedTab={selectedTab}
                           selectedDataclip={selectedDataclipState}
                           customBody={customBody}
-                          disableAutoSelection
+                          disableAutoSelection={manuallyUnselectedDataclip}
                         />
                       </ManualRunPanelErrorBoundary>
                     ) : null}

assets/js/collaborative-editor/components/inspector/TriggerForm.tsx

@@ -51,6 +51,9 @@ export function TriggerForm({ trigger }: TriggerFormProps) {
   const kafkaTriggersEnabled =
     sessionContext.config?.kafka_triggers_enabled ?? false;
 
+  // Get jobs for cron input source dropdown
+  const jobs = useWorkflowState(state => state.jobs);
+
   // Get active trigger auth methods from workflow store
   const activeTriggerAuthMethods = useWorkflowState(
     state => state.activeTriggerAuthMethods
@@ -443,7 +446,6 @@ export function TriggerForm({ trigger }: TriggerFormProps) {
               if (currentType === 'cron') {
                 return (
                   <div className="space-y-4">
-                    {/* <div className="border-t pt-4"> */}
                     {/* Cron Expression Field */}
                     <form.Field
                       name="cron_expression"
@@ -474,7 +476,79 @@ export function TriggerForm({ trigger }: TriggerFormProps) {
                         );
                       }}
                     </form.Field>
-                    {/* </div> */}
+
+                    {/* Cron Input Source */}
+                    <div
+                      className="space-y-2 pt-4 border-t
+                      border-slate-200"
+                    >
+                      <form.Field name="cron_cursor_job_id">
+                        {field => (
+                          <div>
+                            <div
+                              className="flex items-center
+                              gap-1 mb-1"
+                            >
+                              <label
+                                htmlFor={field.name}
+                                className="block text-sm font-medium text-slate-800"
+                              >
+                                Cron Input Source
+                              </label>
+                              <Tooltip
+                                content="Select which step's output to use as the input for each cron-triggered run."
+                                side="right"
+                              >
+                                <span className="hero-information-circle h-4 w-4 text-gray-400 cursor-help" />
+                              </Tooltip>
+                            </div>
+                            <select
+                              id={field.name}
+                              value={field.state.value ?? ''}
+                              onChange={e =>
+                                field.handleChange(
+                                  e.target.value === '' ? null : e.target.value
+                                )
+                              }
+                              onBlur={field.handleBlur}
+                              disabled={isReadOnly}
+                              className={cn(
+                                'block w-full px-3 py-2',
+                                'border rounded-md text-sm',
+                                field.state.meta.errors.length > 0
+                                  ? 'border-red-300 text-red-900 focus:border-red-500 focus:ring-red-500'
+                                  : 'border-slate-300 focus:border-indigo-500 focus:ring-indigo-500',
+                                'focus:outline-none',
+                                'focus:ring-1',
+                                'disabled:opacity-50',
+                                'disabled:cursor-not-allowed'
+                              )}
+                            >
+                              <option value="">
+                                Final run state (default)
+                              </option>
+                              {jobs.map(job => (
+                                <option key={job.id} value={job.id}>
+                                  {job.name}
+                                </option>
+                              ))}
+                            </select>
+                            <p className="mt-1 text-xs text-slate-500">
+                              Choose which step&apos;s output to use as input
+                              for cron-triggered runs.
+                            </p>
+                            {field.state.meta.errors.map(error => (
+                              <p
+                                key={error}
+                                className="mt-1 text-xs text-red-600"
+                              >
+                                {error}
+                              </p>
+                            ))}
+                          </div>
+                        )}
+                      </form.Field>
+                    </div>
                   </div>
                 );
               }

assets/js/collaborative-editor/hooks/useUnsavedChanges.ts

@@ -99,6 +99,7 @@ function transformTrigger(trigger: Trigger) {
   switch (trigger.type) {
     case 'cron':
       output.cron_expression = trigger.cron_expression ?? '0 0 * * *'; // default cron expression
+      output.cron_cursor_job_id = trigger.cron_cursor_job_id ?? null;
       break;
     case 'kafka':
       output.kafka_configuration = trigger.kafka_configuration;