Skip to content

Reject userinfo in public base URLs#77

Merged
ramimbo merged 1 commit into
ramimbo:mainfrom
DYSfu:dysfu/issue-55-reject-public-base-userinfo
May 24, 2026
Merged

Reject userinfo in public base URLs#77
ramimbo merged 1 commit into
ramimbo:mainfrom
DYSfu:dysfu/issue-55-reject-public-base-userinfo

Conversation

@DYSfu
Copy link
Copy Markdown
Contributor

@DYSfu DYSfu commented May 24, 2026

Refs #55

Summary

Reject deploy configurations where MERGEWORK_PUBLIC_BASE_URL includes URL userinfo, for example https://operator:secret@mrwk.example.test.

Observed problem

The deploy-readiness gate already requires HTTPS and a host, but it still accepted a public base URL with embedded userinfo. That is not a clean public origin for externally visible links or GitHub OAuth callback construction, and it can let credential-shaped values slip into deploy configuration.

Changed behavior

  • Detect username or password components after parsing MERGEWORK_PUBLIC_BASE_URL.
  • Return a deploy-readiness error when userinfo is present.
  • Add focused regression coverage for the rejected shape.

Validation

  • Red test before the fix: uv run --extra dev python -m pytest tests/test_deploy_readiness.py::test_deploy_readiness_rejects_public_base_url_userinfo -q failed because the error list was empty.
  • uv run --extra dev python -m pytest tests/test_deploy_readiness.py::test_deploy_readiness_rejects_public_base_url_userinfo -q -> 1 passed
  • uv run --extra dev python -m pytest tests/test_deploy_readiness.py -q -> 5 passed
  • uv run --extra dev python -m pytest -q -> 80 passed
  • uv run --extra dev ruff format --check app/config.py tests/test_deploy_readiness.py -> 2 files already formatted
  • uv run --extra dev ruff check app/config.py tests/test_deploy_readiness.py -> All checks passed
  • uv run --extra dev mypy app -> Success
  • Valid deploy env + uv run --extra dev python scripts/check_deploy_ready.py -> Deploy readiness check passed
  • Userinfo deploy env + uv run --extra dev python scripts/check_deploy_ready.py -> failed with MERGEWORK_PUBLIC_BASE_URL must not include userinfo
  • uv run --extra dev python scripts/docs_smoke.py -> docs smoke ok
  • git diff --check -- app/config.py tests/test_deploy_readiness.py -> passed

Copy link
Copy Markdown

@Ckeplinger199 Ckeplinger199 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers found on PR #77.

Checked:

  • app/config.py
  • tests/test_deploy_readiness.py

The deploy-readiness validator now rejects URL userinfo in MERGEWORK_PUBLIC_BASE_URL before that value can be used as the public origin for links/OAuth callback construction. I verified the branch rejects username-only, password-only, and username+password URL shapes while preserving a normal public HTTPS origin.

Validation:

  • uv run --extra dev python -m pytest tests/test_deploy_readiness.py::test_deploy_readiness_rejects_public_base_url_userinfo tests/test_deploy_readiness.py::test_deploy_readiness_accepts_strong_configuration -q -> 2 passed
  • uv run --extra dev python -m pytest tests/test_deploy_readiness.py -q -> 5 passed
  • uv run --extra dev python -m pytest -q -> 80 passed
  • uv run --extra dev ruff format --check app/config.py tests/test_deploy_readiness.py -> passed
  • uv run --extra dev ruff check app/config.py tests/test_deploy_readiness.py -> passed
  • uv run --extra dev mypy app -> passed
  • git diff --check origin/main...HEAD -- app/config.py tests/test_deploy_readiness.py -> passed

GitHub reports PR #77 as CLEAN and the CI check Quality, readiness, docs, and image checks SUCCESS. No secrets, deployment changes, wallet signing, spending, or price claims included.

@DYSfu DYSfu force-pushed the dysfu/issue-55-reject-public-base-userinfo branch from 4122d4d to fce4847 Compare May 24, 2026 07:16
@DYSfu
Copy link
Copy Markdown
Contributor Author

DYSfu commented May 24, 2026

Rebased this branch onto current main and resolved the deploy-readiness conflict by keeping both validations: the upstream origin-only URL checks and this PR's userinfo rejection.

Validation after the rebase:

  • uv run --with '.[dev]' pytest tests/test_deploy_readiness.py -> 7 passed
  • uv run --with '.[dev]' pytest -> 99 passed
  • uv run --with '.[dev]' ruff format --check app/config.py tests/test_deploy_readiness.py -> 2 files already formatted
  • uv run --with '.[dev]' ruff check app/config.py tests/test_deploy_readiness.py scripts/check_deploy_ready.py -> all checks passed
  • uv run --with '.[dev]' mypy app -> success
  • python -m py_compile app/config.py scripts/check_deploy_ready.py
  • git diff --check

The GitHub check is green again and the PR is mergeable now.

Copy link
Copy Markdown
Contributor

@dobbobalina2 dobbobalina2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers from my pass.

I inspected the narrow deploy-readiness change in app/config.py and the regression coverage in tests/test_deploy_readiness.py. The validator already rejects non-HTTPS public origins, missing hosts, path/params, query, and fragment; this PR adds the missing userinfo guard so values like https://operator:secret@mrwk.example.test are rejected before they can become a public origin for links or OAuth callback configuration. The new test covers that shape directly, and the check is scoped to the existing deployment validation path.

Local validation from PR head fce4847:

  • uv run --extra dev pytest tests/test_deploy_readiness.py -q -> 7 passed
  • uv run --extra dev pytest -q -> 99 passed, 2 warnings
  • uv run --extra dev ruff format --check app/config.py tests/test_deploy_readiness.py -> 2 files already formatted
  • uv run --extra dev ruff check app/config.py tests/test_deploy_readiness.py -> All checks passed!
  • uv run --extra dev mypy app -> Success: no issues found in 11 source files
  • git diff --check origin/main...HEAD -- app/config.py tests/test_deploy_readiness.py -> clean

I also checked that GitHub CI Quality, readiness, docs, and image checks is passing. Prepared with AI assistance.

@ramimbo ramimbo merged commit ee107a9 into ramimbo:main May 24, 2026
1 check passed
@ramimbo ramimbo added mrwk:accepted Maintainer accepted for payout mrwk:paid Ledger payment recorded labels May 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mrwk:accepted Maintainer accepted for payout mrwk:paid Ledger payment recorded

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants