Validate then parameter in grant handler to prevent open redirects

[yuvalk]

Summary

• Validate the then URL parameter with IsServerRelativeURL in the grant handler's POST path (handleGrant) to block redirects to external sites after grant approve/deny
• Add defense-in-depth validation in the GET path (handleForm) to reject malicious then values before they are embedded in the HTML form
• Add 4 test cases covering absolute URL and protocol-relative URL redirect attempts

Test plan

• All 21 grant handler tests pass (17 existing + 4 new)
• Open redirect blocked on approve, deny, and form display
• Protocol-relative URLs (//evil.com) blocked
• Existing redirect flows (server-relative then, empty then) unaffected

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Blocked external redirect targets in grant approval and denial flows.
  • Invalid redirect parameters now remain on the local page instead of sending users to untrusted destinations.
  • Added validation for both absolute and protocol-relative redirect URLs.

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bb121dff-ff6a-4f3d-88a0-3d9ffa09125a

📥 Commits

Reviewing files that changed from the base of the PR and between 1600eaf and 0d8eb85.

📒 Files selected for processing (2)

• pkg/server/grant/grant.go
• pkg/server/grant/grant_test.go

---

Walkthrough

The grant handlers now reject external and protocol-relative then redirect parameters. Tests cover form display, approval, and denial behavior.

Changes

Grant redirect validation

Layer / File(s)	Summary
Validate grant redirect parameters pkg/server/grant/grant.go	handleForm rejects non-server-relative then values. handleGrant clears invalid values before approval or denial processing.
Test invalid redirect handling pkg/server/grant/grant_test.go	Tests verify that absolute and protocol-relative redirects are rejected while grant responses remain local.

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

Merge Risk: ⚪ Minimal · up to 0d8eb

The change validates redirect targets in grant approval, denial, and form-display flows to block external redirects while preserving server-relative redirects; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 15

✅ Passed checks (15 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes validation of the then parameter to prevent open redirects, which is the main change.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Stable And Deterministic Test Names	✅ Passed	The four added t.Run titles are literal, descriptive strings with no runtime values, generated identifiers, dates, IPs, or overly specific changing details.
Test Structure And Quality	✅ Passed	Added cases are standard Go table-driven t.Run tests, not Ginkgo; they use in-memory fake clients, no cluster waits, and the shared runner provides diagnostic failure messages and closes responses.
Microshift Test Compatibility	✅ Passed	The PR adds standard Go table-driven unit cases under TestGrant with t.Run; it adds no Ginkgo e2e tests or MicroShift-incompatible API references.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The PR adds standard Go table-driven tests under pkg/server/grant, not Ginkgo e2e tests; they use httptest and contain no multi-node or HA assumptions.
Topology-Aware Scheduling Compatibility	✅ Passed	The diff changes only grant redirect validation and tests; it adds no deployment manifests, operators, controllers, replicas, affinity, topology spread, selectors, tolerations, or PDBs.
Ote Binary Stdout Contract	✅ Passed	The PR changes only grant redirect handling and test data; added lines contain no stdout writes or process-level setup, and the repository states it is not OTE-compatible.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The added cases are standard Go table tests under TestGrant, use httptest.NewServer, and only pass evil.example.com as data; no Ginkgo e2e or external connection is used.
No-Weak-Crypto	✅ Passed	The PR changes only redirect validation and tests. It adds no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, custom crypto, or secret comparison code.
Container-Privileges	✅ Passed	The PR changes only grant-handler Go code and tests; no container or Kubernetes manifest changes introduce privileged, host namespace, SYS_ADMIN, root, or allowPrivilegeEscalation settings.
No-Sensitive-Data-In-Logs	✅ Passed	The diff adds redirect validation and tests only. It introduces no logging calls or sensitive-data logging; existing klog calls are unchanged.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: yuvalk
Once this PR has been reviewed and has the lgtm label, please assign p0lyn0mial for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[openshift-ci]

@yuvalk: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.