Resolve secrets/vars expressions at dispatch + full detection state in the UI - #33
Conversation
…ction state
Expression resolution: workflows written GitHub-style never received their
secrets - `KEY: ${{ secrets.API_KEY }}` shipped to the container as that
literal string. dispatch now substitutes `${{ secrets.NAME }}` and
`${{ vars.NAME }}` blocks (dot or bracket form, single-ref expressions
only) in plan env values and step run strings: known secrets resolve to
their value, unknown secrets and vars to "" (GitHub's unset semantics).
Substitution lives in dispatch memory only - stored plans keep literals,
reruns pick up rotated values, masks register after substitution, and
secret plaintexts remain unconditional log masks.
Detection visibility: the requirements endpoints now return EVERY detected
(name, repository) pair with configured state and the covering secret's /
matching environment's id, not just missing names. The cards become
repo-grouped "Detected in workflows" lists - configured entries link to
their detail pages for editing, missing ones keep one-click Add/Create,
and reserved-prefix secret refs (DOCKER_*, GITHUB_*) are shown with an
explanation instead of being silently hidden (shared secretNameRules
module). The workflow detail Metadata tab gains Variables referenced and
Environments sections plus a re-sync hint when the stored parse predates
detection, and the pipeline Environment tab shows the job's deployment
environment binding.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
✅ Deploy Preview for overup-app ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughThe change adds configured-state requirement APIs, repository-grouped secret and environment detection UIs, shared secret-name validation, workflow metadata surfaces, and dispatch-time substitution for supported secret and variable expressions. ChangesRequirement detection and dispatch behavior
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)Requirement detection flowsequenceDiagram
participant WorkflowMetadata
participant RequirementsEndpoint
participant RequirementStateQueries
participant DetectionCard
WorkflowMetadata->>RequirementsEndpoint: expose detected references
RequirementsEndpoint->>RequirementStateQueries: query configured states
RequirementStateQueries-->>RequirementsEndpoint: repository and configuration data
RequirementsEndpoint-->>DetectionCard: grouped requirement entries
DetectionCard->>DetectionCard: render links or Add/Create actions
Dispatch substitution flowsequenceDiagram
participant Scheduler
participant SecretStore
participant WorkflowParser
participant JobProtocol
Scheduler->>SecretStore: decrypt referenced secrets
SecretStore-->>Scheduler: resolved secret values
Scheduler->>WorkflowParser: substitute environment and run expressions
WorkflowParser-->>Scheduler: resolved strings
Scheduler->>JobProtocol: submit substituted job
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
There was a problem hiding this comment.
This PR successfully implements dispatch-time expression resolution for secrets and variables, along with comprehensive UI improvements for detection state visibility. The implementation is well-designed with proper security considerations:
Key Strengths:
- Expression substitution is properly isolated to dispatch memory, keeping stored plans as literals
- Comprehensive test coverage for the new
substitute_context_refsfunction (158 tests green) - Secure handling: secrets are registered as log masks before payload dispatch
- Proper separation of concerns between parsing (metadata only) and dispatch (value resolution)
- UI improvements make detection state fully transparent (configured/missing/reserved states)
Architecture:
- The substitution only handles single-ref expressions (
secrets.NAME/vars.NAME), passing through compound expressions untouched - this is the correct security-conscious design - Unknown secrets resolve to empty strings, matching GitHub's behavior
- Reruns automatically pick up rotated secrets since stored plans keep the literals
The code is production-ready with no blocking issues identified.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
Why detection appeared broken (and why pipelines didn't read secrets)
${{ … }}expressions. A GitHub-style workflow withenv: KEY: ${{ secrets.API_KEY }}shipped the literal string${{ secrets.API_KEY }}into the container — the secret only worked if the script read$API_KEYdirectly. GitHub evaluates expressions before execution and resolves unset secrets/vars to empty strings.DOCKER_*,GITHUB_*) silently filtered out, a repo whose refs were all reserved or all configured showed literally nothing.varRefs/environmentsarrived untyped and unrendered), and the pipeline UI never showed the job'senvironment:binding.What this PR does
Dispatch-time expression resolution (
workflow_parse::substitute_context_refs+scheduler::dispatch):${{ secrets.NAME }}/${{ vars.NAME }}(dot or bracket form, single-ref expressions only) resolve in plan env values and steprunstrings — known secrets to their value, unknown secrets and all vars to""(GitHub's unset semantics). Compound expressions and other contexts (github.*,matrix.*, …) pass through untouched: substitutor, not evaluator.Full detection state in the UI:
configured/configuredId(highest-precedence covering secret / matching environment picked in SQL).secretNameRulesmodule, also used by the create dialog).Verification
cargo check,cargo clippy --all-targets -- -D warnings,cargo test— 158 tests green (new substitution tests: dot/bracket, unknown→empty, compound/other contexts untouched, multiple blocks).npm run buildcompiles clean.After deploying
Re-sync the connected repository once (the PARSER_VERSION bump from #32 re-parses stored workflows), then check the workflow detail Metadata tab — the detected secrets/variables/environments appear there, and the Secrets/Environments pages show the repo-grouped detection cards.
🤖 Generated with Claude Code
Summary by CodeRabbit