Skip to content

Fix shell interpolation in task reports - #287

Merged
robnester-rh merged 1 commit into
conforma:mainfrom
robnester-rh:EC-2048
Oct 5, 2026
Merged

robnester-rh merged 1 commit into
conforma:mainfrom
robnester-rh:EC-2048

Conversation

@robnester-rh

Copy link
Copy Markdown
Contributor

Pass HOMEDIR through the environment before formatting report output so parameter values are not inserted into shell command text in either published task variant.

Ref: EC-2048

Pass `HOMEDIR` through the environment before formatting report output
so parameter values are not inserted into shell command text in either
published task variant.

Co-Authored-By: Codex <codex@openai.com>
Ref: EC-2048
@robnester-rh
robnester-rh requested a review from a team as a code owner October 2, 2026 14:24
@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 4c249da3-4807-430a-b1ea-e551673ce9da

📥 Commits

Reviewing files that changed from the base of the PR and between 8df20d1 and 6b30625.

📒 Files selected for processing (2)
  • tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml
  • tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

Both verification tasks now expose HOMEDIR to the report-json step and use it to locate the JSON report. JSON formatting and line wrapping remain unchanged.

Changes

Report JSON path handling

Layer / File(s) Summary
Use HOMEDIR in report-json
tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml, tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml
Both report-json steps set HOMEDIR from the task parameter and use it to locate the report file. Formatting and line wrapping remain unchanged.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: jsmid1, joejstuart

Merge Risk: ⚪ Minimal · up to 6b306

Both task variants preserve the report path when HOMEDIR contains spaces. No actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: fixing shell interpolation in task reports.
Description check ✅ Passed The description accurately explains passing HOMEDIR through the environment in both task variants and references the related issue.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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 a new PR

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

@qodo-for-conforma

Copy link
Copy Markdown

PR Summary by Qodo

Fix shell interpolation in task report output

🐞 Bug fix 🕐 Less than 10 minutes

Grey Divider

AI Description

• Pass HOMEDIR through the environment in both published task variants.
• Quote the expanded report path so parameter values cannot alter the shell command.
Diagram

graph TD
  P["HOMEDIR parameter"] --> E["Step environment"] --> S["Shell expansion"] --> R["JSON report"] --> J["jq formatting"] --> A["awk wrapping"] --> L["Step logs"]
Loading
High-Level Assessment

Keep the environment-variable approach: it prevents Tekton parameter substitution from placing HOMEDIR directly in shell command text while preserving the existing report pipeline. Direct substitution with additional quoting would not provide the same separation.

Files changed (2) +8 / -2

Bug fix (2) +8 / -2
verify-conforma-konflux-ta.yamlSafely expand the Konflux task report path +4/-1

Safely expand the Konflux task report path

• Passes HOMEDIR to the report-json step as an environment variable and uses a quoted shell expansion when reading the JSON report. The existing jq and awk formatting remains unchanged.

tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml

verify-enterprise-contract.yamlSafely expand the enterprise-contract task report path +4/-1

Safely expand the enterprise-contract task report path

• Applies the same HOMEDIR environment-variable and quoted-path change to the report-json step, avoiding direct parameter substitution in its shell command.

tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml

@qodo-for-conforma

qodo-for-conforma Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (1) 📜 Skill insights (0)

Grey Divider


Action required

1. Task sync restores unsafe interpolation 🔗 Cross-repo conflict ⛨ Security
Description
Both report-json steps are changed only in the catalog; conforma/cli still publishes the source
tasks with $(params.HOMEDIR) inserted into shell command text. The catalog’s CLI sync will propose
reverting this fix on main, and infra-deployments-ci will overwrite the konflux branch from
the unchanged CLI bundle during promotion.
Code

tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml[584]

+        - "jq . \"${HOMEDIR}/report-json.json\" | awk '{gsub(/^ +/, \"\"); acc += length; if (acc >= 8000) { printf \"\\n\"; acc=length } printf $0 }'"
Evidence
The PR changes both catalog report commands, while the CLI source retains the old commands. The
catalog sync copies CLI tasks over catalog tasks, and bundle promotion copies extracted definitions
directly onto the catalog’s konflux branch.

tekton-catalog -> cli
tekton-catalog -> infra-deployments-ci
tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml[574-584]
tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml[478-488]
hack/sync-ec-cli-tasks.sh[57-68]
External repo: conforma/cli, tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml [569-581]
External repo: conforma/cli, tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml [475-485]
External repo: conforma/cli, .github/workflows/release.yaml [149-155]
External repo: conforma/infra-deployments-ci, .github/workflows/konflux-policy.yaml [291-313]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The catalog-only report fix will be replaced by task definitions published from conforma/cli.
## Fix Focus Areas
- /cross_repos/cli/tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml[569-581]
- /cross_repos/cli/tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml[475-485]
- tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml[574-584]
- tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml[478-488]
## Recommended Fix
Apply the same environment-based HOMEDIR handling to both canonical tasks in conforma/cli, then synchronize the catalog and its promoted bundle definitions.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗


Grey Divider

Context sources
✅ Cross-repo context — repo relationships
  Explored: repo: konflux-ci/release-service-catalog (sha: 34c17022) — View relationship
  Explored: repo: conforma/infra-deployments-ci (sha: 0fff65f3) — View relationship
  Explored: repo: conforma/cli (sha: 286f4573) — View relationship
Review mode: ⚖️ Balanced: This is a runtime task configuration change affecting shell command construction in two published variants, so it has behavioral and injection-related risk despite the small diff.

Grey Divider

Tip of the day
💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@robnester-rh
robnester-rh merged commit 03ed3ec into conforma:main Oct 5, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants