mononoke: verify bookmark namespace even during pushrebase

Summary:
We accidentally were not verifying the bookmark namespace (with regard to infinitepush) when performing a pushrebase.

This is a problem, because it means if a pushrebase was performed into the infinitepush namespace, then the push would be allowed. This could happen by accident (the earlier diff in this stack fixes a Mercurial bug that did this), or simply if the end-user changes their infinitepush branchpattern.

This patch fixes the bug, and extracts the "do basic checks for whether this bookmark can move" logic into a single function to minimize the potential for this validation logic diverging again between pushrebase and push.

Reviewed By: ikostia

Differential Revision: D15576198

fbshipit-source-id: 24cf9999a7370503e5e0173e34185d9aa57903f7
This commit is contained in:
Thomas Orozco 2019-05-31 08:54:12 -07:00 committed by Facebook Github Bot
parent 45847134ac
commit 0b3cd11c26
2 changed files with 217 additions and 26 deletions

View File

@ -1414,14 +1414,14 @@ impl Bundle2Resolver {
params
};
let user = ctx.user_unix_name();
if !self.bookmark_attrs.is_allowed_user(user, bookmark) {
return future::err(format_err!(
"[pushrebase] This user `{:?}` is not allowed to move `{:?}`",
user,
bookmark
))
.boxify();
if let Err(error) = check_plain_bookmark_move_preconditions(
&ctx,
&bookmark,
"pushrebase",
&self.bookmark_attrs,
&self.infinitepush_params,
) {
return future::err(error).boxify();
}
let block_merges = pushrebase.block_merges.clone();
@ -1577,6 +1577,36 @@ fn check_is_ancestor_opt(
}
}
fn check_plain_bookmark_move_preconditions(
ctx: &CoreContext,
bookmark: &BookmarkName,
reason: &'static str,
bookmark_attrs: &BookmarkAttrs,
infinitepush_params: &Option<InfinitepushParams>,
) -> Result<()> {
let user = ctx.user_unix_name();
if !bookmark_attrs.is_allowed_user(user, bookmark) {
return Err(format_err!(
"[{}] This user `{:?}` is not allowed to move `{:?}`",
reason,
user,
bookmark
));
}
if let Some(InfinitepushParams { namespace }) = infinitepush_params {
if namespace.matches_bookmark(bookmark) {
return Err(format_err!(
"[{}] Only Infinitepush bookmarks are allowed to match pattern {}",
reason,
namespace.as_str(),
));
}
}
Ok(())
}
fn check_bookmark_push_allowed(
ctx: CoreContext,
repo: BlobRepo,
@ -1586,24 +1616,14 @@ fn check_bookmark_push_allowed(
bp: PlainBookmarkPush<ChangesetId>,
lca_hint: Arc<dyn LeastCommonAncestorsHint>,
) -> impl Future<Item = PlainBookmarkPush<ChangesetId>, Error = Error> {
let user = ctx.user_unix_name();
if !bookmark_attrs.is_allowed_user(user, &bp.name) {
return future::err(format_err!(
"[push] This user `{:?}` is not allowed to move `{:?}`",
user,
&bp.name
))
.right_future();
}
if let Some(InfinitepushParams { namespace }) = infinitepush_params {
if namespace.matches_bookmark(&bp.name) {
return future::err(format_err!(
"[push] Only Infinitepush bookmarks are allowed to match pattern {}",
namespace.as_str(),
))
.right_future();
}
if let Err(error) = check_plain_bookmark_move_preconditions(
&ctx,
&bp.name,
"push",
&bookmark_attrs,
&infinitepush_params,
) {
return future::err(error).right_future();
}
let fastforward_only_bookmark = bookmark_attrs.is_fast_forward_only(&bp.name);

View File

@ -0,0 +1,171 @@
$ . $TESTDIR/library.sh
setup configuration
$ INFINITE_PUSH_NAMESPACE_REGEX='^(infinitepush1|infinitepush2)/.+$' setup_common_config
$ cd $TESTTMP
setup common configuration for these tests
$ cat >> $HGRCPATH <<EOF
> [extensions]
> infinitepush=
> commitcloud=
> remotenames=
> [infinitepush]
> server=False
> branchpattern=re:(infinitepush1|bad)/.+
> EOF
setup repo
$ hginit_treemanifest repo-hg
$ cd repo-hg
$ touch a && hg addremove && hg ci -q -ma
adding a
$ hg log -T '{short(node)}\n'
3903775176ed
create master bookmark
$ hg bookmark master_bookmark -r tip
$ cd $TESTTMP
setup repo-push and repo-pull
$ hgclone_treemanifest ssh://user@dummy/repo-hg repo-push --noupdate
$ hgclone_treemanifest ssh://user@dummy/repo-hg repo-pull --noupdate
blobimport
$ blobimport repo-hg/.hg repo
start mononoke
$ mononoke
$ wait_for_mononoke $TESTTMP/repo
Prepare push
$ cd repo-push
$ hg up tip
1 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ echo new > newfile
$ hg addremove -q
$ hg ci -m new
Valid infinitepush, with pushrebase disabled
$ hgmn push ssh://user@dummy/repo -r . --to "infinitepush1/123" --create
pushing to ssh://user@dummy/repo
searching for changes
Valid infinitepush, with pushrebase enabled
$ hgmn push ssh://user@dummy/repo -r . --to "infinitepush1/456" --create --config extensions.pushrebase=
pushing to ssh://user@dummy/repo
searching for changes
Invalid infinitepush, with pushrebase disabled
$ hgmn push ssh://user@dummy/repo -r . --to "bad/123" --create
pushing to ssh://user@dummy/repo
searching for changes
remote: Command failed
remote: Error:
remote: bundle2_resolver error
remote: Root cause:
remote: ErrorMessage {
remote: msg: "Invalid Infinitepush bookmark: bad/123 (Infinitepush bookmarks must match pattern ^(infinitepush1|infinitepush2)/.+$)",
remote: }
remote: Caused by:
remote: While updating Bookmarks
remote: Caused by:
remote: While verifying Infinite Push bookmark push
remote: Caused by:
remote: Invalid Infinitepush bookmark: bad/123 (Infinitepush bookmarks must match pattern ^(infinitepush1|infinitepush2)/.+$)
abort: stream ended unexpectedly (got 0 bytes, expected 4)
[255]
Invalid infinitepush, with pushrebase enabled
$ hgmn push ssh://user@dummy/repo -r . --to "bad/456" --create --config extensions.pushrebase=
pushing to ssh://user@dummy/repo
searching for changes
remote: Command failed
remote: Error:
remote: bundle2_resolver error
remote: Root cause:
remote: ErrorMessage {
remote: msg: "Invalid Infinitepush bookmark: bad/456 (Infinitepush bookmarks must match pattern ^(infinitepush1|infinitepush2)/.+$)",
remote: }
remote: Caused by:
remote: While updating Bookmarks
remote: Caused by:
remote: While verifying Infinite Push bookmark push
remote: Caused by:
remote: Invalid Infinitepush bookmark: bad/456 (Infinitepush bookmarks must match pattern ^(infinitepush1|infinitepush2)/.+$)
abort: stream ended unexpectedly (got 0 bytes, expected 4)
[255]
Valid push, with pushrebase disabled
$ hgmn push ssh://user@dummy/repo -r . --to "plain/123" --create
pushing rev 47da8b81097c to destination ssh://user@dummy/repo bookmark plain/123
searching for changes
exporting bookmark plain/123
Valid push, with pushrebase enabled
$ hgmn push ssh://user@dummy/repo -r . --to "plain/456" --create --config extensions.pushrebase=
pushing rev 47da8b81097c to destination ssh://user@dummy/repo bookmark plain/456
searching for changes
no changes found
exporting bookmark plain/456
[1]
Invalid push, with pushrebase disabled
$ hgmn push ssh://user@dummy/repo -r . --to "infinitepush2/123" --create
pushing rev 47da8b81097c to destination ssh://user@dummy/repo bookmark infinitepush2/123
searching for changes
no changes found
remote: Command failed
remote: Error:
remote: bundle2_resolver error
remote: Root cause:
remote: ErrorMessage {
remote: msg: "[push] Only Infinitepush bookmarks are allowed to match pattern ^(infinitepush1|infinitepush2)/.+$",
remote: }
remote: Caused by:
remote: While updating Bookmarks
remote: Caused by:
remote: [push] Only Infinitepush bookmarks are allowed to match pattern ^(infinitepush1|infinitepush2)/.+$
abort: stream ended unexpectedly (got 0 bytes, expected 4)
[255]
Invalid push, with pushrebase enabled
$ hgmn push ssh://user@dummy/repo -r . --to "infinitepush2/456" --create --config extensions.pushrebase=
pushing rev 47da8b81097c to destination ssh://user@dummy/repo bookmark infinitepush2/456
searching for changes
no changes found
remote: Command failed
remote: Error:
remote: [push] Only Infinitepush bookmarks are allowed to match pattern ^(infinitepush1|infinitepush2)/.+$
remote: Root cause:
remote: ErrorMessage {
remote: msg: "[push] Only Infinitepush bookmarks are allowed to match pattern ^(infinitepush1|infinitepush2)/.+$",
remote: }
abort: stream ended unexpectedly (got 0 bytes, expected 4)
[255]
Check everything is as expected
$ cd ..
$ cd repo-pull
$ hgmn pull
pulling from ssh://user@dummy/repo
searching for changes
adding changesets
adding manifests
adding file changes
added 1 changesets with 0 changes to 0 files
new changesets 47da8b81097c
$ hg bookmarks --remote
default/master_bookmark 0:3903775176ed
default/plain/123 1:47da8b81097c
default/plain/456 1:47da8b81097c
$ hgmn bookmarks --list-remote "*"
infinitepush1/123 47da8b81097c5534f3eb7947a8764dd323cffe3d
infinitepush1/456 47da8b81097c5534f3eb7947a8764dd323cffe3d
master_bookmark 3903775176ed42b1458a6281db4a0ccf4d9f287a
plain/123 47da8b81097c5534f3eb7947a8764dd323cffe3d
plain/456 47da8b81097c5534f3eb7947a8764dd323cffe3d