Skip to content

Commit 2d3fdef

Browse files
gustavoavenameta-codesync[bot]
authored andcommitted
restricted_paths: add authorization getters
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 (no-op) Makes `AuthorizationCheckResult` fields private and adds getter methods for the existing authorization flags. Existing access-log callers now use those getters, preserving the same values and behavior without introducing the new typed path or manifest check result structs. ___ overriding_review_checks_triggers_an_audit_and_retroactive_review Oncall Short Name: source_control Differential Revision: D103699260 fbshipit-source-id: 665ac53ec4f7ae72e2d9b8754d16c71e3e69d075
1 parent 342197e commit 2d3fdef

2 files changed

Lines changed: 18 additions & 6 deletions

File tree

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -877,9 +877,9 @@ pub(crate) async fn log_access_to_restricted_path(
877877
restricted_paths: Some(&restricted_paths),
878878
authorization: LoggedAuthorization {
879879
has_authorization: authorization.has_authorization(),
880-
is_allowlisted_tooling: authorization.is_allowlisted_tooling,
881-
is_rollout_allowlisted: authorization.is_rollout_allowlisted,
882-
has_acl_access: authorization.has_acl_access,
880+
is_allowlisted_tooling: authorization.is_allowlisted_tooling(),
881+
is_rollout_allowlisted: authorization.is_rollout_allowlisted(),
882+
has_acl_access: authorization.has_acl_access(),
883883
},
884884
acls,
885885
}),

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

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,18 +63,30 @@ pub struct RestrictionCheckResult {
6363
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
6464
pub(crate) struct AuthorizationCheckResult {
6565
/// Whether the caller has direct read access to every matching path ACL.
66-
pub(crate) has_acl_access: bool,
66+
has_acl_access: bool,
6767
/// Whether the caller is in the tooling allowlist group.
68-
pub(crate) is_allowlisted_tooling: bool,
68+
is_allowlisted_tooling: bool,
6969
/// Whether the caller is in the rollout allowlist group.
70-
pub(crate) is_rollout_allowlisted: bool,
70+
is_rollout_allowlisted: bool,
7171
}
7272

7373
impl AuthorizationCheckResult {
7474
/// Whether the caller has read authorization through ACLs or allowlists.
7575
pub(crate) fn has_authorization(&self) -> bool {
7676
self.has_acl_access || self.is_allowlisted_tooling || self.is_rollout_allowlisted
7777
}
78+
79+
pub(crate) fn has_acl_access(&self) -> bool {
80+
self.has_acl_access
81+
}
82+
83+
pub(crate) fn is_allowlisted_tooling(&self) -> bool {
84+
self.is_allowlisted_tooling
85+
}
86+
87+
pub(crate) fn is_rollout_allowlisted(&self) -> bool {
88+
self.is_rollout_allowlisted
89+
}
7890
}
7991

8092
/// Evaluate ACL and allowlist authorization for a restricted-path access.

0 commit comments

Comments
 (0)