BaryoDev / BaryoDev/barakoCMS

Workflows: credentials inside a Conditional action's child actions are stored and returned in clear

Open
#871 0 comments 0 reactions 0 assignees View on GitHub
core security
Dominant language
C#
Stars
6
Forks
7
Avg merge
4h 42m
Merged PRs (30d)
307

Description

A Conditional action carries its child actions as JSON strings in its `ThenActions` and `ElseActions` parameters. Nothing that protects a credential looks inside them, so a child's `Secret`, `ApiKey`, `Password` or `Token` is stored in clear, copied onto every run in clear, and returned by the API in clear.

Pre-existing. #862 did not cause it: it widened encryption to every credential-named parameter, but only at the top level, the same place `Secret` was encrypted before.

## Where it happens

- `WebhookSigning.ProtectSecrets` walks `workflow.Actions[*].Parameters` only. `ThenActions` and `ElseActions` are not credential-named, so the JSON string holding the child parameters is saved untouched, and the startup migration skips it for the same reason.
- `ConditionalAction.RunAsync` deserialises the branch (`ConditionalAction.cs` around line 80) and calls `plugin.RunAsync(childAction.Parameters, ...)` (around line 119) with the child parameters exactly as stored. The runner's `WebhookSigning.UnprotectCredentials` only sees the parent's parameters.
- `WorkflowActionResponse.From` (`WorkflowResponse.cs` around line 55) builds the response with `WebhookSigning.WithoutSecret(a.Parameters)`, which drops credential-named keys at the top level only and returns `ThenActions` and `ElseActions` whole, child credentials included, to anyone who can read workflows.
- A child Webhook with a `Secret` can never send. Its Secret reaches `WebhookAction` in clear, `ISecretProtector.Unprotect` returns null, and the action refuses permanently with "not protected". A child with a plaintext `ApiKey` does work, which is exactly the value sitting in clear at rest.

## Fix direction

1. On protect (create and the startup migration): parse `ThenActions` and `ElseActions` on a Conditional, run the same credential encryption over each child's parameters (recursively, for a Conditional inside a branch), and write the JSON back. A branch that is not valid JSON is left as it is.
2. On read: `WorkflowActionResponse` drops credential-named keys inside the child JSON the same way it does at the top level, and says whether each child has a secret set.
3. Before running a child: `ConditionalAction` decrypts the child's credentials the way the runner does for a top-level action (everything but `Secret`, which Webhook decrypts itself), and fails the child with a message naming the parameter, never the value, when one will not decrypt.

## Done when

- A workflow created with a Conditional whose child has a `Secret` and an `ApiKey` stores neither in clear (asserted on the raw stored JSON), and a workflow stored that way before the fix is rewritten by the startup migration.
- `GET /api/workflows` and the create response contain neither value.
- The child Webhook signs with the original secret, and a child custom action receives the plaintext `ApiKey`, each proven by a test that fails before the fix.

Contributor guide

Open the contributing guide

Research direction

Start with WebhookSigning.ProtectSecrets and UnprotectCredentials, then read ConditionalAction.cs around the branch deserialization and WorkflowResponse.cs around WorkflowActionResponse.From. Run the existing workflow and Conditional action tests if available. Done means recursive child credentials are protected during creation and migration, omitted from API responses, and child Webhook and custom actions receive the expected values without exposing them.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
api, backend, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.