fix(gmail): redact embedded credentials in watch hook URL output

[bunlongheng]

Summary

gog gmail watch status prints the configured webhook hook URL verbatim in both text and JSON output. Any credentials embedded in that URL are leaked in plaintext even without --show-secrets, which defeats the existing hook bearer-token redaction (landed in #136).

Leaked credential classes:

• Basic-auth userinfo - https://user:pass@host/hook (Go's http.Client uses this for Authorization: Basic, so it is a real, functional webhook credential).
• Secret query params - https://host/hook?token=SECRET (Slack-style and many webhook providers put the secret in the query string; the CLI's own --token help even documents ?token=).

This is the same secret-leak class the project already guards against: the hook bearer token is redacted here, and git remote URLs are redacted via redactGitURL in internal/backup/git.go. The hook URL was simply missed.

Fix

Add redactHookURL and apply it in writeWatchState (text + JSON paths) unless --show-secrets is passed. It strips userinfo, query values, and fragment while keeping scheme/host/path visible so the destination stays recognizable. Credential-free URLs are returned unchanged. This mirrors the existing redactGitURL behavior for consistency.

• Minimal change, no API/flag/schema changes.
• --show-secrets still reveals the full URL (unchanged escape hatch).

Reproduction / Evidence

Test added to the existing internal/cmd/gmail_watch_redact_test.go.

Before the fix (source reverted to main, new test kept) - credentials leak:

--- FAIL: TestWriteWatchState_HookURLCredentialRedaction/userinfo_password_redacted_by_default
    gmail_watch_redact_test.go:157: basic-auth password leaked in hook URL
--- FAIL: TestWriteWatchState_HookURLCredentialRedaction/query_token_redacted_by_default
    gmail_watch_redact_test.go:167: query token leaked in hook URL
--- FAIL: TestWriteWatchState_HookURLCredentialRedaction/json_output_redacts_url_credentials_by_default
    gmail_watch_redact_test.go:188: JSON output leaked hook URL credentials

After the fix - all pass:

ok  github.com/steipete/gogcli/internal/cmd
--- PASS: TestWriteWatchState_HookURLCredentialRedaction (userinfo/query/json/plain-url/show-secrets)
--- PASS: TestWriteWatchState_TokenRedaction (existing suite unchanged)

Verification

go build ./internal/cmd/    # ok
go vet ./internal/cmd/      # ok
gofmt -l ...                # clean
go test ./internal/cmd/ -run 'TestWriteWatchState|Watch'   # ok

Why it matters

gogcli is designed to be driven by agents and scripts; watch status output routinely lands in logs, transcripts, and shared terminals. A hook URL with embedded credentials should be treated exactly like the bearer token it sits next to.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed August 6, 2026, 7:24 PM ET / 23:24 UTC.

ClawSweeper review

What this changes

The PR redacts Gmail watch hook URL userinfo, query values, and fragments in text and JSON status output unless --show-secrets is supplied.

Merge readiness

⛔ Blocked until real behavior proof from a real setup is added - 5 items remain

This focused security fix is still needed on current main, but it is not ready to merge: the proposed redactor deliberately leaves path-embedded webhook credentials visible, and the PR has only unit-test proof.

Priority: P1
Reviewed head: fb0eb9f0d40ebbdf87b36d4275b7718278d10bac

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The PR has a focused regression test and clear goal, but the remaining path-secret leak and mock-only proof keep it below merge-ready.
Proof confidence	🦪 silver shellfish (2/6)	Needs real behavior proof before merge: The PR body provides focused unit-test output but no after-fix run of gog gmail watch status against a real stored webhook configuration; add a redacted terminal transcript before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality	🦐 gold shrimp (3/6)	Security review found an item that needs attention.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs real behavior proof before merge: The PR body provides focused unit-test output but no after-fix run of gog gmail watch status against a real stored webhook configuration; add a redacted terminal transcript before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed	4 items	Current main behavior: Current main writes the stored hook URL directly in text output and only redacts JSON when a separate hook token is present, leaving URL credentials exposed. Path remains intentionally visible: The PR helper explicitly retains the URL path, so an opaque credential encoded in a webhook path remains in default output. Related redaction provenance: The related merged PR established the existing hook-token and --show-secrets redaction pattern that this PR extends.
Findings	1 actionable finding	[P1] Redact path-embedded webhook credentials
Security	Needs attention	Path credentials remain visible: The redactor retains the full URL path, a common location for opaque webhook credentials; users may then share status output believing it is safe.

How this fits together

Gmail watch stores an optional outbound webhook URL in local watch state and prints that state for CLI users and automation. That URL is also passed to the webhook sender, so status output must not expose credentials embedded in the configured destination.

flowchart LR
A[Watch configuration] --> B[Stored watch state]
B --> C[Watch status command]
C --> D{Show secrets?}
D -->|No| E[Redacted status output]
D -->|Yes| F[Full status output]
B --> G[Webhook sender]

Loading

Before merge

• Add real behavior proof - Needs real behavior proof before merge: The PR body provides focused unit-test output but no after-fix run of gog gmail watch status against a real stored webhook configuration; add a redacted terminal transcript before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• Redact path-embedded webhook credentials (P1) - Webhook providers can encode an opaque credential in the path. This helper intentionally preserves that entire path, so ordinary watch status output can still disclose a functional secret despite presenting the URL as redacted. Conceal path tokens by default and cover text and JSON output.
• Resolve security concern: Path credentials remain visible - The redactor retains the full URL path, a common location for opaque webhook credentials; users may then share status output believing it is safe.
• Resolve merge risk (P1) - Webhook providers can put an opaque credential in the URL path; preserving the full path keeps that secret in ordinary status output despite the new redaction.
• Complete next step (P2) - Require the contributor to correct the remaining security boundary and provide real behavior proof before merge.

Findings

• [P1] Redact path-embedded webhook credentials — internal/cmd/gmail_watch_cmds.go:524
• [high] Path credentials remain visible — internal/cmd/gmail_watch_cmds.go:524

Agent review details

Security

Needs attention: The patch adds no dependency or execution surface, but its default renderer still exposes path-carried webhook secrets.

Review metrics

Metric	Value	Why it matters
Patch size	121 added, 3 removed across 2 files	The patch is narrowly focused on watch-status rendering and its regression tests.
Production vs test LOC	production +49, tests +72	Focused tests accompany the security change, though they omit path-carried credentials.

Merge-risk options

Maintainer options:

1. Redact path-carried credentials (recommended)
   Conceal URL paths that may contain opaque webhook secrets in default status output and add text and JSON regression coverage.
2. Accept the limited scope
   Merge userinfo/query/fragment-only redaction while documenting that path-carried webhook credentials remain visible by design.

Technical review

Best possible solution:

Use a conservative default representation that also conceals path-carried webhook credentials, while reserving the original URL for --show-secrets.

Do we have a high-confidence way to reproduce the issue?

Yes—current main directly prints the stored hook URL, and the PR’s focused before/after test transcript exercises that same status renderer.

Is this the best way to solve the issue?

No—the userinfo/query/fragment approach is useful but preserving the full path leaves common opaque webhook secrets exposed; the default representation must protect that placement too.

Full review comments:

• [P1] Redact path-embedded webhook credentials — internal/cmd/gmail_watch_cmds.go:524
  Webhook providers can encode an opaque credential in the path. This helper intentionally preserves that entire path, so ordinary watch status output can still disclose a functional secret despite presenting the URL as redacted. Conceal path tokens by default and cover text and JSON output.
  Confidence: 0.92

Overall correctness: patch is incorrect
Overall confidence: 0.92

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 8fe3e7995d0b.

Labels

Label changes:

• add P1: Default CLI status output can expose functional webhook credentials in shared logs or transcripts.
• add merge-risk: 🚨 security-boundary: The proposed default redaction still leaves path-carried webhook secrets visible, creating a misleadingly safe-looking output.
• add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
• add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body provides focused unit-test output but no after-fix run of gog gmail watch status against a real stored webhook configuration; add a redacted terminal transcript before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

• P1: Default CLI status output can expose functional webhook credentials in shared logs or transcripts.
• merge-risk: 🚨 security-boundary: The proposed default redaction still leaves path-carried webhook secrets visible, creating a misleadingly safe-looking output.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body provides focused unit-test output but no after-fix run of gog gmail watch status against a real stored webhook configuration; add a redacted terminal transcript before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

Security concerns:

• [high] Path credentials remain visible — internal/cmd/gmail_watch_cmds.go:524
  The redactor retains the full URL path, a common location for opaque webhook credentials; users may then share status output believing it is safe.
  Confidence: 0.92

What I checked:

• Current main behavior: Current main writes the stored hook URL directly in text output and only redacts JSON when a separate hook token is present, leaving URL credentials exposed. (internal/cmd/gmail_watch_cmds.go:468, 8fe3e7995d0b)
• Path remains intentionally visible: The PR helper explicitly retains the URL path, so an opaque credential encoded in a webhook path remains in default output. (internal/cmd/gmail_watch_cmds.go:524, fb0eb9f0d40e)
• Related redaction provenance: The related merged PR established the existing hook-token and --show-secrets redaction pattern that this PR extends. (internal/cmd/gmail_watch_redact_test.go:13, b49d9d4b92e2)
• Recent area history: A local file-history pass identifies recent Gmail watch maintenance in this path. (internal/cmd/gmail_watch_cmds.go:437, fd3d054499f3)

Likely related people:

• paveg: Authored merged PR fix(security): redact webhook bearer token in watch status output #136, which introduced the existing hook-token redaction and --show-secrets behavior. (role: introduced related redaction behavior; confidence: medium; commits: b49d9d4b92e2; files: internal/cmd/gmail_watch_cmds.go, internal/cmd/gmail_watch_redact_test.go)
• Peter Steinberger: Recent local history for the central command file includes Gmail watch maintenance in the cited commit. (role: recent Gmail watch contributor; confidence: medium; commits: fd3d054499f3; files: internal/cmd/gmail_watch_cmds.go)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Conceal path-carried webhook credentials in the default text and JSON representations and add regression coverage.
• Add a redacted terminal transcript from a real gog gmail watch status run; if updating the PR body does not trigger review, ask a maintainer to comment @clawsweeper re-review.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

[steipete]

Landed in 3fecbc007a5b90854dfcdad47d1d459ac461e620.

Maintainer fixes on top of the contributor commit:

• Default text and JSON status output now keep only the hook origin and redact userinfo, path, query, and fragment components.
• Malformed or non-origin hook URLs fail closed as [REDACTED].
• --show-secrets retains the exact stored URL.
• Added path-credential, malformed-URL, text, and JSON regression coverage plus the maintainer-owned changelog entry.

Proof:

• GOCACHE=<lane-private-cache> make ci — passed the complete local gate (format, lint, deadcode, unit tests, docs generation/coverage, and agent-skill checks).
• go test ./internal/cmd -run 'TestWriteWatchState' -count=1 — passed.
• Built bin/gog exercised source-blind against an isolated documented watch-state fixture: default text emitted hook_url https://example.com/[REDACTED]; JSON emitted the same URL and [REDACTED] hook token; malformed input emitted [REDACTED]; --show-secrets revealed the synthetic fixture values.
• Final Codex autoreview: clean, no accepted/actionable findings.
• GitHub ClawSweeper dispatch run 31291604621 passed on the final head. The ordinary GitHub CI matrix did not post new checks for the maintainer-updated fork head before merge; the full equivalent local gate above passed.

Thanks @bunlongheng for identifying and fixing this security boundary.