Skip to content

Commit bf615ff

Browse files
gustavoavenameta-codesync[bot]
authored andcommitted
restricted_paths: add typed check primitives
Summary: ## This stack This stack completes the second half of the restricted_paths AclManifest migration after the shadow comparison logging stack. The end result is that restricted_paths no longer relies on legacy construction flags or source-specific call paths: AclManifestMode owns source selection, config and AclManifest fetches are planned explicitly, the fetch results are shared between logging, enforcement, and metadata, and the public callers adapt to the final manifest-aware shape at the API boundary. Visual flow: https://pxl.cl/9GjpF Before this stack: - Behavior was split across the old use_acl_manifest construction state, config-shaped metadata APIs, conditional_enforcement_acls, duplicated source fetches, and unimplemented AclManifest enforcement modes. - Logging and enforcement could ask for overlapping source data independently, and Mononoke API/HG, SCS, and SLAPI still had to bridge around the incomplete manifest restriction shape. After this stack: - Construction and source planning are mode-driven. Disabled, Shadow, Both, and Authoritative modes choose the sources they need; - Config and AclManifest fetches can be killed switched independently by rollout knobs; - Shared typed fetch handles feed both comparison logging and enforcement; - Conditional enforcement uses `enforcement_condition_sets` - `AclManifestMode::Both` mode denies if either source denies; - Authoritative-source errors are surfaced when the authoritative source cannot answer; - Metadata lookup unions config and AclManifest results in Both mode. This makes the codebase better because - Source (config vs acl_manifest) choice is centralized, - Fetch work is not duplicated - Rollout controls are easier to reason about - Restriction check results are typed instead of copied into loosely shaped structs, and API-specific adaptation happens at the edge instead of leaking through the restricted_paths internals. - **The stack also stays reviewable by putting no-op setup and tests before behavior changes** Remaining follow-ups after this stack are - Schematized Scuba source/error parity - The Eden API vector response shape tracked by TODO(T248658346) - Reviewing the both-sources-disabled fail-open behavior also tracked by TODO(T248658346) - Deleting rollout-only fallback paths after AclManifest enforcement is proven. ## This diff (new functionality) Adds typed restriction-check primitives in `restricted_paths` so callers can get restriction info paired with an authorization decision without re-parsing ACLs or running raw permission checks themselves. The new path and manifest check result types compose the existing restriction-info structs with an internal authorization result and expose only the final authorization decision publicly. The legacy result move and authorization getter refactor are handled by the two no-op parent diffs. Reviewed By: lmvasquezg Differential Revision: D103696888 fbshipit-source-id: 5b50bcd1d7923ea03c272bd50af43bb0885f2432
1 parent 2d3fdef commit bf615ff

4 files changed

Lines changed: 330 additions & 34 deletions

File tree

eden/mononoke/repo_attributes/restricted_paths/src/lib.rs

Lines changed: 51 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ pub use crate::access_log::ACCESS_LOG_SCUBA_TABLE;
4040
pub use crate::access_log::has_read_access_to_repo_region_acls;
4141
use crate::access_log::is_member_of_groups;
4242
use crate::access_log::log_access_to_restricted_path;
43+
pub use crate::restriction_check::ManifestRestrictionCheckResult;
44+
pub use crate::restriction_check::PathRestrictionCheckResult;
4345
pub use crate::restriction_check::RestrictionCheckResult;
4446
pub use crate::restriction_info::ManifestRestrictionInfo;
4547
pub use crate::restriction_info::PathRestrictionInfo;
@@ -161,15 +163,15 @@ impl RestrictedPaths {
161163
// Public restriction lookup methods
162164
// -----------------------------------------------------------------------
163165

164-
/// Get exact path restriction info for one or more paths.
165-
/// Does NOT consider parent directories — only exact matches.
166-
pub async fn get_exact_path_restriction(
166+
/// Get restriction info for paths that are themselves restriction roots.
167+
/// Does NOT consider parent directories.
168+
pub async fn get_path_restriction_root_info(
167169
&self,
168170
ctx: &CoreContext,
169171
cs_id: Option<ChangesetId>,
170172
paths: &[NonRootMPath],
171173
) -> Result<Vec<PathRestrictionInfo>> {
172-
restriction_info::get_exact_path_restriction(self, ctx, cs_id, paths).await
174+
restriction_info::get_path_restriction_root_info(self, ctx, cs_id, paths).await
173175
}
174176

175177
/// Get restriction info for one or more paths, considering ancestor restrictions.
@@ -183,6 +185,45 @@ impl RestrictedPaths {
183185
restriction_info::get_path_restriction_info(self, ctx, cs_id, paths).await
184186
}
185187

188+
/// Get restriction and authorization checks for paths that are themselves restriction roots.
189+
/// Does NOT consider parent directories.
190+
pub async fn get_path_restriction_root_check(
191+
&self,
192+
ctx: &CoreContext,
193+
cs_id: Option<ChangesetId>,
194+
paths: &[NonRootMPath],
195+
) -> Result<Vec<PathRestrictionCheckResult>> {
196+
restriction_check::get_path_restriction_root_check(self, ctx, cs_id, paths).await
197+
}
198+
199+
/// Get restriction and authorization checks for one or more paths,
200+
/// considering ancestor restrictions.
201+
pub async fn get_path_restriction_check(
202+
&self,
203+
ctx: &CoreContext,
204+
cs_id: Option<ChangesetId>,
205+
paths: &[NonRootMPath],
206+
) -> Result<Vec<PathRestrictionCheckResult>> {
207+
restriction_check::get_path_restriction_check(self, ctx, cs_id, paths).await
208+
}
209+
210+
/// Get manifest restriction and authorization checks from the config-backed source.
211+
pub async fn get_manifest_restriction_check(
212+
&self,
213+
ctx: &CoreContext,
214+
manifest_id: &ManifestId,
215+
manifest_type: &ManifestType,
216+
) -> Result<Vec<ManifestRestrictionCheckResult>> {
217+
restriction_check::get_manifest_restriction_check(
218+
self,
219+
ctx,
220+
manifest_id,
221+
manifest_type,
222+
restriction_check::ManifestRestrictionSource::Config,
223+
)
224+
.await
225+
}
226+
186227
/// Check if a path is itself a restriction root (exact match).
187228
/// Returns false for paths that are merely under a restriction root.
188229
pub async fn is_restriction_root(
@@ -647,7 +688,7 @@ mod tests {
647688
let test_path = NonRootMPath::new("test/path")?;
648689
assert!(
649690
repo_restricted_paths
650-
.get_exact_path_restriction(&ctx, Some(cs_id), &[test_path])
691+
.get_path_restriction_root_info(&ctx, Some(cs_id), &[test_path])
651692
.await?
652693
.is_empty()
653694
);
@@ -715,7 +756,7 @@ mod tests {
715756
// Test exact match
716757
let exact_path = NonRootMPath::new("restricted/dir")?;
717758
let results = repo_restricted_paths
718-
.get_exact_path_restriction(&ctx, Some(cs_id), &[exact_path])
759+
.get_path_restriction_root_info(&ctx, Some(cs_id), &[exact_path])
719760
.await?;
720761
assert_eq!(results.len(), 1);
721762
assert_eq!(results[0].repo_region_acl, restricted_acl.to_string());
@@ -724,7 +765,7 @@ mod tests {
724765
let sub_path = NonRootMPath::new("restricted/dir/subdir/file.txt")?;
725766
assert!(
726767
repo_restricted_paths
727-
.get_exact_path_restriction(&ctx, Some(cs_id), &[sub_path])
768+
.get_path_restriction_root_info(&ctx, Some(cs_id), &[sub_path])
728769
.await?
729770
.is_empty()
730771
);
@@ -733,7 +774,7 @@ mod tests {
733774
let other_path = NonRootMPath::new("other/dir/file.txt")?;
734775
assert!(
735776
repo_restricted_paths
736-
.get_exact_path_restriction(&ctx, Some(cs_id), &[other_path])
777+
.get_path_restriction_root_info(&ctx, Some(cs_id), &[other_path])
737778
.await?
738779
.is_empty()
739780
);
@@ -742,7 +783,7 @@ mod tests {
742783
let partial_path = NonRootMPath::new("restricted/different")?;
743784
assert!(
744785
repo_restricted_paths
745-
.get_exact_path_restriction(&ctx, Some(cs_id), &[partial_path])
786+
.get_path_restriction_root_info(&ctx, Some(cs_id), &[partial_path])
746787
.await?
747788
.is_empty()
748789
);
@@ -751,7 +792,7 @@ mod tests {
751792
let partial_path = NonRootMPath::new("restricted/di")?;
752793
assert!(
753794
repo_restricted_paths
754-
.get_exact_path_restriction(&ctx, Some(cs_id), &[partial_path])
795+
.get_path_restriction_root_info(&ctx, Some(cs_id), &[partial_path])
755796
.await?
756797
.is_empty()
757798
);

0 commit comments

Comments
 (0)