[sameerforge]

What this PR does / why we need it:

Sanitize URL queries/fragments and error fields before presenting command results. Keep the original rendered manifest separate from its diagnostic copy. Omit opaque ytt diagnostics when Secret-backed values are supplied and provide corrective guidance. In kctrl, display resource identities instead of complete App/PackageInstall objects, omit vendir configuration-file printing and stop teeing raw child output into debug logs.

Validation

• Controller: go test ./pkg/exec ./pkg/template ./pkg/app ./pkg/pkgrepository with the baseline ytt/vendir tools on PATH.
• CLI: cd cli && go test ./pkg/....
• Controller: golangci-lint run passes.
• CLI: golangci-lint run reports 25 existing findings, reproduced on unchanged develop; golangci-lint run --new-from-rev upstream/develop passes.
• The broader controller go test ./pkg/... has two existing failures in pkg/config and pkg/fetch, also reproduced on unchanged develop.
• New tests cover Secret-backed ytt errors, original rendered output, all command-result diagnostic fields, resource/configuration display and normal/cancelable subprocess execution.

Does this PR introduce a user-facing change?

Controller status and kctrl diagnostics avoid exposing credentials and configuration bodies. Secret-backed ytt failures provide safe corrective guidance; child output remains available to its intended consumer.

[joaopapereira]

Issues

Critical (must fix)

1. urlQuery regex misses git+ssh and bare SSH schemes — pkg/exec/cmd_run_result.go:121 — The regex ([a-zA-Z][a-zA-Z0-9+.-]*://[^\s?\#]+[?\#])[^\s"'<>]+ requires :// so it does not match ssh://user@host/path?query=secret (matches userinfo via urlUserinfo but not query), git+ssh://host/path?token=secret, or ssh://host/path#frag=secret. The urlUserinfo regex at line 122 handles ssh://[^\s/@]+@ for userinfo but there is no corresponding query/fragment pattern for ssh:// or git+ssh:// schemes. A URL like ssh://git@example.com/repo.git?token=abc would have userinfo redacted but ?token=abc left intact. Fix: extend urlQuery to also match ssh:// and git+ssh:// schemes, or add a separate regex for them.

2. Secret-backed ytt path drops Stderr entirely even on success — pkg/template/ytt.go:94-98 — When source.SecretRef != nil and result.Stderr != "", the code unconditionally replaces Stderr with the guidance message, losing any legitimate warnings (e.g., deprecation notices) that are not secret-related. The PR description says "Omit opaque ytt diagnostics when Secret-backed values are supplied" but the implementation omits all stderr. Fix: only redact/replace stderr when it actually contains secret material, or preserve non-secret lines. At minimum, document the trade-off in a comment.

3. DetailedCmdRunner drops command args from debug log — cli/pkg/kctrl/local/detailed_cmd_runner.go:33,48 — Changed from fmt.Fprintf(r.log, "==> Executing %s %v\n", cmd.Path, cmd.Args) to fmt.Fprintf(r.log, "==> Executing %s\n", cmd.Path). Args can contain non-secret context (e.g., sync -f vendir.yml) useful for debugging. The PR description says "stop teeing raw child output into debug logs" but this also removes args. Fix: log args with any sensitive values redacted, or log a redacted args summary.

Important (should fix)

4. urlQuery regex may over-redact / under-redact in complex URLs — pkg/exec/cmd_run_result.go:121,132-134 — The pattern ([a-zA-Z][a-zA-Z0-9+.-]*://[^\s?\#]+[?\#])[^\s"'<>]+ captures from the first ? or # to the next whitespace/quote/angle-bracket. This works for simple cases but: (a) in https://host/path?token=abc&other=def it redacts token=abc&other=def (correct); (b) in https://host/path?token=abc https://other.com it redacts token=abc (correct); (c) in https://host/path?token=abc#frag it redacts token=abc#frag (correct); (d) but it does not handle multiple query params where only some are sensitive — it redacts the entire query string. This is a reasonable safe default but worth noting. No code change required, but consider adding a test for multi-param queries.

5. Vendir sync error wraps original error but loses stderr context — cli/pkg/kctrl/local/sources/vendir_runner.go:52 — Changed from return fmt.Errorf("Running vendir sync: %s", stderrBs.String()) to return fmt.Errorf("running vendir sync failed; check the source configuration and authentication: %w", err). The original stderr (which could contain non-secret error details like network errors) is dropped. The wrapped err from cmd.Run() only contains the exit status, not stderr. Fix: include a redacted stderr in the error message, or log it separately at a non-debug level.

6. Test coverage gap: fetch result redaction in pkgrepository — pkg/pkgrepository/app_reconcile.go:148 uses fetchResult.WithRedactedSecrets() but there is no test verifying fetch diagnostics (which can contain URLs with credentials from vendir/httpproxy sources) are redacted. The existing test failures in pkg/pkgrepository are pre-existing (missing vendir binary). Add a test in pkg/pkgrepository or pkg/exec covering fetch redaction.

Minor (nice to have)

7. RedactSecrets idempotence comment could be explicit about query redaction — pkg/exec/cmd_run_result.go:127 — The comment says "RedactSecrets is idempotent: calling it multiple times produces the same result." This is true but the query/fragment redaction uses ${1}[REDACTED] which removes the original query/fragment entirely; a second pass has nothing to match, so it is idempotent. Consider adding a test case for double-redaction with query params.

8. Inconsistent truncation: pkgrepository fetch lacks WithTruncatedStrings — pkg/pkgrepository/app_reconcile.go:148 vs pkg/app/app_reconcile.go:148 — The App controller truncates fetch output with .WithTruncatedStrings(a.maxOutputBytes()) but PackageRepository does not. This is a pre-existing inconsistency, not introduced by the PR, but the PR touches both files. Consider aligning in a follow-up.