[pshickeydev]

Summary

• Replace generated HTTP shell workflows with structured, engagement-scoped probes, including exact Route grants, controlled node discovery, owned Service/Pod tunnels, session checks, and evidence redaction.
• Bind safe-exec hosts to each run, preserve precise lease expiry across validation, browser, and crypto consumers, and refuse unsupported curl replay rather than substituting a different request.
• Address the agentic review: fail HTTP transport errors inconclusively, preserve safe spoof evidence, enforce destructive opt-in for uncompensated HTTP writes, distinguish command refusal from process exit 126, and preserve exact browser report expiry.
• Adapt affected consumer calls to the current Core APIs. This supersedes the applicable work in traust #30.

Dependencies and review hold

Draft for review; do not merge as-is. The temporary dependency now points to commit e40d132d4f1617dbebd67e1dd9b449797c7b4256 through the organization-owned Core repository's PR ref, not the author's fork. It depends on Core #395, contracts #126, and the Core remediation cutover. Repin to the merged upstream Core revision and regenerate the lockfile before merge. Explicit insecure TLS is restricted to approved lab policy and can expose credentials to an interceptor.

Verification

• Focused consumer tests: 622 passed against the coordinated Core worktree; 388 write/recording tests passed again after Core's destructive-classification enforcement.
• Local HTTP smoke used real oc kubeconfig lookup and curl against a local server: an explicitly granted namespace-less host was reachable, the spoof canary remained visible, and an ungranted host was refused.
• Installed the exact locked revision from the organization Core repository in an isolated target and confirmed direct HTTP scope succeeds without granting Kubernetes cluster-wide access.
• Prior full consumer suite on the earlier revision: 3,095 passed, 169 skipped. No authorized live-cluster operator/core acceptance was performed.

This pull request description was generated with AI assistance.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pshickeydev

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [pshickeydev]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[pshickeydev]

/agentic_review

[qodo-code-review]

Code Review by Qodo

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

1. Builds still depend on a review fork 🐞 Bug ⛨ Security

Description

pyproject.toml sources traust-core from a personal review fork instead of the organization
repository, and uv.lock resolves that fork for the package and its extras. If this draft merges
without the stated repin, installs and CI fetch security-critical Core policy code, including
safe-exec and scope decisions, from the individual's fork rather than the intended upstream
revision.

Code

pyproject.toml[36]

+traust-core = { git = "https://github.com/pshickeydev/traust-core.git", rev = "3553b1b16020aed8bc04630462e8926bf277c085" }

Evidence

The dependency declaration and resolved lock entries both point to the personal review fork.
Although the source is commit-locked, its provenance remains the individual's repository; the PR
comment and description explicitly call for repinning to upstream before merge.

pyproject.toml[34-36]
uv.lock[1575-1577]
uv.lock[1598-1601]

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 dependency declaration and lockfile still install `traust-core` from a personal review fork instead of the organization repository.

## Fix Focus Areas
- pyproject.toml[34-36]
- uv.lock[1575-1577]
- uv.lock[1601-1601]

## Recommended Fix
After Core #395 and the companion changes merge, point the source to `https://github.com/traust-security/traust-core.git` at the reviewed merged commit SHA and regenerate `uv.lock`. Verify the resolved source and revision for the base package and extras before lifting the merge hold.

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

Dismiss ↗ | View ↗

---

2. Failed HTTP probes can confirm findings ✓ Resolved

Resolving commit: ce4d5bf

Resolution reason

Status is parsed from the curl-owned stdout trailer before stderr is appended; rc!=0 or missing/000
status forces inconclusive before expected-token matching, so diagnostics cannot confirm a finding.

Description

The new HttpExecutor path returns curl’s write-out status on stdout and diagnostics on stderr,
which the adapter concatenates before calling the end-anchored status parser. When a failed request
emits vf-http-status:000 followed by a diagnostic matching the expected text, the parser misses
000 and its fallback token matcher can return confirmed.

---

3. Spoof probes on auth-proxy headers always fail ✓ Resolved

Resolving commit: ce4d5bf

Resolution reason

In the PR head plan.py, _spoof_adapted_step now builds the header map via
HttpProbe.model_validate({**s.target["http"], "headers": ...}). On ValueError it returns a Step with
technique="skip" and skip="spoof-header-adapter-owned" instead of an unvalidated probe, so
auth-proxy header names are never emitted as a structured probe that fails at execution. The test
test_adapter_owned_spoof_headers_are_explicitly_skipped covers X-Auth-Request-User,
X-Forwarded-Access-Token and X-Api-Key. The fallback is an explicit skip rather than the plain HTTP
step the suggestion mentions, but the reported issue is resolved.

Description

_spoof_adapted_step copies every X-... header named in the finding into target.http.headers
after the plan-time HttpProbe has been built. The executor-time HttpProbe.valid_headers rejects
any name containing auth, token, session, key and similar. As a result, findings about
spoofing X-Auth-Request-User, X-Forwarded-Access-Token or X-Api-Key always produce a plan step
that ends as inconclusive "invalid structured HTTP probe", and the plain HTTP adapted step that used
to be emitted is now skipped because the spoof builder runs first.

---

4. Mutating probes leave target changes ✓ Resolved

Resolving commit: ce4d5bf

Resolution reason

Structured HTTP probes with non-read methods are forced to classification "destructive" in
execute.run(), so they require the destructive opt-in; plan.py also forces manual HTTP writes to
destructive. Writes without opt-in are not attempted.

Description

execute.run() classifies HTTP POST, PUT and PATCH as mutating but categorically excludes
structured HTTP steps from rollback. Because the structured-step checks also reject rollback text,
an authorized write can change the target without a compensating action or destructive opt-in.

---

5. Current-context cluster steps are denied 🐞 Bug ≡ Correctness

Description

Scope.is_in_scope() passes Action.context=None to Core even after its own lookup has resolved
that action against the __current__ cluster. Namespace-only inline runs and plans that omit a
context therefore reach Core with a target context that does not match the registered cluster.

Code

harnessing/5-validate/validate-findings/scope.py[R493-497]

+            decision = self._core_scope().decide(
+                ValidationAction(kind=ValidationActionKind.K8S, verb=a.verb),
+                ValidationTarget(
+                    context=a.context,
+                    namespace=a.namespace,

Evidence

Inline namespace selection creates __current__, the legacy lookup uses it for missing contexts,
and the new decision instead forwards the original None.

harnessing/5-validate/validate-findings/scope.py[317-326]
harnessing/5-validate/validate-findings/scope.py[406-417]
harnessing/5-validate/validate-findings/scope.py[482-503]
harnessing/5-validate/validate-findings/execute.py[79-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
The new Core scope decision receives a missing context for actions already resolved against `__current__`.
## Fix Focus Areas
- harnessing/5-validate/validate-findings/scope.py[482-503]
## Recommended Fix
Pass the resolved cluster context to `ValidationTarget`, while preserving the existing current-context restrictions. Cover a namespace-only inline scope and a context-less Kubernetes plan target.

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

Dismiss ↗ | View ↗

---

View medium (5)

6. One stray Route blocks probes for a namespace 🐞 Bug ≡ Correctness

Description

When EndpointResolver.route lists every Route in a namespace (no name and no hint), it raises
PermissionError("discovered route destination is not authorized") on the first Route whose host
matches no exact target or grant domain. It does not skip that Route. Any unrelated Route with a
custom or external host therefore makes every name-less Route probe in that namespace
blocked_by_scope, even when exactly one authorized Route exists.

Code

harnessing/5-validate/validate-findings/http_endpoints.py[R468-474]

+                    credentials = self.explicit(host, context, namespace, "routes", route_name)
+                    if credentials is None:
+                        matching = [g for g in grants if g.accepts(route_name, host)]
+                        if matching:
+                            credentials = any(g.credentials for g in matching)
+                    if credentials is None:
+                        raise PermissionError("discovered route destination is not authorized")

Evidence

With a grant that has no names, names collapses to {None} and objects() lists all Routes.
Inside the loop, an unauthorized host raises instead of continue. By contrast, Routes with a
non-matching origin or hint are skipped. The adapted planner emits name-less route probes, so every
such probe in the namespace is affected.

harnessing/5-validate/validate-findings/http_endpoints.py[439-490]
harnessing/5-validate/validate-findings/plan.py[1148-1160]

Agent prompt

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

## Issue description
Route discovery by listing aborts the whole resolution on the first Route whose host is not granted, instead of excluding it from the candidates.

## Fix Focus Areas
- harnessing/5-validate/validate-findings/http_endpoints.py[468-474]

## Recommended Fix
When `name is None` (listing), `continue` past Routes with `credentials is None`. Keep the `PermissionError` only when a specific Route name was requested. If no authorized candidates remain, raise PermissionError so the outcome is still `blocked_by_scope`. Add a test with two Routes in one namespace, one in the approved domain and one outside it.

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

Dismiss ↗ | View ↗

---

7. Spoof probe evidence hides the reflected canary ✓ Resolved

Resolving commit: ce4d5bf

Resolution reason

At PR head, adapters/http.py line 334 builds secrets as `[value for value in headers.values() if
value and value != "vf-spoof-canary"]`, so the fixed spoof marker is excluded from redaction.
Non-secret probe header values are no longer redacted wholesale, and the status trailer is extracted
before redaction (line 439). The suggested fix (start secrets empty, add only token and cookies) is
not followed exactly: non-canary header values are still treated as secrets, so only the canary case
is addressed. The reported canary-erasure symptom is resolved, which is the issue the finding
describes.

Description

HttpExecutor.execute adds every probe header value to secrets (`secrets = [value for value in
headers.values() if value]) and replaces each one with [REDACTED]` across the whole response. The
header-spoof steps from _spoof_adapted_step carry only the non-secret value vf-spoof-canary, so
a reflected or honoured canary shows up in stored evidence as [REDACTED], the same as a redacted
credential. Any short header value (for example 1) would also be removed from every occurrence in
the body and from the vf-http-status: trailer that _verdict parses.

---

8. Explicit direct probes are denied ✓ Resolved

Resolving commit: ce4d5bf

Resolution reason

In scope.py is_in_scope(), a new direct_http flag (a.resource == "http_targets" and a.namespace is
None and a.url is not None) bypasses cs.ns_allowed() for direct HTTP actions (the namespace check is
guarded by "not direct_http"). Instead, direct HTTP actions are authorized via a dedicated path:
they require http_context_allowed(a.context), are checked against verbs_denied, and are decided by
the Core ValidationScope.decide() with ValidationActionKind.HTTP and the URL as target. The
control-plane lock is also skipped for namespace-less cluster-scoped HTTP actions only when granted
via _http_node_granted. Tests in test_scope.py
(test_namespace_less_direct_action_uses_core_http_policy,
test_direct_action_permits_only_granted_http_verbs) and test_http_endpoints.py
(test_direct_probe_matches_core_approval_and_binding with namespace=None) cover the regression with
a namespace-scoped cluster.

Description

EndpointResolver.resolve() creates namespace-less direct endpoints, but the new
Scope.is_in_scope() gate treats their missing namespace as a cluster-wide Kubernetes action. With
a normal namespace-scoped engagement, even a GET to an explicitly granted direct host fails unless
the cluster also grants "*".

---

9. In-pod exit code 126 is reported as a scope block ✓ Resolved

Resolving commit: ce4d5bf

Resolution reason

The PR head no longer uses the rc == 126 heuristic. Vetting refusals are now signalled explicitly:
AdapterBase._run returns a BlockedCommand (a str subclass) for safe_exec vetting failures (base.py,
_run), and K8sAdapter.execute checks isinstance(err, BlockedCommand) to set verdict
"blocked_by_scope" with scope_reason=err.reason. All other results, including rc 126 from a process,
go through self._verdict(...). Exec tools fallback and raw fallback also skip BlockedCommand
results. This is a different implementation from the suggested sentinel-based fix, but it addresses
the same issue.

Description

K8sAdapter.execute treats every exit code 126 as a policy refusal, sets the verdict to
blocked_by_scope, and hard-codes the HTTP scope_reason for every verb. When an exec, raw,
rbac-can-i, or tools-pod command genuinely exits 126—including an in-container command that fails
with “Permission denied”—that process result reaches the policy mapping instead of _verdict, which
previously classified the exec permission failure as refuted.

---

10. Transient service refusal ends the probe 🐞 Bug ☼ Reliability

Description

The replacement HTTP execution path makes one request and removes the previous connection-refused
retries. A Running Pod and ready port-forward can still precede the application listener, so that
temporary refusal now leaves the validation inconclusive without a retry.

Code

harnessing/5-validate/validate-findings/adapters/k8s.py[R329-332]

+            try:
+                rc, out, err = HttpExecutor(self, scope, kb, audit).execute(
+                    target.get("context"), probe
+                )

Evidence

The new path executes once, while tunnel readiness checks Pod state rather than application
readiness; the diff removes two retries with backoff.

harnessing/5-validate/validate-findings/adapters/k8s.py[329-348]
harnessing/5-validate/validate-findings/adapters/http.py[58-70]
harnessing/5-validate/validate-findings/adapters/http.py[387-400]

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 structured service probe loses the previous bounded retry for a listener that starts after its port-forward.
## Fix Focus Areas
- harnessing/5-validate/validate-findings/adapters/k8s.py[329-348]
- harnessing/5-validate/validate-findings/adapters/http.py[387-400]
## Recommended Fix
Restore a bounded retry for demonstrably transient connection failures on safe, idempotent probes, rechecking the lease and tunnel before each attempt. Do not automatically replay mutating HTTP methods; cover delayed listener readiness.

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

Dismiss ↗ | View ↗

---

11. Browser report puts a datetime in the date field ✓ Resolved

Resolving commit: ce4d5bf

Resolution reason

The PR's current browser run.py ReportBuilder.build now computes metadata.expires as
(self._scope.expires or self._scope.expires_at.date()).isoformat(), so metadata.expires is always a
date-only value. The precise instant is preserved under
metadata.additional.validation_producer.expires_at. This matches the findings report's date-only
convention. The Targets.from_file split also means expires is set to the date when a datetime is
given (via parse_expiry in scope code), and the browser run.py keeps expires_at separately. The test
test_browser_report_preserves_date_and_exact_expiry confirms metadata.expires == "2099-01-02" for a
datetime lease.

Description

The browser ValidationReport.build now writes (expires_at or expires).isoformat() into
metadata.expires, so a lease timestamp lands in that field as a full datetime. The findings report
keeps metadata.expires date-only and puts the instant under
metadata.additional.validation_producer.expires_at, so the two reports disagree and any consumer
expecting a date in metadata.expires gets a timestamp.

---

Context sources

✅ Linked PRs:

  🌐 PR #30

  🌐 PR #126

❌ Tickets: failed to load — Ticket collection could not establish complete evidence

Review mode: Auto: 🧠 Deep: Dense, security-critical new HTTP probe logic across many independent paths; redundancy valuable.

Tip of the day

💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[pshickeydev]

Qodo follow-up: ce4d5bf addresses the reported HTTP transport verdict, direct-host scope, current-context, bounded safe retry, uncompensated-write, exit-126, spoof evidence/planning, route-listing, and browser expiry findings. Credential-like header values remain redacted; only the fixed synthetic spoof marker is retained, and it is still redacted if it is also a credential. Unsupported spoof headers are explicitly skipped rather than replaced with a different probe.

The fork-pin commit a653b3c was removed from this branch's history. The draft now resolves the reviewed Core revision from the organization repository; the merged-upstream repin and other coordinated holds remain.

Verification: 622 focused tests passed, plus 388 write/recording tests after Core tightened approval. Local real-oc/curl smoke reached an explicitly granted namespace-less target and rejected an ungranted host. No live cluster was probed.

This comment was generated with AI assistance.

[pshickeydev]

Additional verification: the complete consumer suite passed: 3,256 passed, 169 skipped. It started before the final Core write-classification change; the 388 focused HTTP/write/recording tests were rerun successfully after that change. Git signing configuration was neutralized only for the test run's throwaway repositories.

This comment was generated with AI assistance.