Skip to content

feat: support secrets in expressions and HTTP node authorization#5708

Open
andrecalil wants to merge 15 commits into
mainfrom
feat/support-secrets-http-node
Open

feat: support secrets in expressions and HTTP node authorization#5708
andrecalil wants to merge 15 commits into
mainfrom
feat/support-secrets-http-node

Conversation

@andrecalil

@andrecalil andrecalil commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Adds first-class secret support across components and the HTTP node, and hardens secret handling.

  • New secrets("name").key expression function usable in any component's text/expression fields; resolved at execution time and never persisted into stored configuration. Surfaced in expression validation, autocomplete, and in-app expression help.
  • HTTP node authorization (Bearer / Basic Auth / Custom Header) backed by organization secrets via SecretKeyRef.
  • Secret-usage redesign addressing security concerns in the node configuration builder / secret resolver.
  • Docs: new general "Expressions" page documenting secrets(); component docs regenerated.

Signed-off-by: André Calil <andre@calil.com.br>
@superplanehq-integration

Copy link
Copy Markdown

👋 Commands for maintainers:

  • /sp start - Start an ephemeral machine (takes ~30s)
  • /sp stop - Stop a running machine (auto-executed on pr close)

@andrecalil

Copy link
Copy Markdown
Collaborator Author

/sp-staging

@superplane-gh-integration-9000

Copy link
Copy Markdown

Docs Impact Review

This PR introduces inline {{ secrets.NAME.KEY }} references for the HTTP node, a new user-facing secret consumption pattern not mentioned anywhere in the current docs.

Suggested docs updates:

  • docs/security/secrets — "Using Secrets in Components" section: Update to describe the new inline syntax ({{ secrets.NAME.KEY }}), list where it applies (URL, query-param values, form-data values, JSON string values, plain text, XML body), and note the limitation that secret names or keys containing a literal . must use the Authorization section instead. Also remove or revise the placeholder sentence "If a component supports organization secrets in the future, it will be called out in that component's documentation" — it reads as if HTTP is excluded, which is now incorrect.

Why: The security/secrets page currently states that only the SSH Command component uses organization secrets in the Core set, making it actively misleading now that HTTP supports them. The entirely new {{ secrets.NAME.KEY }} inline-template syntax (as opposed to the existing structured-field approach) is a conceptual pattern that belongs on the Secrets concept page; the auto-generated Core.mdx will cover the per-field reference detail, but the syntax, scope, and constraints need explanation at the concept level where users learn how secrets work.


Maintainer commands

Command What it does
/docs-agree Open a tracking issue in superplanehq/docs and assign it to you.
/docs-reject <reason> Dismiss this check. A short reason is required (e.g. /docs-reject not user-facing).

Posted automatically by warp-gateway · commit d93dbc0

@andrecalil

Copy link
Copy Markdown
Collaborator Author

/docs-agree

Comment thread pkg/components/http/http.go Outdated
@superplane-gh-integration-9000

superplane-gh-integration-9000 Bot commented Jun 26, 2026

Copy link
Copy Markdown

Risk: 65/100 (high)

Summary

Adds a secrets() expression function that defers secret resolution to runtime, guards against full-map secret exposure, and wires the encryptor through the worker pipeline so HTTP node configurations can reference org secrets.

Concerns

  • Missing secret key silently returns empty string instead of erroring.
  • No authorization check: any canvas expression can access any org secret by name.
  • RuntimeSecretResolver wraps errors with %v, breaking error chain unwrapping.
  • No per-execution secret caching; duplicate secrets() calls each hit the database.
  • No end-to-end integration tests for secrets in HTTP Authorization headers.
  • configBuilder in process_queue_context.go intentionally omits resolver but is easy to misuse at future call sites.

Recommended reviewers: forestileao

Signed-off-by: André Calil <andre@calil.com.br>
Signed-off-by: André Calil <andre@calil.com.br>
@shiroyasha

Copy link
Copy Markdown
Collaborator

From the security side:

What happens if I enter value = {{ secrets.myserver.sshKey + "aaa" }}?

@shiroyasha

Copy link
Copy Markdown
Collaborator

From the product side:

I don't like that expressions are working differently in the HTTP component compared to the rest of the system. It introduces diverging behavior, that is harder to document and maintain.

Take a look at some of the previous PRs that tried to address this in the whole app, you might find some inspiration:

#2899

@shiroyasha shiroyasha left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets try to figure out a way for all components to be able to access secrets in expressions.

Comment thread pkg/components/http/http.go Outdated
Signed-off-by: André Calil <andre@calil.com.br>
@cursor

cursor Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Current version of PR was reviewed by /review-bugbot on Jun 26, 16:21 GMT-3. It flagged 2 findings.

Show 2 findings

1. Loop until secrets() unavailable

pkg/workers/contexts/node_configuration_builder.go:397 · High Severity

This change adds secrets() to the expression engine and documents it for the Loop component’s until expression, but Loop evaluates that expression through ProcessQueueContext.Expressions, whose builder is still created without a SecretResolver. Any until condition that calls secrets() fails at runtime with secrets() is not available in this context and the loop errors out.

2. HTTP docs use wrong syntax

docs/components/Core.mdx:688 · High Severity

The new HTTP “Inline Secrets” section documents {{ secrets.SECRET_NAME.KEY_NAME }}, but the implementation removed dot-notation secret substitution and only supports secrets("name").key inside standard {{ ... }} expressions (as described in pkg/components/http/http.go). Users following the Core docs will configure syntax that does not resolve secrets and fails validation or runtime evaluation.

Bugbot on commit e7db6ef is skipped.

Signed-off-by: André Calil <andre@calil.com.br>
Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread pkg/workers/node_request_worker.go
Signed-off-by: André Calil <andre@calil.com.br>
@andrecalil

Copy link
Copy Markdown
Collaborator Author

Lets try to figure out a way for all components to be able to access secrets in expressions.

What about this?

Every text field in this component accepts standard expression placeholders
`{{ ... }}`. Use `secrets("name").key` inside an expression to inject an
organization secret value (for example: `{{ secrets("api").token }}`). Secrets are
resolved at execution time and are not stored back into the saved configuration.

@andrecalil

Copy link
Copy Markdown
Collaborator Author

/score-pr

@superplanehq-integration

Copy link
Copy Markdown

Clean Code Score: 82/100

This PR introduces a clear, safety-conscious design for secrets() resolution (deferred at queue/build time, resolved at runtime) with good documentation and helpful tests around the core expression/building behavior. The main deductions come from only moderate measurable coverage in the affected packages, plus a couple of Clean Code issues at the boundary layer (notably error wrapping and context propagation in the new runtime secret loader).

Measured metrics

  • Test coverage of changed code: 52.3% total statement coverage for the targeted changed packages (measured via make test.coverage PKG_TEST_PACKAGES='./pkg/workers/... ./pkg/configuration/expressionvalidation/... ./pkg/components/http ./pkg/components/merge'; per-package: pkg/workers 47.6%, pkg/workers/contexts 57.6%, pkg/configuration/expressionvalidation 72.8%, pkg/components/http 67.9%, pkg/components/merge 56.8%). Changed-line coverage specifically is not measured (coverage reported at package/statement level).
  • Dependency structure / direction: measured by diff inspection only (no cycle/layer tool run). The new dependency from pkg/workers/contexts to pkg/secrets/pkg/crypto stays in the worker/runtime layer; no obvious new dependency inversion issues introduced.
  • Cyclomatic complexity of changed/added functions: not measured with a complexity tool. By inspection, new/changed functions are mostly low-branch (e.g., expressionInjectsSecret, secretsCallCollector.Visit, ResolveStoredConfiguration), with the most branching in ResolveTemplateExpressions and the new secrets expr function arity/type checks.
  • Module / file / function sizes: not measured with a sizing tool. Diff adds one small focused file (pkg/workers/contexts/secret_resolver.go, ~70 LOC) and modestly expands node_configuration_builder.go with well-scoped helpers; no obviously bloated functions introduced.
  • Mutation testing: not measured (no repo-configured, fast mutation target/tool observed in existing workflow).

Top improvements (with concrete examples from THIS PR)

  1. Improve error handling fidelity at the secrets boundary: - pkg/workers/contexts/secret_resolver.go uses fmt.Errorf("secret %q: %v", name, err) (twice), which loses wrapping (%w) and makes errors.Is/As impossible; consider switching to %w and adding context once (also helps testability and reliability).
  2. Propagate request/execution context rather than using context.Background(): - RuntimeSecretResolver.Resolve calls provider.Load(context.Background()) in pkg/workers/contexts/secret_resolver.go; this bypasses cancellation/timeouts from the worker/execution path and can make failures harder to control operationally.
  3. Add at least one higher-level test that exercises the runtime resolution path through the worker boundary: - the PR has strong unit tests for builder behavior (pkg/workers/contexts/node_configuration_builder_secrets_test.go) but the new orchestration glue (e.g. NodeExecutor.executeActionNode resolving stored config via ResolveStoredConfiguration, and NodeRequestWorker.invokeExecutionComponentHook rebuilding config with WithInput) is only covered indirectly; a focused integration-style test around one of those flows would raise confidence that secrets resolve correctly across the full execution lifecycle.

Comment thread pkg/components/http/http.go Outdated
- **Plain Text**: Raw text content
- **XML**: XML formatted content

## Secrets in expressions

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be here, right? Since this is not specific to the HTTP node, we should document it here instead

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the PR title and description is no longer true. Please, update it as well so we don't mess our changelogs

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed on both, addressed

@superplane-policy-bot superplane-policy-bot Bot requested a review from lucaspin June 29, 2026 20:52
@andrecalil andrecalil changed the title feat: Support Secrets at Http Node feat: support secrets in expressions and HTTP node authorization Jun 29, 2026
Signed-off-by: André Calil <andre@calil.com.br>
Comment thread pkg/workers/contexts/node_configuration_builder.go
Comment thread pkg/workers/contexts/node_configuration_builder.go

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 2a2d1ae. Configure here.

"parameters": spec.InvokeAction.Parameters,
}).
WithConfigurationFields(hookProvider.Configuration()).
WithSecretResolver(contexts.NewRuntimeSecretResolver(tx, w.encryptor, models.DomainTypeOrganization, workflow.OrganizationID)).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gRPC trigger hook skips resolver

Medium Severity

User-invoked trigger hooks still build configuration with a deferred NodeConfigurationBuilder, so secrets() placeholders in hook fields are not resolved before TriggerHookContext runs, unlike the worker trigger-hook path that now uses WithSecretResolver.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 2a2d1ae. Configure here.

- Are never written back into the saved component configuration. Only the expression `secrets("api").token` is persisted; the actual token only exists in memory during execution.
- Must select a specific key. Embedding the whole secret (`secrets("api")` without a key) is rejected so the entire decrypted secret cannot leak into a URL, header, payload, or log.

If a referenced secret or key does not exist at execution time, the run fails with an error identifying the missing secret.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing secret keys stay silent

Medium Severity

The new expressions docs say a run fails when a referenced secret or key is missing, but runtime resolution treats a missing key inside secrets("name").key as an empty string and continues, which can send requests with blank tokens or credentials.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 2a2d1ae. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants