feat(RELEASE-2476): add Python script for collect-charon-params task

[querti]

Assisted-by: Cursor

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.70%. Comparing base (a0c68a7) to head (b881f0b).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #861      +/-   ##
==========================================
+ Coverage   95.51%   95.70%   +0.18%     
==========================================
  Files          69       73       +4     
  Lines        6893     7198     +305     
==========================================
+ Hits         6584     6889     +305     
  Misses        309      309              

Flag	Coverage Δ
unit-tests	95.70% <100.00%> (+0.18%)	⬆️

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

Files with missing lines	Coverage Δ
...ipts/python/tasks/managed/collect_charon_params.py	100.00% <100.00%> (ø)

... and 4 files with indirect coverage changes

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a0c68a7...b881f0b. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[qodo-app-for-konflux-ci]

PR Summary by Qodo

Add managed task to collect Charon params and emit Tekton results

✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

AI Description

• Add a managed Python task to derive Charon publishing parameters from release inputs.
• Emit a shell env file, Charon config file, and Tekton result values for downstream steps.
• Add comprehensive unit tests covering extraction logic, file outputs, and CLI behavior.

Diagram

graph TD
  W["Work dir"] --> D["data.json"] --> T["collect_charon_params.py"] --> P["CharonParams"] --> O["charon.env + charon-config.yaml"] --> R["Tekton result files"]
  W --> S["snapshot.json"] --> T
  W --> L["release.json"] --> T

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use existing Tekton env/result helpers instead of argparse

• ➕ Aligns with other tasks that read result paths via env (e.g., result_paths_from_env)
• ➕ Less CLI plumbing in Task/Pipeline YAML (fewer explicit args)
• ➖ May require changes to how this specific Tekton task is wired today
• ➖ Less explicit invocation can reduce local reproducibility without a wrapper

2. Emit a single structured JSON result instead of env + config files

• ➕ Downstream steps can parse one canonical artifact; avoids shell quoting pitfalls
• ➕ Easier to extend without adding more result keys/files
• ➖ Requires downstream consumers to parse JSON (vs. sourcing env)
• ➖ May not match existing downstream expectations for env/config file inputs

Recommendation: The current approach is reasonable given existing managed tasks already use argparse and explicit file inputs/outputs. If downstream consumers primarily source env files, keeping charon.env is pragmatic. Consider adopting the shared Tekton helper patterns only if pipeline wiring complexity becomes a recurring issue across similar tasks.

Files changed (2) +736 / -0

Enhancement (1) +203 / -0

collect_charon_params.pyNew managed task to extract and write Charon params + Tekton results +203/-0

New managed task to extract and write Charon params + Tekton results

• Adds a Python CLI task that loads data/snapshot/release JSON files, derives a Charon target identifier and related metadata, and writes a shell-sourceable env file plus a config file. The task also writes Tekton result files containing the generated artifact paths and secret names needed by downstream steps.

scripts/python/tasks/managed/collect_charon_params.py

Tests (1) +533 / -0

test_collect_charon_params.pyUnit tests for Charon param extraction, writers, and CLI/run orchestration +533/-0

Unit tests for Charon param extraction, writers, and CLI/run orchestration

• Introduces comprehensive pytest coverage for parameter extraction (including defaults and missing-key failures), env/config file writing behavior, end-to-end run() outputs, and CLI parsing/main() behavior. Validates both happy paths and error propagation (e.g., missing input files).

scripts/python/tasks/managed/test_collect_charon_params.py

[qodo-app-for-konflux-ci]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 25 rules

1. Tekton result paths from CLI ✓ Resolved 📘 Rule violation ≡ Correctness

Description

The script accepts Tekton result file paths via CLI flags instead of resolving them with the shared
tekton.result_paths_from_env() helper. This can break standard Tekton wiring (results passed as
env vars) and duplicates logic the shared helper is meant to centralize.

Code

scripts/python/tasks/managed/collect_charon_params.py[R163-183]

+    parser.add_argument(
+        "--result-charon-param-file-path",
+        required=True,
+        help="Tekton result file for charon env path",
+    )
+    parser.add_argument(
+        "--result-charon-config-file-path",
+        required=True,
+        help="Tekton result file for charon config path",
+    )
+    parser.add_argument(
+        "--result-charon-aws-secret",
+        required=True,
+        help="Tekton result file for charon AWS secret name",
+    )
+    parser.add_argument(
+        "--result-charon-sign-ca-secret",
+        required=True,
+        help="Tekton result file for charon sign CA secret",
+    )
+    return parser.parse_args(argv)

Relevance

⭐⭐⭐ High

Strong precedent: repo standardizes Tekton result paths via tekton.result_paths_from_env (renamed in
PR783; used in PR795).

PR-#783
PR-#795

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 903 requires resolving Tekton result file paths via
tekton.result_paths_from_env() rather than passing/building paths manually. The new code defines
CLI arguments for Tekton result paths and then converts them to Path(...) and writes to them,
without using the shared helper.

Rule 903: Use helper to resolve Tekton result file paths
scripts/python/tasks/managed/collect_charon_params.py[163-183]
scripts/python/tasks/managed/collect_charon_params.py[186-198]

Agent prompt

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

## Issue description
`scripts/python/tasks/managed/collect_charon_params.py` writes Tekton results but currently takes the Tekton result file paths via CLI flags (e.g., `--result-charon-param-file-path`). The compliance requirement is to resolve Tekton result file paths using the shared helper `tekton.result_paths_from_env()` and then use those returned `Path`s directly.

## Issue Context
The repository already provides `scripts/python/helpers/tekton.py` with `result_paths_from_env(...)` for the standard Tekton contract (result paths provided via environment variables). This new script should follow that same convention.

## Fix Focus Areas
- scripts/python/tasks/managed/collect_charon_params.py[163-183]
- scripts/python/tasks/managed/collect_charon_params.py[186-198]

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

---

2. Unsafe env export quoting 🐞 Bug ⛨ Security

Description

write_charon_env() writes a shell-sourceable env file but interpolates values without shell escaping
(and leaves CHARON_TARGET unquoted), so special characters (quotes, newlines, command substitutions)
can break sourcing or change shell parsing. This is a potential injection vector if downstream steps
source the file.

Code

scripts/python/tasks/managed/collect_charon_params.py[R80-91]

+def write_charon_env(env_path: Path, params: CharonParams) -> None:
+    """Write charon parameters as a shell-sourceable env file."""
+    lines = [
+        f"export CHARON_TARGET={params.target}",
+        f'export CHARON_PRODUCT_NAME="{params.product_name}"',
+        f'export CHARON_PRODUCT_VERSION="{params.product_version}"',
+    ]
+    if params.sign_key:
+        lines.append(f'export CHARON_SIGN_KEY="{params.sign_key}"')
+    lines.append(f'export CHARON_OCI_REGISTRY="{params.oci_registry}"')
+    lines.append(f'export CHARON_AUTHOR="{params.author}"')
+    env_path.write_text("\n".join(lines) + "\n", encoding="utf-8")

Relevance

⭐⭐ Medium

No clear review precedent found for shell-escaping env exports; only indirect pattern mention
(sign_mac.py).

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The script explicitly states it writes a “shell-sourceable env file” and then writes unescaped
f-string exports, including an unquoted CHARON_TARGET. In other parts of this repo, shell exports
are emitted using shlex.quote() to ensure safe shell parsing.

scripts/python/tasks/managed/collect_charon_params.py[4-6]
scripts/python/tasks/managed/collect_charon_params.py[80-91]
scripts/python/helpers/sign_mac.py[80-90]

Agent prompt

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

### Issue description
`write_charon_env()` generates a file intended to be sourced by a shell, but it writes `export` lines using raw string interpolation. This is unsafe/fragile because values are not shell-escaped, and `CHARON_TARGET` is not quoted at all.

### Issue Context
The module docstring and function docstring explicitly describe the file as “shell-sourceable”, so the content must be safe to `source`.

### Fix Focus Areas
- scripts/python/tasks/managed/collect_charon_params.py[1-7]
- scripts/python/tasks/managed/collect_charon_params.py[80-91]

### Suggested fix
- Import `shlex` and use `shlex.quote()` for *every* exported value, e.g.:
 - `export CHARON_TARGET={shlex.quote(params.target)}`
 - `export CHARON_PRODUCT_NAME={shlex.quote(params.product_name)}`
 - `export CHARON_PRODUCT_VERSION={shlex.quote(params.product_version)}`
 - `export CHARON_SIGN_KEY={shlex.quote(params.sign_key)}` (when present)
 - `export CHARON_OCI_REGISTRY={shlex.quote(params.oci_registry)}`
 - `export CHARON_AUTHOR={shlex.quote(params.author)}`
- Remove the current ad-hoc `"..."` quoting and rely on `shlex.quote()` consistently to avoid malformed shell syntax and injection.

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

---

3. Workdir path traversal 🐞 Bug ⛨ Security

Description

run() treats --data-json-path/--snapshot-path/--release-path as “relative” but joins them into
filesystem paths without rejecting absolute paths or '..', so it can read/write outside work_dir.
This can lead to unintended file access and writing env/config files outside the workspace.

Code

scripts/python/tasks/managed/collect_charon_params.py[R118-128]

+    data = load_json_dict(work_dir / data_json_path)
+    snapshot = load_json_dict(work_dir / snapshot_path)
+    release_data = load_json_dict(work_dir / release_path)
+
+    params = collect_charon_params(data, snapshot, release_data)
+
+    env_rel = str(Path(data_json_path).parent / "charon.env")
+    cfg_rel = str(Path(data_json_path).parent / "charon-config.yaml")
+
+    write_charon_env(work_dir / env_rel, params)
+    write_charon_config(work_dir / cfg_rel, params.config)

Relevance

⭐⭐ Medium

Repo has raised traversal-sanitization concerns before, but acceptance outcome unclear (origin path
hardening suggestion in PR795).

PR-#795

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The script documents the JSON paths as relative, but run() directly joins them with work_dir and
also uses data_json_path to compute output paths, with no checks for absolute paths or '..'.
Elsewhere in this repo, similar “path under root” operations explicitly reject absolute and
traversal paths before joining.

scripts/python/tasks/managed/collect_charon_params.py[118-128]
scripts/python/tasks/managed/collect_charon_params.py[149-162]
scripts/python/tasks/internal/process_file_updates.py[349-364]

Agent prompt

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

### Issue description
`run()` assumes several CLI paths are relative to `--work-dir`, but it never enforces that assumption. Because it directly joins `work_dir / <user_path>` and also derives output locations from `data_json_path`, callers can supply absolute paths or `..` segments and cause reads/writes outside `work_dir`.

### Issue Context
The CLI help text explicitly says these inputs are **relative paths**, so we should validate and enforce that contract.

### Fix Focus Areas
- scripts/python/tasks/managed/collect_charon_params.py[107-128]
- scripts/python/tasks/managed/collect_charon_params.py[138-183]

### Suggested fix
- Add a small validator like `def require_relative(path_str: str, *, arg_name: str) -> Path:` that rejects:
 - empty/whitespace
 - absolute paths (`Path(...).is_absolute()`)
 - Windows drive paths (`len(s) >= 2 and s[1] == ':'`)
 - any `".."` in `Path(...).parts`
- Use it for `data_json_path`, `snapshot_path`, and `release_path` before doing `work_dir / ...`.
- (Optional hardening) After constructing the final path, resolve it and ensure it stays under `work_dir.resolve()` (similar to `resolve_target_file()` pattern used elsewhere in the repo).

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

---

[theflockers]

maybe add a try/except as this might cause a KeyError if containerImage is missing (for whatever reason) and logging the error.

[querti]

Added!

[elenagerman]

Bare raise lost the exception chain; AGENTS.md requires raise ... from e

Suggested change

	logger.error("One or more components are missing the 'containerImage' key")
	raise
	msg = "One or more components are missing the 'containerImage' key"
	logger.error(msg)
	raise KeyError(msg) from e

[elenagerman]

Values written without proper escaping could break shell parsing or allow injection; you can use shlex.quote to solve it