Fix/pass secret when execution

[hungz]

fix issue #3994

Summary by CodeRabbit

• Bug Fixes

  • Survey secret values are now consistently included in task environment variables for both shell execution and JSON extra-vars output.
  • Fixed an issue where secret values could be cleared during processing, which could lead to missing environment data in later steps.

• Tests

  • Added coverage for secret survey variables being passed through with standard environment variables.
  • Added coverage for merging secret values into the shared extra-vars output.

[Copilot]

Pull request overview

Fixes issue #3994 where Survey variables of type Secret were not being passed into task execution for Shell/Bash (and other non-Ansible) runs by merging LocalExecutor.Secret into the same extra-vars map used to build runtime arguments.

Changes:

• Merge Survey Secret variables (from LocalExecutor.Secret) into the computed extra vars map in getEnvironmentExtraVars.
• Refactor getEnvironmentExtraVarsJSON to reuse getEnvironmentExtraVars to avoid duplicated parsing/merge logic.
• Add unit tests covering secret var propagation into Shell args and extra vars merging.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
services/tasks/local_executor.go	Merges survey secret vars into extra vars so Shell/Terraform paths receive them; deduplicates extra-vars JSON generation.
services/tasks/local_executor_test.go	Adds regression tests ensuring survey secret vars are included for Shell args and extra vars.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: add31211-e800-4176-8426-714b61fd7dea

📥 Commits

Reviewing files that changed from the base of the PR and between 5902d9c and 1a42710.

📒 Files selected for processing (2)

• services/tasks/local_executor.go
• services/tasks/local_executor_test.go

---

📝 Walkthrough

Walkthrough

LocalExecutor's getEnvironmentExtraVars now parses and merges Secret JSON into the extra vars map, and getEnvironmentExtraVarsJSON delegates to it instead of duplicating parsing logic, removing the prior side-effect of clearing t.Secret. New unit tests validate secret merging in shell args and extra vars.

Changes

Secret merging in LocalExecutor

Layer / File(s)	Summary
Merge Secret into extra vars and simplify JSON path services/tasks/local_executor.go	getEnvironmentExtraVars parses t.Secret JSON and merges it into extraVars via maps.Copy; getEnvironmentExtraVarsJSON delegates to it and no longer clears t.Secret.
Tests validating secret merging services/tasks/local_executor_test.go	Adds setupExecutorConfig helper and tests confirming secret vars appear in getShellArgs output and are merged by getEnvironmentExtraVars.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant getEnvironmentExtraVarsJSON
  participant getEnvironmentExtraVars

  Caller->>getEnvironmentExtraVarsJSON: request extra vars JSON
  getEnvironmentExtraVarsJSON->>getEnvironmentExtraVars: delegate call
  getEnvironmentExtraVars->>getEnvironmentExtraVars: parse Environment.JSON
  getEnvironmentExtraVars->>getEnvironmentExtraVars: parse t.Secret JSON
  getEnvironmentExtraVars->>getEnvironmentExtraVars: maps.Copy Secret into extraVars
  getEnvironmentExtraVars-->>getEnvironmentExtraVarsJSON: return merged extraVars
  getEnvironmentExtraVarsJSON-->>Caller: return marshaled JSON

Loading

Related Issues: None specified.

Related PRs: None specified.

Suggested labels: go, tests, bug-fix

Suggested reviewers: semaphoreui maintainers familiar with task execution services

🐇 A secret hopped in, quiet and small,
Merged with its friends, one map for all,
No more clearing what should have stayed,
Now tests confirm the merge is made,
Hop hop hooray, the vars align!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is related to the main change: passing secret values during execution.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}