Skip to content

Commit d7b7959

Browse files
gustavoavenameta-codesync[bot]
authored andcommitted
restricted_paths: add shadow path dispatch tests
Summary: ## This stack This stack moves restricted paths toward AclManifest-backed restriction lookup and Shadow-mode comparison logging while keeping existing `RestrictedPaths` public APIs and config-backed enforcement behavior stable. The end state is that path and manifest access checks can evaluate both the legacy config/manifest-id-store source and the new AclManifest source, log source-prefixed comparison results to Scuba, and still return config-authoritative authorization results while Shadow mode is being validated. Before this stack, restricted path logging was effectively single-source: path access read `path_acls` from config, manifest access read restricted paths from the manifest-id store, and Scuba rows only described the aggregate config-backed result. That made it hard to prove whether AclManifest-derived restrictions matched production behavior before flipping traffic to them. After this stack, lookup and logging are split into source-specific primitives: config and AclManifest restriction lookup live behind explicit source selectors, authorization result construction is shared, and Shadow mode dispatch runs both sources side by side for supported path and HgAugmented manifest accesses. Scuba rows now include aggregate config-authoritative fields plus `config_*`, `acl_manifest_*`, `acl_manifest_mode`, errors, and `considered_restricted_by`, so we can compare behavior without changing enforcement. ```text Before: path access -> config path_acls -> aggregate auth/log row manifest access -> manifest-id store + config -> aggregate auth/log row After: path access -> config path_acls -> authoritative result -> AclManifest at changeset -> shadow comparison fields -> Scuba: aggregate + config_* + acl_manifest_* + considered_restricted_by HgAugmented manifest access -> config manifest-id store -> authoritative result -> manifest acl_manifest pointer -> shadow comparison fields -> Scuba: aggregate + config_* + acl_manifest_* + considered_restricted_by ``` This makes the codebase better by separating lookup, authorization, and logging responsibilities; making source choice explicit in tests and implementation; and giving us production observability for AclManifest parity before changing enforcement. Follow-up work can use the Shadow data to fix mismatches, expand support beyond HgAugmented manifest logging, move repos toward `Both`/`Authoritative` modes, and eventually remove legacy manifest-type handling and config-only fallbacks once AclManifest is proven safe. ## This diff (no-op) This diff adds Shadow path dispatch tests and reusable observation helpers. This is test-only setup with no production behavior change; it defines the expected path-side comparison rows, skipped comparisons, error logging, and unrestricted-row behavior before dispatch is wired. ___ overriding_review_checks_triggers_an_audit_and_retroactive_review Oncall Short Name: source_control Differential Revision: D103433632 fbshipit-source-id: aae009a742e5b3236dc58faa28da4c84742425f2
1 parent 9f2009a commit d7b7959

2 files changed

Lines changed: 287 additions & 15 deletions

File tree

eden/mononoke/repo_attributes/restricted_paths/test/main.rs

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ use std::str::FromStr;
99

1010
use anyhow::Result;
1111
use fbinit::FacebookInit;
12+
use metaconfig_types::AclManifestMode;
1213
use mononoke_macros::mononoke;
14+
use mononoke_types::ChangesetId;
1315
use mononoke_types::NonRootMPath;
1416
use mononoke_types::RepoPath;
1517
use mononoke_types::RepositoryId;
@@ -2036,3 +2038,122 @@ async fn test_use_acl_manifest_without_derivation_enabled_fails(fb: FacebookInit
20362038

20372039
Ok(())
20382040
}
2041+
2042+
// What it tests: Shadow path dispatch can run config and AclManifest sources side by side.
2043+
// Expected: compact mismatch fields are present while config remains authoritative.
2044+
#[mononoke::fbinit_test]
2045+
async fn test_shadow_path_dispatch_logs_config_and_acl_manifest_sources(
2046+
fb: FacebookInit,
2047+
) -> Result<()> {
2048+
let restricted_acl = MononokeIdentity::from_str("REPO_REGION:restricted_acl")?;
2049+
2050+
let restricted_root = NonRootMPath::new("restricted/dir")?;
2051+
let result = RestrictedPathsTestDataBuilder::new()
2052+
.with_acl_manifest_mode(AclManifestMode::Shadow)
2053+
.with_use_acl_manifest(false)
2054+
.with_config_restricted_paths(vec![(restricted_root.clone(), restricted_acl.clone())])
2055+
.with_acl_manifest_restricted_paths(vec![(restricted_root, restricted_acl)])
2056+
.with_file_path_changes(vec![("restricted/dir/a", None)])
2057+
.build(fb)
2058+
.await?
2059+
.observe_restricted_paths_scenario(&[])
2060+
.await?;
2061+
2062+
// TODO(T248660053): assert the real path logger emits compact mismatch
2063+
// fields for Shadow mode.
2064+
let _ = result;
2065+
Ok(())
2066+
}
2067+
2068+
// What it tests: Shadow path dispatch emits comparison rows for AclManifest-only restrictions.
2069+
// Expected: config aggregate fields stay unrestricted while the mismatch summary reports the restriction.
2070+
#[mononoke::fbinit_test]
2071+
async fn test_shadow_path_dispatch_logs_acl_manifest_only_restriction(
2072+
fb: FacebookInit,
2073+
) -> Result<()> {
2074+
let restricted_acl = MononokeIdentity::from_str("REPO_REGION:restricted_acl")?;
2075+
2076+
let restricted_root = NonRootMPath::new("acl_manifest_only/dir")?;
2077+
let result = RestrictedPathsTestDataBuilder::new()
2078+
.with_acl_manifest_mode(AclManifestMode::Shadow)
2079+
.with_use_acl_manifest(false)
2080+
.with_acl_manifest_restricted_paths(vec![(restricted_root, restricted_acl)])
2081+
.with_file_path_changes(vec![("acl_manifest_only/dir/a", None)])
2082+
.build(fb)
2083+
.await?
2084+
.observe_restricted_paths_scenario(&[])
2085+
.await?;
2086+
2087+
// TODO(T248660053): assert AclManifest-only restricted paths emit
2088+
// comparison rows while config remains authoritative.
2089+
let _ = result;
2090+
Ok(())
2091+
}
2092+
2093+
// What it tests: Shadow path dispatch handles comparison-source lookup errors as logging-only data.
2094+
// Expected: AclManifest errors populate acl_manifest_error without changing request success.
2095+
#[mononoke::fbinit_test]
2096+
async fn test_shadow_path_dispatch_logs_comparison_errors(fb: FacebookInit) -> Result<()> {
2097+
let restricted_acl = MononokeIdentity::from_str("REPO_REGION:restricted_acl")?;
2098+
2099+
let restricted_root = NonRootMPath::new("restricted/dir")?;
2100+
let missing_cs_id =
2101+
ChangesetId::from_str("1111111111111111111111111111111111111111111111111111111111111111")?;
2102+
let result = RestrictedPathsTestDataBuilder::new()
2103+
.with_acl_manifest_mode(AclManifestMode::Shadow)
2104+
.with_use_acl_manifest(false)
2105+
.with_config_restricted_paths(vec![(restricted_root.clone(), restricted_acl)])
2106+
.build(fb)
2107+
.await?
2108+
.observe_path_access(restricted_root, Some(missing_cs_id), &[])
2109+
.await?;
2110+
2111+
// TODO(T248660053): inject an AclManifest path lookup failure and assert
2112+
// it is logged without failing the request.
2113+
let _ = result;
2114+
Ok(())
2115+
}
2116+
2117+
// What it tests: Shadow path dispatch skips AclManifest comparison when a changeset id is unavailable.
2118+
// Expected: skipped comparison leaves acl_manifest_error and mismatch detail empty.
2119+
#[mononoke::fbinit_test]
2120+
async fn test_shadow_path_dispatch_skips_acl_manifest_without_changeset(
2121+
fb: FacebookInit,
2122+
) -> Result<()> {
2123+
let restricted_acl = MononokeIdentity::from_str("REPO_REGION:restricted_acl")?;
2124+
2125+
let restricted_root = NonRootMPath::new("restricted/dir")?;
2126+
let result = RestrictedPathsTestDataBuilder::new()
2127+
.with_acl_manifest_mode(AclManifestMode::Shadow)
2128+
.with_use_acl_manifest(false)
2129+
.with_config_restricted_paths(vec![(restricted_root.clone(), restricted_acl.clone())])
2130+
.with_acl_manifest_restricted_paths(vec![(restricted_root.clone(), restricted_acl)])
2131+
.build(fb)
2132+
.await?
2133+
.observe_path_access(restricted_root, None, &[])
2134+
.await?;
2135+
2136+
// TODO(T248660053): call path access without a changeset id and assert
2137+
// AclManifest comparison is skipped.
2138+
let _ = result;
2139+
Ok(())
2140+
}
2141+
2142+
// What it tests: Shadow path dispatch does not log unrestricted rows.
2143+
// Expected: no row is emitted when both sources are unrestricted.
2144+
#[mononoke::fbinit_test]
2145+
async fn test_shadow_path_dispatch_omits_unrestricted_rows(fb: FacebookInit) -> Result<()> {
2146+
let result = RestrictedPathsTestDataBuilder::new()
2147+
.with_acl_manifest_mode(AclManifestMode::Shadow)
2148+
.with_use_acl_manifest(false)
2149+
.with_file_path_changes(vec![("unrestricted/dir/a", None)])
2150+
.build(fb)
2151+
.await?
2152+
.observe_restricted_paths_scenario(&[])
2153+
.await?;
2154+
2155+
// TODO(T248660053): assert no Scuba rows are emitted when config and
2156+
// AclManifest both report unrestricted.
2157+
let _ = result;
2158+
Ok(())
2159+
}

0 commit comments

Comments
 (0)