Skip to content

Fix: store sensitive step outputs in a companion Secret instead of the plaintext context ConfigMap - #245

Open
sakirr05 wants to merge 1 commit into
kubevela:mainfrom
sakirr05:fix/sensitive-step-outputs-secret-store
Open

Fix: store sensitive step outputs in a companion Secret instead of the plaintext context ConfigMap#245
sakirr05 wants to merge 1 commit into
kubevela:mainfrom
sakirr05:fix/sensitive-step-outputs-secret-store

Conversation

@sakirr05

@sakirr05 sakirr05 commented Jul 31, 2026

Copy link
Copy Markdown

Description of your changes

Workflow vars — including step outputs whose values come straight out of
Kubernetes Secrets — all get serialized into workflow-<name>-context ConfigMap
data by writeToStore. So a step like kubevela's generate-jdbc-connection,
which reads a Secret and exposes the decoded password, leaks that password in
plaintext to anyone with configmap read access the moment a user declares an
output on it. Same class of problem as kubevela/kubevela#6840, which recently
got fixed on the policy side — this is the workflow-side variant.

The fix follows the same idea, adapted to how the context store works:

  • Step templates can declare $sensitivePaths: ["password", ...] in their
    rendered value. Outputs whose valueFrom overlaps a declared path (either
    direction — extracting a parent of a sensitive path still includes the value)
    get routed through a new SetSensitiveVar instead of SetVar.
  • Sensitive vars are persisted to a companion Opaque Secret
    (workflow-<name>-context-sensitive) with the same owner references as the
    ConfigMap, so GC is unchanged. The Secret is only created once something
    sensitive exists — plain workflows never get one.
  • GetVar reads both stores transparently, so inputs/data-passing between
    steps work exactly as before regardless of where a var lives.
  • If valueFrom is an expression rather than a plain dotted path and the
    template declared any sensitive paths, it's treated as sensitive — can't
    prove an expression doesn't embed the value, so it fails closed.
  • A failed Secret write fails the Commit (reconcile retries). It never falls
    back to writing the ConfigMap.
  • In-memory context mode (EnableInMemoryContext) keeps sensitive vars in the
    in-memory object, which never reaches the API server anyway.

Couple of honest limitations: templates have to opt in by declaring
$sensitivePaths, existing templates aren't retroactively protected. And the
alternative — a sensitive field on the outputs API type — would be a
kubevela/pkg change plus CRD schema updates across repos, which felt too heavy
for a first pass. Open to going that way if preferred.

How has this code been tested

  • New tests: round-trip (value in Secret, absent from ConfigMap, readable via
    GetVar), no-Secret-when-nothing-sensitive, LoadContext recovery from an
    existing Secret, sync-on-later-commits, end-to-end Output() routing, and a
    table test for the path matching.
  • Existing pkg/context and pkg/hooks tests all pass.
  • Full ./pkg/... suite: the two http provider packages fail identically on a
    clean main checkout (SHA1-signed test cert rejected by go1.23), so those are
    pre-existing and unrelated.
  • gofmt / go vet / go build clean.

Summary by cubic

Stores sensitive step outputs in a companion Secret instead of the plaintext workflow context ConfigMap to prevent leaks. Templates declare $sensitivePaths; matching outputs are saved securely, and GetVar reads remain seamless.

  • Bug Fixes

    • Added SetSensitiveVar to persist sensitive vars to workflow-<name>-context-sensitive (Opaque, owner-referenced).
    • Output() routes outputs whose valueFrom overlaps $sensitivePaths; expressions are treated as sensitive.
    • GetVar reads from both ConfigMap and Secret; EnableInMemoryContext keeps sensitive data in memory only.
    • Secret write errors fail commit; no fallback to ConfigMap; no Secret is created for non-sensitive workflows.
  • Migration

    • Add $sensitivePaths to step templates to protect outputs; existing templates stay unchanged.

Written for commit 210737c. Summary will update on new commits.

Review in cubic

… the plaintext context ConfigMap

All workflow vars, including step outputs derived from Kubernetes Secrets,
are persisted to the workflow-<name>-context ConfigMap in plaintext. Let step
templates declare $sensitivePaths; matching outputs are routed through a new
SetSensitiveVar into an owner-referenced Opaque Secret instead. GetVar reads
both stores transparently, ambiguous valueFrom expressions fail closed, and a
failed Secret write fails the Commit rather than falling back to the ConfigMap.

Workflow-side counterpart of kubevela/kubevela#6840.

Signed-off-by: sakirr05 <sakirahmed75531@gmail.com>
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 24.32432% with 84 lines in your changes missing coverage. Please review.
✅ Project coverage is 24.77%. Comparing base (d7db9c4) to head (210737c).
⚠️ Report is 19 commits behind head on main.

Files with missing lines Patch % Lines
pkg/context/context.go 22.50% 58 Missing and 4 partials ⚠️
pkg/hooks/data_passing.go 29.03% 20 Missing and 2 partials ⚠️

❌ Your patch check has failed because the patch coverage (24.32%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage.

❗ There is a different number of reports uploaded between BASE (d7db9c4) and HEAD (210737c). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (d7db9c4) HEAD (210737c)
unit-test 1 0
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #245       +/-   ##
===========================================
- Coverage   62.49%   24.77%   -37.72%     
===========================================
  Files          62       64        +2     
  Lines        4415     5336      +921     
===========================================
- Hits         2759     1322     -1437     
- Misses       1324     3780     +2456     
+ Partials      332      234       -98     
Flag Coverage Δ
e2etests 24.77% <24.32%> (?)
unit-test ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

3 issues found across 5 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="pkg/hooks/data_passing.go">

<violation number="1" location="pkg/hooks/data_passing.go:78">
P1: Malformed `$sensitivePaths` silently disables protection and routes every output to the plaintext ConfigMap. Reject invalid declarations or fail closed by routing outputs through `SetSensitiveVar` when this field exists but cannot decode.</violation>
</file>

<file name="pkg/context/context.go">

<violation number="1" location="pkg/context/context.go:83">
P1: A sensitive output reusing an existing normal-context name still resolves to, and leaves behind, the ConfigMap value. Clear/migrate that ordinary path when classifying it sensitive and make the sensitive value authoritative, so opt-in updates cannot retain or consume stale plaintext.</violation>

<violation number="2" location="pkg/context/context.go:310">
P1: A pre-existing Secret with the companion name is overwritten with sensitive workflow output without proving it belongs to this context. Validate owner references/identity before patching (or fail and choose a safe store) to avoid disclosing data to a pre-created Secret and corrupting it.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread pkg/hooks/data_passing.go
return nil
}
var paths []string
if err := v.Decode(&paths); err != nil {

@cubic-dev-ai cubic-dev-ai Bot Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: Malformed $sensitivePaths silently disables protection and routes every output to the plaintext ConfigMap. Reject invalid declarations or fail closed by routing outputs through SetSensitiveVar when this field exists but cannot decode.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/hooks/data_passing.go, line 78:

<comment>Malformed `$sensitivePaths` silently disables protection and routes every output to the plaintext ConfigMap. Reject invalid declarations or fail closed by routing outputs through `SetSensitiveVar` when this field exists but cannot decode.</comment>

<file context>
@@ -59,9 +59,70 @@ func Input(ctx wfContext.Context, paramValue cue.Value, step oamv1alpha1.Workflo
+		return nil
+	}
+	var paths []string
+	if err := v.Decode(&paths); err != nil {
+		return nil
+	}
</file context>
Fix with cubic

Comment thread pkg/context/context.go
}
return err
}
return cli.Patch(ctx, secret, client.MergeFrom(existing.DeepCopy()))

@cubic-dev-ai cubic-dev-ai Bot Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: A pre-existing Secret with the companion name is overwritten with sensitive workflow output without proving it belongs to this context. Validate owner references/identity before patching (or fail and choose a safe store) to avoid disclosing data to a pre-created Secret and corrupting it.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/context/context.go, line 310:

<comment>A pre-existing Secret with the companion name is overwritten with sensitive workflow output without proving it belongs to this context. Validate owner references/identity before patching (or fail and choose a safe store) to avoid disclosing data to a pre-created Secret and corrupting it.</comment>

<file context>
@@ -189,6 +244,72 @@ func (wf *WorkflowContext) sync(ctx context.Context) error {
+		}
+		return err
+	}
+	return cli.Patch(ctx, secret, client.MergeFrom(existing.DeepCopy()))
+}
+
</file context>
Suggested change
return cli.Patch(ctx, secret, client.MergeFrom(existing.DeepCopy()))
if !reflect.DeepEqual(existing.OwnerReferences, wf.store.OwnerReferences) {
return fmt.Errorf("sensitive context Secret %s has unexpected owner references", secret.Name)
}
return cli.Patch(ctx, secret, client.MergeFrom(existing.DeepCopy()))
Fix with cubic

Comment thread pkg/context/context.go
func (wf *WorkflowContext) GetVar(paths ...string) (cue.Value, error) {
v := wf.vars.LookupPath(value.FieldPath(paths...))
if !v.Exists() {
if sv := wf.sensitiveVars.LookupPath(value.FieldPath(paths...)); sv.Exists() {

@cubic-dev-ai cubic-dev-ai Bot Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: A sensitive output reusing an existing normal-context name still resolves to, and leaves behind, the ConfigMap value. Clear/migrate that ordinary path when classifying it sensitive and make the sensitive value authoritative, so opt-in updates cannot retain or consume stale plaintext.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/context/context.go, line 83:

<comment>A sensitive output reusing an existing normal-context name still resolves to, and leaves behind, the ConfigMap value. Clear/migrate that ordinary path when classifying it sensitive and make the sensitive value authoritative, so opt-in updates cannot retain or consume stale plaintext.</comment>

<file context>
@@ -56,12 +62,27 @@ type WorkflowContext struct {
 func (wf *WorkflowContext) GetVar(paths ...string) (cue.Value, error) {
 	v := wf.vars.LookupPath(value.FieldPath(paths...))
 	if !v.Exists() {
+		if sv := wf.sensitiveVars.LookupPath(value.FieldPath(paths...)); sv.Exists() {
+			return sv, nil
+		}
</file context>
Fix with cubic

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant