Skip to content

Validate integrate URL prefixes on URL boundaries - #359

Open
sai5880 wants to merge 1 commit into
google:masterfrom
sai5880:fix/integrate-prefix-boundary-358
Open

sai5880 wants to merge 1 commit into
google:masterfrom
sai5880:fix/integrate-prefix-boundary-358

Conversation

@sai5880

@sai5880 sai5880 commented Oct 21, 2026

Copy link
Copy Markdown

Summary

Fixes #358.

allowed_ref_prefixes previously used String.startsWith, so https://github.com also allowed https://github.com.evil.example, https://github.com/google allowed https://github.com/google-evil, and an empty prefix allowed every URL.

This change parses the integrate URL and each allowed prefix as URIs and matches on URL boundaries instead of raw string prefixes.

Changes

  • Added IntegrateUrlPrefixes, a small helper that:
    • requires each allowed prefix to be non-empty and an absolute hierarchical URI.
    • requires scheme and host equality, ignoring case.
    • normalizes default ports (80 for http, 443 for https) before comparing effective ports.
    • allows path containment only when paths are equal, the prefix path ends in /, or the next character in the URL path is /.
    • rejects prefixes containing user information, a query, or a fragment, since those are not part of repository identity and would make prefix matching ambiguous.
  • Used the helper in IntegrateLabel.genericGitRevision and GitHubPrIntegrateLabel.maybeParse.
  • Validated allowed_ref_prefixes when the Starlark git.integrate() config is evaluated, reporting failures as EvalException. Simple URL prefixes continue to work.
  • Added unit tests for host suffix spoofing, org suffix spoofing, empty prefixes, path prefixes without a slash boundary, exact repository matches, allowed org prefixes, host case handling, default port normalization, invalid prefix forms, and the GitHub PR label path.

Tests

Bazel is not installed in my environment, so the Bazel test suite was not run locally. A best-effort javac invocation could not resolve project dependencies because there is no Bazel dependency cache available, so it did not provide a meaningful compile check. No test results are claimed.

CI should run:

bazel test //javatests/com/google/copybara/git:IntegrateLabelTest_all_tests //javatests/com/google/copybara/git:GitHubPrIntegrateLabelTest_all_tests //javatests/com/google/copybara/git:GitDestinationIntegrateTest_all_tests

Fixes #358

@sai5880
sai5880 force-pushed the fix/integrate-prefix-boundary-358 branch 3 times, most recently from 9b89ede to 6a73c5c Compare October 22, 2026 19:36
@sai5880

sai5880 commented Oct 22, 2026

Copy link
Copy Markdown
Author

The import check still fails with a generic error after flattening to one commit and removing the new file. Could a maintainer retry the import or check the internal log?

@sai5880
sai5880 force-pushed the fix/integrate-prefix-boundary-358 branch from 6a73c5c to 5df581e Compare October 22, 2026 19:42
@sai5880

sai5880 commented Oct 22, 2026

Copy link
Copy Markdown
Author

The change now only modifies existing files, with a single commit on current master. The import check has moved from error to pending: waiting for internal safe review approval.

@sai5880

sai5880 commented Oct 26, 2026

Copy link
Copy Markdown
Author

The import check has been pending internal safe review since October 22. Is there anything you need from me to move it forward?

Match allowed integrate prefixes on URL boundaries instead of raw string prefixes. Reject empty prefixes and enforce scheme, host, port and path boundaries.
@sai5880

sai5880 commented Oct 27, 2026

Copy link
Copy Markdown
Author

Rebased onto current master so the branch is up to date. Commit: $h. The import check may need a retry after the force-push.

@sai5880
sai5880 force-pushed the fix/integrate-prefix-boundary-358 branch from 5df581e to 2fcede9 Compare October 27, 2026 18:33
@sai5880

sai5880 commented Oct 27, 2026

Copy link
Copy Markdown
Author

The public import succeeded at 18:37:31, but \ eedback/copybara\ has failed repeatedly. The latest status at 19:05:08 says \Google internal checks FAILED\ for the internal run created at 18:59:20 and points to \cl/1008190291. The public check gives no further detail. Could a maintainer check the internal presubmit log or let me know what failed? The branch is rebased on current \master\ and contains one commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Integrate URL allow-list uses raw string prefixes and can be spoofed

1 participant