From 727e15c053dde0a48ccf7bda923370ea015e45a6 Mon Sep 17 00:00:00 2001 From: Nicholas Petrasek Date: Fri, 23 Oct 2026 20:20:51 +0200 Subject: [PATCH 01/10] fix: left people expect nothing even when a directory group assigns them --- crates/permesh-cli/src/profile_command.rs | 9 ++----- crates/permesh-cli/tests/profiles.rs | 29 +++++++++++++++++++++++ crates/permesh-core/src/profiles.rs | 9 ++++++- crates/permesh-core/tests/access_plan.rs | 1 + crates/permesh-core/tests/profiles.rs | 1 + 5 files changed, 41 insertions(+), 8 deletions(-) diff --git a/crates/permesh-cli/src/profile_command.rs b/crates/permesh-cli/src/profile_command.rs index 9567983..b664d7e 100644 --- a/crates/permesh-cli/src/profile_command.rs +++ b/crates/permesh-cli/src/profile_command.rs @@ -92,6 +92,7 @@ pub(crate) fn input(set: &ProfileSet, artifact: &Artifact) -> core::ProfileInput profiles: d.profiles.clone(), }) .collect(), + left: set.left.clone(), observable: artifact .providers .iter() @@ -116,13 +117,7 @@ pub(crate) fn review( now: OffsetDateTime, ) -> Result { let (snapshots, aliases) = crate::resource_command::domain(artifact); - // Someone who left is expected to hold nothing: remaining access is unexplained, - // and their former profiles are not missing. - let mut input = input.clone(); - for identity in &set.left { - input.people.insert(identity.clone(), Default::default()); - } - let comparison = core::compare_profiles(&snapshots, &aliases, &input)?; + let comparison = core::compare_profiles(&snapshots, &aliases, input)?; let (mut missing, mut unexplained, mut cannot_tell, mut excepted, mut covered) = (0, 0, 0, 0, 0); let mut identities = Vec::new(); diff --git a/crates/permesh-cli/tests/profiles.rs b/crates/permesh-cli/tests/profiles.rs index 47ae718..4998fb9 100644 --- a/crates/permesh-cli/tests/profiles.rs +++ b/crates/permesh-cli/tests/profiles.rs @@ -323,6 +323,35 @@ fn left_person_expects_nothing_and_remaining_access_is_unexplained() { assert_eq!(bob["unexplained"].as_array().unwrap().len(), 1, "{bob}"); } +#[test] +fn left_person_in_directory_group_expects_nothing() { + let dir = demo(&[ + ("profiles/reader.yaml", READER), + ( + "profiles/admin.yaml", + "profile: admin\ngrants:\n - {instance: demo, role: Admin, resource: repo-21}\n", + ), + ( + "people/alice.yaml", + "identity: alice@example.com\nstatus: left\nprofiles: [reader]\n", + ), + ]); + let mut config = fs::read_to_string(dir.path().join("permesh.yaml")).unwrap(); + config.push_str( + "profiles:\n directory_groups:\n - {instance: demo, group: team-10, profiles: [admin]}\n", + ); + fs::write(dir.path().join("permesh.yaml"), config).unwrap(); + let (code, result) = check(dir.path(), &[]); + assert_eq!(code, Some(6), "{result}"); + let alice = identity(&result, "alice@example.com"); + // A directory group does not bring back profiles of someone who left. + assert_eq!(alice["missing"], json!([]), "{alice}"); + assert!( + !alice["unexplained"].as_array().unwrap().is_empty(), + "{alice}" + ); +} + #[test] fn profile_check_rejects_unknown_version() { let dir = demo(&[ diff --git a/crates/permesh-core/src/profiles.rs b/crates/permesh-core/src/profiles.rs index 244e12b..eeeed05 100644 --- a/crates/permesh-core/src/profiles.rs +++ b/crates/permesh-core/src/profiles.rs @@ -26,6 +26,8 @@ pub struct ProfileInput { pub profiles: BTreeMap>, pub people: BTreeMap>, pub directory_groups: Vec, + /// People who left: compared against no profiles, whatever assigns them. + pub left: BTreeSet, /// Completely observed instances and their declared capabilities. pub observable: BTreeMap>, } @@ -233,10 +235,15 @@ pub fn compare_profiles( let index = IdentityIndex::build(snapshots, aliases)?; let graph = MembershipGraph::new(snapshots); let Assignments { - profiles: assigned, + profiles: mut assigned, unresolved_assignments, unresolved_members, } = assign(snapshots, &index, &graph, input)?; + // Someone who left is expected to hold nothing: remaining access is unexplained, + // and their former profiles are not missing. + for identity in &input.left { + assigned.insert(identity.clone(), BTreeSet::new()); + } let assignments_complete = unresolved_assignments.is_empty(); let (owned, claimed, ambiguous_accounts) = owned_accounts(&index); let mut identities = Vec::new(); diff --git a/crates/permesh-core/tests/access_plan.rs b/crates/permesh-core/tests/access_plan.rs index 547e462..392cc92 100644 --- a/crates/permesh-core/tests/access_plan.rs +++ b/crates/permesh-core/tests/access_plan.rs @@ -167,6 +167,7 @@ fn input(profiles: &[(&str, Vec)]) -> ProfileInput { .collect(), people: BTreeMap::new(), directory_groups: vec![], + left: BTreeSet::new(), observable: observable(), } } diff --git a/crates/permesh-core/tests/profiles.rs b/crates/permesh-core/tests/profiles.rs index f1b9c3e..a300628 100644 --- a/crates/permesh-core/tests/profiles.rs +++ b/crates/permesh-core/tests/profiles.rs @@ -161,6 +161,7 @@ fn input(profile: Vec, people: &[&str]) -> ProfileInput { .map(|p| (p.to_string(), ["backend".to_string()].into())) .collect(), directory_groups: vec![], + left: BTreeSet::new(), observable: observable(), } } From f8822b41659438dfddf92fa24b15404056e98d05 Mon Sep 17 00:00:00 2001 From: Nicholas Petrasek Date: Fri, 23 Oct 2026 20:21:42 +0200 Subject: [PATCH 02/10] fix: escape Markdown in the detailed access review summary --- crates/permesh-cli/src/access/review.rs | 8 ++++---- crates/permesh-cli/src/output.rs | 13 +++++++++++++ crates/permesh-cli/tests/access.rs | 22 ++++++++++++++++++++++ 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/crates/permesh-cli/src/access/review.rs b/crates/permesh-cli/src/access/review.rs index 72a96d3..7746c20 100644 --- a/crates/permesh-cli/src/access/review.rs +++ b/crates/permesh-cli/src/access/review.rs @@ -11,7 +11,7 @@ use crate::{ artifact::{self, Artifact}, error::AppError, offboard, - output::safe, + output::markdown, }; use permesh_config::profiles::ProfileSet; use permesh_core as core; @@ -325,12 +325,12 @@ pub(crate) fn summary( let kind = serde_json::to_value(row.kind).unwrap_or_default(); text.push_str(&format!( "\n## {} {}\n\n", - safe(kind.as_str().unwrap_or_default()), - safe(&row.identity) + markdown(kind.as_str().unwrap_or_default()), + markdown(&row.identity) )); for step in &plan.steps { let operation = serde_json::to_value(&step.operation).unwrap_or_default(); - text.push_str(&format!("- {}\n", words(&operation))); + text.push_str(&format!("- {}\n", markdown(&words(&operation)))); } } } diff --git a/crates/permesh-cli/src/output.rs b/crates/permesh-cli/src/output.rs index 2714881..898e5b3 100644 --- a/crates/permesh-cli/src/output.rs +++ b/crates/permesh-cli/src/output.rs @@ -14,6 +14,19 @@ pub(crate) fn safe(value: &str) -> String { }) .collect() } +/// `safe`, then backslash-escape Markdown syntax so provider names render as text. +pub(crate) fn markdown(value: &str) -> String { + safe(value) + .chars() + .flat_map(|c| { + if "\\`*_{}[]()<>#!|~&".contains(c) { + vec!['\\', c] + } else { + vec![c] + } + }) + .collect() +} pub(crate) fn field<'a>(value: &'a serde_json::Value, name: &str) -> &'a str { value[name].as_str().unwrap_or("") } diff --git a/crates/permesh-cli/tests/access.rs b/crates/permesh-cli/tests/access.rs index 7b55235..6658918 100644 --- a/crates/permesh-cli/tests/access.rs +++ b/crates/permesh-cli/tests/access.rs @@ -802,6 +802,28 @@ fn review_plans_changed_people_only() { assert!(summary.contains("bob@example.com"), "{summary}"); } +#[test] +fn detail_summary_escapes_markdown() { + let identity = "x_[a](https://e.example)@example.com"; + let head = demo(&[ + ("profiles/backend.yaml", BACKEND), + ( + "people/x.yaml", + &format!("identity: '{identity}'\nprofiles: [backend]\n"), + ), + ]); + let base = base_checkout(head.path(), &[("profiles/backend.yaml", BACKEND)]); + let (code, result) = review(head.path(), base.path(), "detailed", &["--detail"]); + assert_eq!(code, Some(0), "{result}"); + let summary = fs::read_to_string(head.path().join("detailed/summary.md")).unwrap(); + // Names render as text: no HTML, no link. + assert!( + summary.contains(r"\x\\_\[a\]\(https://e.example\)@example.com"), + "{summary}" + ); + assert!(!summary.contains("") && !summary.contains("]("), "{summary}"); +} + #[test] fn deleted_people_file_is_a_move_and_left_is_a_leave() { let head = demo(&[ From 308c8245044424e4ab05dce88cd519daa8f3b902 Mon Sep 17 00:00:00 2001 From: Nicholas Petrasek Date: Fri, 23 Oct 2026 20:22:10 +0200 Subject: [PATCH 03/10] fix: refuse existing plan files before the review capture --- crates/permesh-cli/src/access/review.rs | 12 ++++++++++- crates/permesh-cli/tests/access.rs | 27 +++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/crates/permesh-cli/src/access/review.rs b/crates/permesh-cli/src/access/review.rs index 7746c20..d3679b7 100644 --- a/crates/permesh-cli/src/access/review.rs +++ b/crates/permesh-cli/src/access/review.rs @@ -337,10 +337,20 @@ pub(crate) fn summary( text } -/// Fail before capture when the fixed output files already exist. +/// Fail before capture when output files already exist, plans included. pub(crate) fn check_out(out: &Path, overwrite: bool) -> Result<(), AppError> { for name in [INDEX, SUMMARY] { artifact::check_destination(&out.join(name), overwrite)?; } + let Ok(entries) = std::fs::read_dir(out) else { + return Ok(()); + }; + for entry in entries.flatten() { + let name = entry.file_name(); + let name = name.to_string_lossy(); + if valid_file(&name.replace(".execution.json", ".json")) { + artifact::check_destination(&entry.path(), overwrite)?; + } + } Ok(()) } diff --git a/crates/permesh-cli/tests/access.rs b/crates/permesh-cli/tests/access.rs index 6658918..31809da 100644 --- a/crates/permesh-cli/tests/access.rs +++ b/crates/permesh-cli/tests/access.rs @@ -824,6 +824,33 @@ fn detail_summary_escapes_markdown() { assert!(!summary.contains("") && !summary.contains("]("), "{summary}"); } +#[test] +fn existing_plan_file_is_refused_before_capture() { + let head = demo(&[ + ("profiles/backend.yaml", BACKEND), + ("people/bob.yaml", &person("bob@example.com", "backend")), + ]); + let base = base_checkout(head.path(), &[("profiles/backend.yaml", BACKEND)]); + fs::create_dir_all(head.path().join("plans")).unwrap(); + fs::write(head.path().join("plans/000.json"), "{}").unwrap(); + // The missing snapshot would fail the capture; the output check must come first. + let (code, result) = review( + head.path(), + base.path(), + "plans", + &["--snapshot", "missing.json"], + ); + assert_eq!(code, Some(2), "{result}"); + assert!( + result["error"]["message"] + .as_str() + .unwrap() + .starts_with("Output exists"), + "{result}" + ); + assert_eq!(fs::read_to_string(head.path().join("plans/000.json")).unwrap(), "{}"); +} + #[test] fn deleted_people_file_is_a_move_and_left_is_a_leave() { let head = demo(&[ From 577e797318928af51ad4e6e5c8a128b4a2afa83c Mon Sep 17 00:00:00 2001 From: Nicholas Petrasek Date: Fri, 23 Oct 2026 20:24:20 +0200 Subject: [PATCH 04/10] fix: verify workflow accepts only post-merge plan runs and keeps exit precedence; document GitLab variables --- crates/permesh-cli/tests/starter.rs | 109 ++++++++++++++++++ docs/access-plans.md | 3 +- docs/profiles.md | 5 +- docs/starter.md | 10 +- .../.github/workflows/permesh-verify.yml | 14 ++- examples/starter/.gitlab-ci.yml | 4 +- 6 files changed, 138 insertions(+), 7 deletions(-) diff --git a/crates/permesh-cli/tests/starter.rs b/crates/permesh-cli/tests/starter.rs index c775a8c..aa2b62f 100644 --- a/crates/permesh-cli/tests/starter.rs +++ b/crates/permesh-cli/tests/starter.rs @@ -320,3 +320,112 @@ fn post_merge_job_binds_only_the_merged_pull_request() { let group = workflow["concurrency"]["group"].as_str().unwrap(); assert!(group.contains("github.sha"), "{group}"); } + +/// The `run` script of one step of the verify workflow. +#[cfg(unix)] +fn verify_step(name: &str) -> String { + let parsed = yaml(".github/workflows/permesh-verify.yml"); + parsed["jobs"]["verify"]["steps"] + .as_array() + .unwrap() + .iter() + .find(|s| s["name"] == name) + .unwrap()["run"] + .as_str() + .unwrap() + .to_owned() +} +/// Runs a workflow script with `bin` first on PATH, as GitHub runs `run:` steps. +#[cfg(unix)] +fn run_step(dir: &Path, bin: &Path, script: &str, env: &[(&str, &str)]) -> Option { + let path = format!("{}:{}", bin.display(), std::env::var("PATH").unwrap()); + std::process::Command::new("sh") + .args(["-e", "-c", script]) + .current_dir(dir) + .env("PATH", path) + .env("GITHUB_STEP_SUMMARY", dir.join("summary.md")) + .env("GITHUB_REPOSITORY", "acme/access") + .envs(env.iter().copied()) + .status() + .unwrap() + .code() +} +#[cfg(unix)] +fn fake(bin: &Path, name: &str, body: &str) { + use std::os::unix::fs::PermissionsExt; + fs::create_dir_all(bin).unwrap(); + let file = bin.join(name); + fs::write(&file, format!("#!/bin/sh\n{body}")).unwrap(); + fs::set_permissions(&file, fs::Permissions::from_mode(0o755)).unwrap(); +} + +#[cfg(unix)] +#[test] +fn verify_workflow_exit_follows_precedence() { + let script = verify_step("Verify each plan"); + for (codes, expected) in [ + (&[6, 4, 0][..], 4), + (&[6, 3, 4][..], 3), + (&[0, 6][..], 6), + (&[2, 3][..], 2), + (&[0, 0][..], 0), + ] { + let dir = tempfile::tempdir().unwrap(); + let bin = dir.path().join("bin"); + // `permesh access verify PLAN --json` exits with the code stored beside PLAN. + fake( + &bin, + "permesh", + "echo '{\"result\":{\"counts\":{}}}'\nexit \"$(cat \"$3.code\")\"\n", + ); + fs::create_dir_all(dir.path().join("execution")).unwrap(); + for (n, code) in codes.iter().enumerate() { + let plan = dir.path().join(format!("execution/{n:03}.execution.json")); + fs::write(&plan, "{}").unwrap(); + fs::write(plan.with_extension("json.code"), code.to_string()).unwrap(); + } + let code = run_step(dir.path(), &bin, &script, &[("RUN_ID", "1")]); + assert_eq!(code, Some(expected), "{codes:?}"); + let summary = fs::read_to_string(dir.path().join("summary.md")).unwrap(); + assert_eq!(summary.lines().count(), codes.len(), "{summary}"); + } +} + +#[cfg(unix)] +#[test] +fn verify_workflow_accepts_only_post_merge_plan_runs() { + let script = verify_step("Fetch the execution plans"); + let good = serde_json::json!({ + "path": ".github/workflows/permesh-plan.yml", + "event": "push", + "head_branch": "main", + "conclusion": "success", + "repository": {"id": 7}, + "head_repository": {"id": 7} + }); + let mut cases = vec![(good.clone(), true)]; + for (field, value) in [ + ("path", serde_json::json!(".github/workflows/permesh-check.yml")), + ("event", serde_json::json!("pull_request")), + ("head_branch", serde_json::json!("feature")), + ("conclusion", serde_json::json!("failure")), + ("head_repository", serde_json::json!({"id": 8})), + ] { + let mut run = good.clone(); + run[field] = value; + cases.push((run, false)); + } + for (run, accepted) in cases { + let dir = tempfile::tempdir().unwrap(); + let bin = dir.path().join("bin"); + fs::write(dir.path().join("run.fixture"), run.to_string()).unwrap(); + fake( + &bin, + "gh", + "case \"$1 $2\" in\n 'api repos/acme/access') echo '{\"default_branch\":\"main\"}' ;;\n 'api repos/acme/access/actions/runs/42') cat run.fixture ;;\n 'run download') touch downloaded ;;\n *) exit 9 ;;\nesac\n", + ); + let code = run_step(dir.path(), &bin, &script, &[("RUN_ID", "42")]); + assert_eq!(code == Some(0), accepted, "{run}"); + assert_eq!(dir.path().join("downloaded").exists(), accepted, "{run}"); + } +} diff --git a/docs/access-plans.md b/docs/access-plans.md index a9bb932..e9f21db 100644 --- a/docs/access-plans.md +++ b/docs/access-plans.md @@ -127,7 +127,8 @@ sensitive access metadata; keep them out of public places. A step is `cannot_tell` when its provider was not fully read, or when an account the person may own is claimed by another identity too. Either makes the plan -incomplete (exit 4). +incomplete (exit 4). Completeness covers what each provider reports: a provider that +has no `groups` capability has no group routes to remove. ## Pull-request flow diff --git a/docs/profiles.md b/docs/profiles.md index 8ff0b85..ea2ac47 100644 --- a/docs/profiles.md +++ b/docs/profiles.md @@ -66,8 +66,9 @@ profiles: People assigned through a directory group and a people file get the union of their profiles. `status: left` marks an explicit departure: `permesh check` counts that -person's remaining access as departed, `profile check` expects nothing for them -(remaining access is unexplained, not their old profiles missing), and +person's remaining access as departed, `profile check` expects nothing for them, +even when a directory group still assigns them profiles (remaining access is +unexplained, not their old profiles missing), and `permesh access review` plans a leave. To turn a change in someone's profiles into a reviewable plan, see [joiner, mover and leaver plans](access-plans.md). Only `*.yaml` files are read; directories that hold nothing else count as diff --git a/docs/starter.md b/docs/starter.md index d5b1ce1..bc371b2 100644 --- a/docs/starter.md +++ b/docs/starter.md @@ -60,14 +60,20 @@ before carrying out any step. A push that changes people files without a pull request fails, because nothing was reviewed. When the steps are done, start `permesh-verify.yml` from the Actions tab with the -post-merge run ID. It verifies each execution plan, writes counts to the step -summary and fails after all plans ran if any is not done or incomplete. +post-merge run ID. It only accepts a successful post-merge `permesh-plan.yml` run on +the default branch of this repository. It verifies each execution plan, writes +counts to the step summary and, after all plans ran, exits like the CLI would for +the worst one: an error first, then 3, 4 and 6. Pull-request jobs from branches in the same repository receive the provider secrets, so restrict who can push branches. Pull requests from forks get no secrets and their plan job fails. If your default branch is not `main`, change `branches:` in `permesh-plan.yml`. +On GitLab, don't mark the provider variables **Protected**: protected variables +only reach pipelines on protected branches, so `permesh-plan` would get no token on +a merge request branch and exit 3. Restrict who can push branches instead. + GitLab ships the merge-request job only; run the post-merge steps by hand as in [access plans](access-plans.md#pull-request-flow). diff --git a/examples/starter/.github/workflows/permesh-verify.yml b/examples/starter/.github/workflows/permesh-verify.yml index f591e13..b3f357f 100644 --- a/examples/starter/.github/workflows/permesh-verify.yml +++ b/examples/starter/.github/workflows/permesh-verify.yml @@ -50,6 +50,16 @@ jobs: case "$RUN_ID" in ''|*[!0-9]*) echo "run_id must be a numeric workflow run ID." >&2; exit 2 ;; esac + # Accept only a successful post-merge run of permesh-plan.yml on the default + # branch of this repository, so no branch or fork can supply the plans. + default_branch=$(gh api "repos/${GITHUB_REPOSITORY}" | jq -r .default_branch) + gh api "repos/${GITHUB_REPOSITORY}/actions/runs/${RUN_ID}" > run.json + jq -e --arg branch "$default_branch" '.path == ".github/workflows/permesh-plan.yml" + and .event == "push" and .head_branch == $branch and .conclusion == "success" + and .head_repository.id == .repository.id' run.json > /dev/null || { + echo "Run $RUN_ID is not a successful post-merge Permesh plan run on $default_branch." >&2 + exit 2 + } gh run download "$RUN_ID" -n execution-plans -D execution # Every plan runs before the job fails; the summary carries counts only. - name: Verify each plan @@ -58,6 +68,8 @@ jobs: PERMESH_GITHUB_TOKEN: ${{ secrets.PERMESH_GITHUB_TOKEN }} PERMESH_GOOGLE_TOKEN: ${{ secrets.PERMESH_GOOGLE_TOKEN }} run: | + # The job exit follows the CLI's precedence: errors, then 3, 4 and 6. + rank() { case "$1" in 0) echo 0 ;; 6) echo 1 ;; 4) echo 2 ;; 3) echo 3 ;; *) echo 4 ;; esac; } mkdir -p verification status=0 found=0 @@ -72,7 +84,7 @@ jobs: jq -r --arg name "$name" --arg code "$code" \ '"\($name): exit \($code); done \(.result.counts.done // 0), not done \(.result.counts.not_done // 0), cannot verify \(.result.counts.cannot_verify // 0), extra \(.result.counts.extra // 0)"' \ "verification/$name.json" >> "$GITHUB_STEP_SUMMARY" || echo "$name: exit $code" >> "$GITHUB_STEP_SUMMARY" - [ "$code" = 0 ] || status=$code + [ "$(rank "$code")" -gt "$(rank "$status")" ] && status=$code done [ "$found" = 1 ] || { echo "No execution plans in run $RUN_ID." >&2; exit 1; } exit "$status" diff --git a/examples/starter/.gitlab-ci.yml b/examples/starter/.gitlab-ci.yml index d3aa58d..b1f0637 100644 --- a/examples/starter/.gitlab-ci.yml +++ b/examples/starter/.gitlab-ci.yml @@ -5,7 +5,9 @@ # # CI/CD variables (masked): PERMESH_GITHUB_TOKEN, PERMESH_GOOGLE_TOKEN, ... for the # env:// references in permesh.yaml; optional PERMESH_ISSUE_TOKEN (project access -# token with api scope) to keep a review issue updated. +# token with api scope) to keep a review issue updated. Don't mark the provider +# variables Protected: merge request pipelines on unprotected branches would not get +# them, and permesh-plan would exit 3. # Keep this project private: job artifacts hold the full access report. variables: # A Permesh release that includes `permesh check`, and the SHA-256 of its From d529940637dc7403a967f7f42ce8cf6d8fca9846 Mon Sep 17 00:00:00 2001 From: Nicholas Petrasek Date: Fri, 23 Oct 2026 20:27:24 +0200 Subject: [PATCH 05/10] fix: MCP start errors, symlink answers, empty profile lists and instructions; complete the MCP docs --- crates/permesh-cli/src/mcp/mod.rs | 7 +++ crates/permesh-cli/src/mcp/paths.rs | 27 ++++++----- crates/permesh-cli/src/mcp/server.rs | 2 +- crates/permesh-cli/src/mcp/tools.rs | 53 ++++++++++++++++++++- crates/permesh-cli/tests/access.rs | 10 +++- crates/permesh-cli/tests/mcp.rs | 67 ++++++++++++++++++++++++++- crates/permesh-cli/tests/starter.rs | 5 +- docs/adr/0035-read-only-mcp-server.md | 20 +++++++- docs/mcp.md | 19 +++++--- 9 files changed, 184 insertions(+), 26 deletions(-) diff --git a/crates/permesh-cli/src/mcp/mod.rs b/crates/permesh-cli/src/mcp/mod.rs index 34b928a..c98fa87 100644 --- a/crates/permesh-cli/src/mcp/mod.rs +++ b/crates/permesh-cli/src/mcp/mod.rs @@ -39,6 +39,13 @@ pub(crate) async fn serve(cli: &Cli, blocking: &BlockingPool) -> Result<(), AppE let running = match session { Ok(running) => running, Err(ServerInitializeError::ConnectionClosed(_)) => return Ok(()), + Err(ServerInitializeError::Cancelled) => return Err(AppError::new(130, "Cancelled")), + Err(ServerInitializeError::TransportError { .. }) => { + return Err(AppError::new( + 5, + "The MCP session could not start: Permesh could not reach the assistant over stdio.", + )); + } Err(_) => { return Err(AppError::input( "The MCP session could not start. permesh mcp supports MCP protocol 2026-07-28 only; update the assistant or its MCP client.", diff --git a/crates/permesh-cli/src/mcp/paths.rs b/crates/permesh-cli/src/mcp/paths.rs index ff77b38..4eff427 100644 --- a/crates/permesh-cli/src/mcp/paths.rs +++ b/crates/permesh-cli/src/mcp/paths.rs @@ -57,7 +57,8 @@ fn canonical_root(root: &Path) -> Result { std::fs::canonicalize(root).map_err(|_| AppError::input(MISSING)) } -/// An input file inside the root; symlinks are followed and must stay inside. +/// An input file inside the root; symlinks are followed and must stay inside. A link +/// that leaves the root reads as missing, so it never tells whether its target exists. pub(crate) fn existing(root: &Path, relative: &str) -> Result { let path = lexical(relative)?; let root = canonical_root(root)?; @@ -65,7 +66,7 @@ pub(crate) fn existing(root: &Path, relative: &str) -> Result if resolved.starts_with(&root) { Ok(resolved) } else { - Err(AppError::input(OUTSIDE)) + Err(AppError::input(MISSING)) } } @@ -87,7 +88,7 @@ pub(crate) fn new_file(root: &Path, relative: &str) -> Result if parent.starts_with(&root) { Ok(parent.join(name)) } else { - Err(AppError::input(OUTSIDE)) + Err(AppError::input(MISSING)) } } @@ -140,14 +141,18 @@ mod tests { let dir = tempfile::tempdir().unwrap(); std::os::unix::fs::symlink(outside.path().join("file"), dir.path().join("link")).unwrap(); std::os::unix::fs::symlink(outside.path(), dir.path().join("out")).unwrap(); - assert_eq!( - message(existing(dir.path(), "link")), - "Path is outside the workspace" - ); - assert_eq!( - message(new_file(dir.path(), "out/new.json")), - "Path is outside the workspace" - ); + std::os::unix::fs::symlink(outside.path().join("none"), dir.path().join("gone")).unwrap(); + std::os::unix::fs::symlink(outside.path().join("none"), dir.path().join("nodir")).unwrap(); + // Same answer whether the target outside exists, so links reveal nothing about it. + for (link, gone) in [("link", "gone"), ("out/new.json", "nodir/new.json")] { + let (link, gone) = if link == "link" { + (existing(dir.path(), link), existing(dir.path(), gone)) + } else { + (new_file(dir.path(), link), new_file(dir.path(), gone)) + }; + assert_eq!(message(link), "File not found in the workspace"); + assert_eq!(message(gone), "File not found in the workspace"); + } } #[test] diff --git a/crates/permesh-cli/src/mcp/server.rs b/crates/permesh-cli/src/mcp/server.rs index 3990cdc..584a7ec 100644 --- a/crates/permesh-cli/src/mcp/server.rs +++ b/crates/permesh-cli/src/mcp/server.rs @@ -15,7 +15,7 @@ use rmcp::{ }; use std::{borrow::Cow, path::PathBuf, sync::Arc}; -pub(crate) const INSTRUCTIONS: &str = "Permesh answers who has access to what, read-only. Every tool result has `complete`: when it is false, some providers could not be fully read and missing access is not proof of absence, so say you cannot tell. Plan tools write a new plan file for a person to review and carry out; Permesh never changes access."; +pub(crate) const INSTRUCTIONS: &str = "Permesh answers who has access to what, read-only. Successful tool results have `report.complete`: when it is false, some providers could not be fully read and missing access is not proof of absence, so say you cannot tell. Plan tools write a new plan file for a person to review and carry out; Permesh never changes access."; const VERSIONS: &[ProtocolVersion] = &[ProtocolVersion::V_2026_07_28]; pub(crate) struct Server { diff --git a/crates/permesh-cli/src/mcp/tools.rs b/crates/permesh-cli/src/mcp/tools.rs index 3637776..4ff0265 100644 --- a/crates/permesh-cli/src/mcp/tools.rs +++ b/crates/permesh-cli/src/mcp/tools.rs @@ -135,6 +135,20 @@ fn plan_properties(mut properties: Value) -> Value { fn names(description: &str) -> Value { json!({"type": "array", "items": {"type": "string"}, "description": description}) } +/// A required profile list, which the plan command needs at least one name in. +fn required_names(description: &str) -> Value { + let mut value = names(description); + value["minItems"] = json!(1); + value +} +fn at_least_one(field: &str, values: &[String]) -> Result<(), AppError> { + if values.is_empty() { + return Err(AppError::input(format!( + "Name at least one profile in {field}" + ))); + } + Ok(()) +} const SNAPSHOT: &str = "Saved snapshot to read offline instead of fresh discovery; a path relative to the workspace."; @@ -194,7 +208,7 @@ pub(crate) fn definitions() -> Vec { "plan_join", "Prepare a reviewable plan file of the access a person needs for added profiles, including accounts still to create. Permesh never applies it.", object( - plan_properties(json!({"profiles": names("Profiles to add.")})), + plan_properties(json!({"profiles": required_names("Profiles to add.")})), &["person", "profiles", "output"], ), ), @@ -203,7 +217,7 @@ pub(crate) fn definitions() -> Vec { "Prepare a reviewable plan file for a profile change: removals only for access no remaining profile justifies, and shared access listed as kept. Permesh never applies it.", object( plan_properties( - json!({"remove": names("Profiles to take away."), "add": names("Profiles to add.")}), + json!({"remove": required_names("Profiles to take away."), "add": names("Profiles to add.")}), ), &["person", "remove", "output"], ), @@ -298,12 +312,14 @@ impl Tool { argv.extend(file("policy", &args.policy)?); } Tool::PlanJoin(args) => { + at_least_one("profiles", &args.profiles)?; argv.extend(["access".into(), "join".into()]); argv.extend(args.profiles.iter().map(|p| option("profile", p))); argv.extend(common(&args.output, &args.snapshot, args.expires_in_hours)?); argv.extend(["--".into(), (&args.person).into()]); } Tool::PlanMove(args) => { + at_least_one("remove", &args.remove)?; argv.extend(["access".into(), "move".into()]); argv.extend(args.remove.iter().map(|p| option("remove", p))); argv.extend(args.add.iter().map(|p| option("add", p))); @@ -456,4 +472,37 @@ mod tests { ); } } + + #[test] + fn empty_profile_lists_name_the_argument() { + let dir = tempfile::tempdir().unwrap(); + let workspace = dir.path().join("permesh.yaml"); + let join = Tool::PlanJoin(JoinArgs { + person: "a@example.com".into(), + profiles: vec![], + output: "new.json".into(), + snapshot: None, + expires_in_hours: None, + }); + let error = join.argv(&workspace).unwrap_err(); + assert_eq!(error.message, "Name at least one profile in profiles"); + let movement = Tool::PlanMove(MoveArgs { + person: "a@example.com".into(), + remove: vec![], + add: vec!["b".into()], + output: "new.json".into(), + snapshot: None, + expires_in_hours: None, + }); + let error = movement.argv(&workspace).unwrap_err(); + assert_eq!(error.message, "Name at least one profile in remove"); + let listed = definitions(); + let schema = |name: &str, field: &str| { + listed.iter().find(|t| t.name == name).unwrap().input_schema["properties"][field] + ["minItems"] + .clone() + }; + assert_eq!(schema("plan_join", "profiles"), json!(1)); + assert_eq!(schema("plan_move", "remove"), json!(1)); + } } diff --git a/crates/permesh-cli/tests/access.rs b/crates/permesh-cli/tests/access.rs index 31809da..96d7ab9 100644 --- a/crates/permesh-cli/tests/access.rs +++ b/crates/permesh-cli/tests/access.rs @@ -821,7 +821,10 @@ fn detail_summary_escapes_markdown() { summary.contains(r"\x\\_\[a\]\(https://e.example\)@example.com"), "{summary}" ); - assert!(!summary.contains("") && !summary.contains("]("), "{summary}"); + assert!( + !summary.contains("") && !summary.contains("]("), + "{summary}" + ); } #[test] @@ -848,7 +851,10 @@ fn existing_plan_file_is_refused_before_capture() { .starts_with("Output exists"), "{result}" ); - assert_eq!(fs::read_to_string(head.path().join("plans/000.json")).unwrap(), "{}"); + assert_eq!( + fs::read_to_string(head.path().join("plans/000.json")).unwrap(), + "{}" + ); } #[test] diff --git a/crates/permesh-cli/tests/mcp.rs b/crates/permesh-cli/tests/mcp.rs index 5db90e7..2191baf 100644 --- a/crates/permesh-cli/tests/mcp.rs +++ b/crates/permesh-cli/tests/mcp.rs @@ -22,7 +22,7 @@ const TOOLS: &[&str] = &[ "verify_plan", ]; const VERSION: &str = "2026-07-28"; -const INSTRUCTIONS: &str = "Permesh answers who has access to what, read-only. Every tool result has `complete`: when it is false, some providers could not be fully read and missing access is not proof of absence, so say you cannot tell. Plan tools write a new plan file for a person to review and carry out; Permesh never changes access."; +const INSTRUCTIONS: &str = "Permesh answers who has access to what, read-only. Successful tool results have `report.complete`: when it is false, some providers could not be fully read and missing access is not proof of absence, so say you cannot tell. Plan tools write a new plan file for a person to review and carry out; Permesh never changes access."; fn command(dir: &Path) -> Command { let mut command = Command::new(env!("CARGO_BIN_EXE_permesh")); @@ -183,6 +183,36 @@ fn legacy_initialize_is_rejected() { ); } +#[cfg(unix)] +#[test] +fn closed_output_is_not_blamed_on_protocol() { + let dir = demo(); + let mut child = command(dir.path()) + .arg("mcp") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + // Nobody reads the answer: replying to the client fails. + drop(child.stdout.take()); + let mut stdin = child.stdin.take().unwrap(); + writeln!( + stdin, + "{}", + json!({"jsonrpc": "2.0", "id": 1, "method": "initialize", + "params": {"protocolVersion": "2025-11-25", "capabilities": {}, + "clientInfo": {"name": "old", "version": "0"}}}) + ) + .unwrap(); + drop(stdin); + let output = child.wait_with_output().unwrap(); + let stderr = String::from_utf8_lossy(&output.stderr); + assert_eq!(output.status.code(), Some(5), "{stderr}"); + assert!(stderr.contains("could not reach the assistant"), "{stderr}"); + assert!(!stderr.contains("2026-07-28"), "{stderr}"); +} + #[test] fn missing_meta_is_invalid_params() { let dir = demo(); @@ -729,6 +759,39 @@ fn cancelled_call_drains_and_server_continues() { } } +#[cfg(unix)] +#[test] +fn cancelled_queued_call_never_starts() { + let (_temporary, root) = slow_workspace(); + let mut client = Client::start(&root); + let running = client.start_request( + "tools/call", + json!({"name": "user_access", "arguments": {"identity": "missing"}}), + ); + let provider = wait_for_provider(&root); + let queued = client.start_request( + "tools/call", + json!({"name": "user_access", "arguments": {"identity": "missing"}}), + ); + for id in [queued, running] { + client.send( + &json!({"jsonrpc": "2.0", "method": "notifications/cancelled", + "params": {"requestId": id, "reason": "test", "_meta": meta()}}), + ); + } + let deadline = std::time::Instant::now() + Duration::from_secs(10); + while alive(&provider) && std::time::Instant::now() < deadline { + std::thread::sleep(Duration::from_millis(20)); + } + assert!(!alive(&provider), "cancelled provider was not reaped"); + // The server stays usable, and the cancelled queued call started no provider. + assert_eq!(client.tool_names().len(), TOOLS.len()); + std::thread::sleep(Duration::from_millis(500)); + let starts = std::fs::read_to_string(root.join("ready.starts")).unwrap(); + assert_eq!(starts.lines().count(), 1, "{starts}"); + let _ = client.finish(); +} + /// A real native provider speaks either legacy protocol, exposes a private /// observation, then hangs after marking its PID. This synchronizes SIGINT with /// active discovery rather than relying on startup timing. @@ -750,6 +813,8 @@ fn main() { std::io::stdout().flush().unwrap(); eprintln!("CANCELLATION_PRIVATE_STDERR"); let marker = std::path::Path::new(MARKER); + let mut starts = std::fs::OpenOptions::new().create(true).append(true).open(marker.with_extension("starts")).unwrap(); + writeln!(starts, "{}", std::process::id()).unwrap(); let temporary = marker.with_extension("tmp"); std::fs::write(&temporary, std::process::id().to_string()).unwrap(); std::fs::rename(temporary, marker).unwrap(); diff --git a/crates/permesh-cli/tests/starter.rs b/crates/permesh-cli/tests/starter.rs index aa2b62f..65425cd 100644 --- a/crates/permesh-cli/tests/starter.rs +++ b/crates/permesh-cli/tests/starter.rs @@ -405,7 +405,10 @@ fn verify_workflow_accepts_only_post_merge_plan_runs() { }); let mut cases = vec![(good.clone(), true)]; for (field, value) in [ - ("path", serde_json::json!(".github/workflows/permesh-check.yml")), + ( + "path", + serde_json::json!(".github/workflows/permesh-check.yml"), + ), ("event", serde_json::json!("pull_request")), ("head_branch", serde_json::json!("feature")), ("conclusion", serde_json::json!("failure")), diff --git a/docs/adr/0035-read-only-mcp-server.md b/docs/adr/0035-read-only-mcp-server.md index d25c339..29b2e55 100644 --- a/docs/adr/0035-read-only-mcp-server.md +++ b/docs/adr/0035-read-only-mcp-server.md @@ -14,8 +14,9 @@ CLI action. becomes an exact `permesh --json` argument vector, which the normal parser and command code run. The server therefore uses the same workspace, approvals and credentials as the CLI. There is no generic pass-through, so no tool can reach - `check` (it sends notifications), `snapshot save`, `identity map`, `auth`, - `provider`, `init`, `offboard` or `access review`. + any other command, such as `check` (it sends notifications), `snapshot`, + `identity`, `auth`, `provider`, `init`, `offboard` or `access review`. The + complete list is in [docs/mcp.md](../mcp.md). - **Tools.** `doctor`, `user_access`, `resource_access`, `admins`, `orphaned`, `profile_check`, `plan_join`, `plan_move`, `plan_leave` and `verify_plan`. - **One write.** The plan tools write a new `.json` plan file, which a person @@ -46,6 +47,21 @@ answers identical to the CLI's and inherits every safety rule they already follo Supporting one protocol revision keeps the surface small while there are no users to stay compatible with. +## Rejected options + +- **A generic command tool** taking any CLI arguments: one tool could then approve + providers, map identities or send notifications. A closed list keeps every + reachable command visible in review. +- **A second implementation** of queries on the library crates: answers could drift + from the CLI's, and every safety rule would need a copy. +- **Network transports** (HTTP or SSE): a listening port needs authentication and + exposes access data beyond the local machine. Stdio is reachable only by the + process that started the server. +- **Older protocol revisions** alongside 2026-07-28: with no users yet, + compatibility code would be surface without benefit. +- **Plan writes anywhere**, or overwriting: a plan in `people/` or `profiles/` breaks + profile loading, and an overwrite could replace a reviewed plan. + ## Consequences The assistant sees the same access data the CLI shows, so it is only as private as diff --git a/docs/mcp.md b/docs/mcp.md index d981e96..de2bba6 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -58,9 +58,13 @@ you see that error. `snapshot` to read a saved snapshot instead of running fresh discovery. See [access plans](access-plans.md) for what plans contain. -Not available through MCP, on purpose: `check` (it sends notifications), -`snapshot save`, `identity map`, `auth`, `provider`, `init`, `offboard` and -`access review`. Use the CLI for those. +Every other command is left out on purpose; use the CLI for it: + +- `init`, `provider`, `auth` and `identity`: they set up the workspace, trust, + credentials and mappings, which stay your decisions. +- `check` (it sends notifications) and `access review` (the pull-request flow). +- `snapshot`, `diff`, `policy`, `offboard` and `profile suggest`. +- `completion`, `version` and `mcp` itself. ## Reading results @@ -85,14 +89,17 @@ access". The server's instructions tell it so. ## Files and paths File arguments (`snapshot`, `policy`, `annotations`, `plan`, `output`) are paths -relative to the folder that holds `permesh.yaml`. Absolute paths, `..` that leaves -the folder and symlinks that point outside it are refused. +relative to the folder that holds `permesh.yaml`. Absolute paths and `..` that leaves +the folder are refused. A symlink that points outside it reads as "File not found in +the workspace", so it never tells whether its target exists. The plan tools are the only tools that write. They create `output` as a new `.json` file; its folder must already exist (for example `plans/`) and the file must not. Plans can't be written into `people/`, `profiles/` or hidden folders, where they would be read as workspace files. Permesh -never overwrites a plan and never carries one out. +never overwrites a plan and never carries one out. The checks run just before the +command; they don't protect against another program on your machine swapping in a +symlink in between, which could already write anywhere you can. ## Privacy From 18a3df9b5caa9125b49f48ebecd9fd3d039d28cd Mon Sep 17 00:00:00 2001 From: Nicholas Petrasek Date: Fri, 23 Oct 2026 20:27:53 +0200 Subject: [PATCH 06/10] fix: name the directory and the CI remedy when a reviewed index is refused --- crates/permesh-cli/src/access/review.rs | 13 +++++++------ crates/permesh-cli/tests/access.rs | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/crates/permesh-cli/src/access/review.rs b/crates/permesh-cli/src/access/review.rs index d3679b7..b04be71 100644 --- a/crates/permesh-cli/src/access/review.rs +++ b/crates/permesh-cli/src/access/review.rs @@ -11,7 +11,7 @@ use crate::{ artifact::{self, Artifact}, error::AppError, offboard, - output::markdown, + output::{markdown, safe}, }; use permesh_config::profiles::ProfileSet; use permesh_core as core; @@ -31,11 +31,12 @@ pub(crate) const INDEX_VERSION: u32 = 1; pub(crate) fn index_value(rows: &[&IndexRow]) -> Value { json!({"access_review_index_version": INDEX_VERSION, "plans": rows}) } -fn index_rows(index: Value) -> Result, AppError> { +fn index_rows(dir: &Path, index: Value) -> Result, AppError> { let unsupported = || { - AppError::input( - "Unsupported index.json format; regenerate the reviewed plans with this Permesh version", - ) + AppError::input(format!( + "Unsupported index.json format in {}; regenerate the reviewed plans with this Permesh version (in the starter CI, re-run the pull request's Permesh plan job)", + safe(&dir.display().to_string()) + )) }; let Value::Object(mut index) = index else { return Err(unsupported()); @@ -209,7 +210,7 @@ fn valid_file(name: &str) -> bool { /// The reviewed plans by (identity, kind), checked against their index. pub(crate) fn load_reviewed(dir: &Path) -> Result { let mismatch = || AppError::input("Reviewed plans do not match their index.json"); - let rows = index_rows(artifact::load_document(&dir.join(INDEX))?)?; + let rows = index_rows(dir, artifact::load_document(&dir.join(INDEX))?)?; let mut reviewed = BTreeMap::new(); for row in rows { if !valid_file(&row.file) { diff --git a/crates/permesh-cli/tests/access.rs b/crates/permesh-cli/tests/access.rs index 96d7ab9..363bfa1 100644 --- a/crates/permesh-cli/tests/access.rs +++ b/crates/permesh-cli/tests/access.rs @@ -1124,7 +1124,7 @@ fn bare_array_index_is_refused() { assert_eq!(code, Some(2), "{result}"); assert_eq!( result["error"]["message"], - "Unsupported index.json format; regenerate the reviewed plans with this Permesh version" + "Unsupported index.json format in r1; regenerate the reviewed plans with this Permesh version (in the starter CI, re-run the pull request's Permesh plan job)" ); assert!(!head.path().join("r2/000.execution.json").exists()); } From 6762c9ca07c6a3ac0bc36a4fa258cf3db47b16ea Mon Sep 17 00:00:00 2001 From: Nicholas Petrasek Date: Fri, 23 Oct 2026 20:29:31 +0200 Subject: [PATCH 07/10] test: pin the move plan and notification results, check renamed plan fields, strengthen the draft reload; clarify fixture guidance --- .../tests/fixtures/stable/README.md | 18 +- .../fixtures/stable/access-plan-move.json | 66 ++++++++ .../stable/check-report-notifications.json | 160 ++++++++++++++++++ crates/permesh-cli/tests/stable_formats.rs | 89 +++++++++- docs/adr/0036-second-stability-declaration.md | 8 +- 5 files changed, 329 insertions(+), 12 deletions(-) create mode 100644 crates/permesh-cli/tests/fixtures/stable/access-plan-move.json create mode 100644 crates/permesh-cli/tests/fixtures/stable/check-report-notifications.json diff --git a/crates/permesh-cli/tests/fixtures/stable/README.md b/crates/permesh-cli/tests/fixtures/stable/README.md index 215d763..f5c10fd 100644 --- a/crates/permesh-cli/tests/fixtures/stable/README.md +++ b/crates/permesh-cli/tests/fixtures/stable/README.md @@ -10,11 +10,15 @@ Only run-specific values are replaced with `""`: string values of ke ending in `_at`, `sha256` or `digest`, numbers under keys ending in `_ms`, and strings starting with the temporary workspace path. Paths under `files` compare with `/` separators, so Windows output matches. Human-readable text (`summary`, -`message` and the `profile suggest` draft) is normalized too; the draft is checked -to load as a profile instead. +`message`, notification failure reasons and the `profile suggest` draft) is +normalized too; the draft is checked to load as a profile instead. -A failing fixture means a stable format changed. Additive changes (new optional -fields) are allowed; anything else needs a version bump and a migration guide in -`docs/migrations/`. After such a reviewed change, regenerate with -`PERMESH_BLESS=1 cargo test -p permesh-cli --test stable_formats` and review the -diff. Never regenerate to make a test pass. +People and profile files, and `permesh.yaml` other than its `check` and `profiles` +sections, are inputs: their parsing tests in `permesh-config` keep them readable. + +A failing fixture means a stable format changed. Because the comparison is exact, +an allowed additive change (a new optional field) fails too: regenerate with +`PERMESH_BLESS=1 cargo test -p permesh-cli --test stable_formats` and check that the +diff only adds. Any other difference (a renamed, removed or retyped field, a new +enum spelling, a different order) needs a version bump and a migration guide in +`docs/migrations/`. Never regenerate to make a test pass. diff --git a/crates/permesh-cli/tests/fixtures/stable/access-plan-move.json b/crates/permesh-cli/tests/fixtures/stable/access-plan-move.json new file mode 100644 index 0000000..2c4088d --- /dev/null +++ b/crates/permesh-cli/tests/fixtures/stable/access-plan-move.json @@ -0,0 +1,66 @@ +{ + "accounts": [ + { + "id": "101", + "provider": "demo" + } + ], + "complete": true, + "created_at": "", + "expires_at": "", + "format": "permesh_access_plan", + "format_version": 1, + "identity": "bob@example.com", + "identity_authorities": [ + "demo" + ], + "identity_bindings": [], + "identity_context_sha256": "", + "kept": [ + { + "because": [ + "base" + ], + "entry": { + "instance": "demo", + "kind": "role", + "resource": "repo-21", + "role": "Read" + } + } + ], + "kind": "move", + "max_age_hours": 24, + "mode": "snapshot_replay", + "profiles_after": [ + "base" + ], + "profiles_removed": [ + "backend" + ], + "profiles_sha256": "", + "reviewed_plan_sha256": null, + "source_snapshot_sha256": "", + "state": "compared", + "steps": [ + { + "evidence": [], + "id": "1dddccdef53956e1f92f7f63edbc2ebf3784c5a985bea0d81caf4d083ae2bbb3", + "justification": { + "profiles": [ + "base" + ] + }, + "operation": { + "instance": "demo", + "resource": "repo-21", + "role": "Read", + "type": "add_role" + }, + "prerequisites": [], + "reason": "not_observed", + "state": "planned", + "verification": "observe_role" + } + ] +} diff --git a/crates/permesh-cli/tests/fixtures/stable/check-report-notifications.json b/crates/permesh-cli/tests/fixtures/stable/check-report-notifications.json new file mode 100644 index 0000000..5041f6e --- /dev/null +++ b/crates/permesh-cli/tests/fixtures/stable/check-report-notifications.json @@ -0,0 +1,160 @@ +{ + "changes": "no_baseline", + "changes_state": { + "reasons": [], + "state": "no_baseline" + }, + "check_report_version": 1, + "complete": true, + "counts": { + "departed_with_access": 1, + "incomplete": 0, + "missing_access": 2, + "policy_finding": 0, + "provider_failed": 0, + "unexplained_access": 2 + }, + "departed": [ + { + "access_paths": 1, + "account": "102", + "instance": "demo", + "login": "former-dev", + "reason": "inactive_identity" + } + ], + "departed_assessed": true, + "files": { + "report": "out/check.json", + "snapshot": "out/snapshot.json", + "summary": "out/summary.md" + }, + "findings": [], + "notifications": [ + { + "delivered": false, + "id": "admins", + "reason": null, + "skipped": true + }, + { + "delivered": false, + "id": "siem", + "reason": "", + "skipped": false + } + ], + "policy": { + "rules": [ + "inactive_access", + "external_privileged", + "machine_owner" + ] + }, + "profiles": { + "complete": true, + "counts": { + "ambiguous_accounts": 0, + "cannot_tell": 0, + "covered": 2, + "excepted": 0, + "missing": 2, + "not_covered": 2, + "unexplained": 2, + "unresolved_members": 0 + }, + "enforce": false, + "identities": [ + { + "accounts": [ + { + "account": "100", + "instance": "demo" + } + ], + "explained": 1, + "identity": "alice@example.com", + "missing": [], + "profiles": [ + "base" + ], + "state": "compared", + "unexplained": [ + { + "account": "100", + "certainty": "derived", + "grant_id": "grant-1", + "instance": "demo", + "resource": "repo-20", + "resource_name": "acme/payments-api", + "role": "Write", + "state": "finding", + "via": [ + "team-10" + ] + } + ] + }, + { + "accounts": [ + { + "account": "101", + "instance": "demo" + } + ], + "explained": 0, + "identity": "bob@example.com", + "missing": [ + { + "entry": { + "group": "team-10", + "instance": "demo", + "kind": "group" + }, + "reason": "not_observed", + "state": "finding" + }, + { + "entry": { + "instance": "demo", + "kind": "role", + "resource": "repo-21", + "role": "Read" + }, + "reason": "not_observed", + "state": "finding" + } + ], + "profiles": [ + "backend" + ], + "state": "compared", + "unexplained": [ + { + "account": "101", + "certainty": "observed", + "grant_id": "grant-3", + "instance": "demo", + "resource": "repo-21", + "resource_name": "acme/infrastructure", + "role": "Admin", + "state": "finding", + "via": [] + } + ] + } + ], + "message": "", + "profile_review_version": 1, + "unresolved_assignments": [] + }, + "providers": [ + { + "id": "demo", + "limitations": [], + "state": "complete" + } + ], + "run_url": null, + "status": "findings" +} diff --git a/crates/permesh-cli/tests/stable_formats.rs b/crates/permesh-cli/tests/stable_formats.rs index 9dcb202..5728e21 100644 --- a/crates/permesh-cli/tests/stable_formats.rs +++ b/crates/permesh-cli/tests/stable_formats.rs @@ -177,6 +177,38 @@ fn check_report_fixture() { assert_fixture("check-report.json", &normalized(dir.path(), report)); } +/// Notifications run (not `--no-notify`): one is skipped by its filter, one fails offline. +#[test] +fn check_report_with_notifications_fixture() { + let dir = workspace(); + let mut config = fs::read_to_string(dir.path().join("permesh.yaml")).unwrap(); + config.push_str( + "check:\n notifications:\n - id: admins\n type: slack_webhook\n url: env://PERMESH_FIXTURE_UNSET_SLACK\n on: [new_admin]\n - id: siem\n type: webhook\n url: env://PERMESH_FIXTURE_UNSET_HOOK\n", + ); + fs::write(dir.path().join("permesh.yaml"), config).unwrap(); + let output = Command::new(env!("CARGO_BIN_EXE_permesh")) + .current_dir(dir.path()) + .env("NO_COLOR", "1") + .env("PERMESH_DATA_DIR", dir.path().join("state")) + .env_remove("PERMESH_FIXTURE_UNSET_SLACK") + .env_remove("PERMESH_FIXTURE_UNSET_HOOK") + .args(["check", "--out", "out"]) + .output() + .unwrap(); + // A failed delivery never changes the exit code. + assert_eq!(output.status.code(), Some(6), "{output:?}"); + let mut report = normalized(dir.path(), read(dir.path(), "out/check.json")); + let notifications = report["notifications"].as_array_mut().unwrap(); + assert_eq!(notifications.len(), 2); + // A failure reason is a short human sentence (notifications.md); only its type is frozen. + for delivery in notifications { + if delivery["reason"].is_string() { + delivery["reason"] = json!(NORMALIZED); + } + } + assert_fixture("check-report-notifications.json", &report); +} + #[test] fn check_report_incomplete_fixture() { let dir = workspace(); @@ -227,10 +259,25 @@ fn profile_suggest_fixture() { // The draft is review text (human output, not frozen); it must stay a loadable profile. let draft = result["draft"].as_str().unwrap().to_owned(); write(dir.path(), "profiles/team.yaml", &draft); - assert_ne!( - run(dir.path(), &["profile", "check"]).status.code(), - Some(2) + write( + dir.path(), + "people/alice.yaml", + "identity: alice@example.com\nprofiles: [team]\n", ); + // The draft loads, and the team's own member is fully explained by it. + let output = run(dir.path(), &["profile", "check", "--json"]); + assert_ne!(output.status.code(), Some(2), "{output:?}"); + let check = json(&output); + let alice = check["result"]["identities"] + .as_array() + .unwrap() + .iter() + .find(|i| i["identity"] == "alice@example.com") + .unwrap() + .clone(); + assert_eq!(alice["profiles"], json!(["team"]), "{alice}"); + assert_eq!(alice["missing"], json!([]), "{alice}"); + assert_eq!(alice["unexplained"], json!([]), "{alice}"); result["draft"] = json!(NORMALIZED); assert_fixture("profile-suggest.json", &normalized(dir.path(), result)); } @@ -333,6 +380,42 @@ fn access_plan_leave_fixture() { ); } +#[test] +fn access_plan_move_fixture() { + let dir = workspace(); + snapshot(dir.path()); + let (code, report) = access( + dir.path(), + &[ + "move", + "bob@example.com", + "--remove", + "backend", + "--add", + "base", + "--snapshot", + "s.json", + "--output", + "p.json", + ], + ); + assert_eq!(code, Some(0), "{report}"); + let plan = read(dir.path(), "p.json"); + assert_eq!(plan["kind"], "move", "{plan}"); + assert_fixture("access-plan-move.json", &normalized(dir.path(), plan)); +} + +#[test] +fn fixture_comparison_detects_renamed_plan_field() { + let mut value: Value = + serde_json::from_slice(&fs::read(stable("access-plan-join.json")).unwrap()).unwrap(); + assert!(compare("access-plan-join.json", &value).is_ok()); + let step = value["steps"][0].as_object_mut().unwrap(); + let operation = step.remove("operation").unwrap(); + step.insert("op".into(), operation); + assert!(compare("access-plan-join.json", &value).is_err()); +} + fn joined_and_verified(partial: bool) -> (Option, Value, tempfile::TempDir) { let dir = workspace(); snapshot(dir.path()); diff --git a/docs/adr/0036-second-stability-declaration.md b/docs/adr/0036-second-stability-declaration.md index c5666a4..15d3ac3 100644 --- a/docs/adr/0036-second-stability-declaration.md +++ b/docs/adr/0036-second-stability-declaration.md @@ -28,8 +28,12 @@ guide and a release that still reads the previous version, as described in Another value is an input error that says to remove the line or upgrade. `index.json` becomes `{"access_review_index_version": 1, "plans": [...]}`. - **Golden fixtures** in `crates/permesh-cli/tests/fixtures/stable/` pin every - frozen format. Only times, durations, digests and temporary paths are - normalized, and their shapes are still checked. + frozen output: the check report (also with notifications), the webhook body, + `profile check` and `profile suggest`, the join, move and leave plans and the + execution plan, and the `access` results and review index. Only times, durations, + digests, temporary paths and human-readable text are normalized, and their shapes + are still checked. Frozen inputs (people and profile files, policy version 2 and + the new `permesh.yaml` sections) are pinned by parsing tests instead. ## Why From 695ee592667fedbc454fe797695430cdb768d00b Mon Sep 17 00:00:00 2001 From: Nicholas Petrasek Date: Fri, 23 Oct 2026 20:31:23 +0200 Subject: [PATCH 08/10] docs: add the small-team setup guide; record the fixes in the changelog --- CHANGELOG.md | 14 ++++ docs/README.md | 1 + docs/getting-started.md | 3 +- docs/mcp.md | 3 +- docs/team-setup.md | 158 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 177 insertions(+), 2 deletions(-) create mode 100644 docs/team-setup.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 16e09e8..ad29299 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,20 @@ User-visible changes are recorded here. Alpha releases are for evaluation; stabl ## Unreleased +- Add a [team setup guide](docs/team-setup.md) for a small team on GitHub and + Google Workspace. +- Fix: someone with `status: left` is expected to hold nothing in `profile check`, + even when a directory group still assigns them profiles. +- `access review` refuses existing plan files before discovery, escapes Markdown + in `--detail` summaries, and names the folder and the CI remedy when a reviewed + `index.json` has an old format. +- The starter verify workflow accepts only a successful post-merge plan run from + this repository's default branch, and exits like the CLI for the worst plan + (errors, then 3, 4, 6). +- `permesh mcp`: a failure to reach the client exits 5 instead of blaming the + protocol version; symlinks that leave the workspace read as "File not found", so + they reveal nothing about their target; empty `profiles` or `remove` lists get a + specific message. - Declare the formats added since the first stability declaration stable: the `check` and `profiles` configuration, people and profile files, policy `version: 2`, the check report, profile results, access plan files and the diff --git a/docs/README.md b/docs/README.md index d96db64..edc1b4a 100644 --- a/docs/README.md +++ b/docs/README.md @@ -9,6 +9,7 @@ connects them to a resource. Choose a starting point below. 2. [Try the demo and connect GitHub](getting-started.md): see useful results before granting API access. 3. [Understand identity matching](identity-resolution.md): connect a provider login to a canonical identity safely. 4. [Check provider coverage](providers.md): know what is available and what each adapter can prove. +5. [Set up a small team](team-setup.md): GitHub and Google Workspace, weekly checks and reviewed joins, moves and leaves. Documentation on `main` follows current source. [Release status and upgrade notes](status.md) describe what the published release includes; [stability](stability.md) diff --git a/docs/getting-started.md b/docs/getting-started.md index b5beee5..3b4ffbf 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -119,7 +119,8 @@ workspace explicitly. Queries do not rewrite configuration or change provider state. Adding providers rewrites YAML formatting, so review the Git diff. `init` never overwrites an existing workspace. Share configuration with your team and keep credentials local; -see [team workflows](team-workflows.md) for portable platform pins and approvals. +see [team workflows](team-workflows.md) for portable platform pins and approvals, +and [team setup](team-setup.md) for a complete walkthrough. For existing built-in configurations, use the explicit [GitHub](github-migration.md) or [Google](google-migration.md) migration. Do not create duplicate instances to diff --git a/docs/mcp.md b/docs/mcp.md index de2bba6..6be9064 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -37,7 +37,8 @@ The server uses them as they are; it never approves, stores credentials or promp **Client requirement.** The server speaks MCP protocol version 2026-07-28 only. A client that starts with the older `initialize` handshake gets error `-32022` listing `2026-07-28`, and `permesh mcp` exits with code 2. Update the client if -you see that error. +you see that error. If Permesh cannot write to the client at all, it exits with +code 5 instead. ## Tools diff --git a/docs/team-setup.md b/docs/team-setup.md new file mode 100644 index 0000000..d10d43f --- /dev/null +++ b/docs/team-setup.md @@ -0,0 +1,158 @@ +# Set up Permesh for a small team + +This guide sets up Permesh for a team of about ten people who use GitHub and +Google Workspace. At the end, a weekly CI job reports departed people who still +have access, new admins and access nobody's role explains, and every join, move or +leave goes through a reviewed pull request. + +Only the one or two administrators who look after access use Permesh. The other +people don't install anything: they appear as small files in a private Git +repository. Permesh only reads access. People carry out every change in GitHub or +Google, and Permesh checks the result. + +You need a Permesh release that includes `permesh check` and `permesh access` +(see [installation](installation.md)), admin access to the GitHub organization and +read access to the Google Workspace directory. + +## 1. Create the configuration repository + +Create a **private** repository: CI keeps full access reports as artifacts, and on +a public repository anyone signed in can download them. + +```bash +mkdir access-review && cd access-review && git init +permesh init --starter --organization "Example Ltd" +``` + +This writes `permesh.yaml`, the CI workflows, a review policy and empty `profiles/` +and `people/` folders ([starter layout](starter.md)). + +## 2. Connect GitHub and Google Workspace + +```bash +permesh provider add github --portable +permesh provider add google --portable --authoritative +permesh auth login github-main +permesh auth login google-main --browser +permesh doctor +``` + +- `--portable` pins the provider for every platform, so CI and your colleague's + machine run exactly the same binary. +- `--authoritative` makes the Google directory the source of truth for who is + employed: a suspended or archived Google account counts as departed. GitHub only + shows access; it can't say whether someone still works for you. +- Use read-only credentials: see [GitHub permissions](providers/github.md) and + [Google permissions](providers/google.md). The Google login runs a local browser + sign-in ([browser login](browser-auth.md)); an access token or refresh + credentials work too. Credentials stay in your keychain + locally and in CI secrets in CI, never in the repository. + +The Google provider reads directory users only, not Google groups or Drive +sharing. Expected access below is therefore described with GitHub teams and +repository roles. + +## 3. Link each person's GitHub account + +Each person's ID is their Google directory identity, written +`google:CUSTOMER_ID:USER_ID`. It stays the same when their email changes. GitHub +doesn't report verified emails to Permesh, so link each GitHub account once: + +```bash +permesh identity unresolved +``` + +lists GitHub accounts that belong to nobody yet, with the directory identities you +can link them to. For each one, propose the link, check it, then save it with the +fingerprint the proposal printed: + +```bash +permesh identity map github-main 987654321 --identity google:C01234567:123456789 +permesh identity map github-main 987654321 --identity google:C01234567:123456789 \ + --fingerprint REVIEWED_FINGERPRINT +``` + +`987654321` is the GitHub account's numeric ID, which survives login renames. For +ten people this takes a few minutes, once. Commit the change to `permesh.yaml`. +Details: [identity mapping](identity-mapping.md). + +To look up someone's identity ID by email: + +```bash +permesh user anna@example.com --json | jq -r .result.identity.id +``` + +## 4. Describe the roles + +A profile lists what one role should have. Most teams need two to four, for +example everyone, developers and admins. Let Permesh draft one from a team that +already has the right access: + +```bash +permesh user anna@example.com --json | jq -r '.result.access[].groups[]? | "\(.key.id) \(.name)"' | sort -u +permesh profile suggest --group github-main:4712 --name developer > profiles/developer.yaml +``` + +The first command shows the GitHub team IDs Anna is in; `4712` is one of them. +Review the draft before committing: it describes what the team has today, not what +it should have. A finished profile looks like this: + +```yaml +# profiles/developer.yaml +profile: developer +description: Developers +includes: [everyone] +grants: + - {instance: github-main, group: "4712"} # team developers + - {instance: github-main, role: Write, resource: "98765"} # repository api +``` + +## 5. Add one file per person + +```yaml +# people/anna.yaml +identity: google:C01234567:123456789 # anna@example.com +profiles: [developer] +``` + +That's ten files of two or three lines. The comment is for people reading the +file; Permesh uses only the identity ID. Run `permesh profile check` to see who is +missing expected access and which access no profile explains. Fix the files or +the access until the result is what you expect. + +Profile findings are report-only in the weekly check until you set +`profiles: {enforce: true}` in `permesh.yaml`. + +## 6. Turn on CI + +1. List the provider versions from step 2 in `ci/providers.txt`. +2. Add repository secrets for each `env://` reference in `permesh.yaml`, such as + `PERMESH_GITHUB_TOKEN` and `PERMESH_GOOGLE_TOKEN`. +3. In each workflow file, set `PERMESH_VERSION` and `PERMESH_ARCHIVE_SHA256` (the + `x86_64-unknown-linux-gnu` archive digest from the release page). +4. Run `permesh check --out out` once locally, then commit and push. + +Restrict who can push branches: pull-request jobs from branches in the same +repository receive the provider secrets. + +## Day to day + +| When | What you do | What Permesh does | +| --- | --- | --- | +| Every week | Read the issue if one opens | Checks everything and keeps one issue updated with counts only | +| Someone joins | Open a pull request adding `people/NAME.yaml` | Comments the plan: accounts to create, teams and roles to add | +| Someone moves | Change `profiles:` in their file | Plans only what to add and what no remaining profile needs | +| Someone leaves | Set `status: left` in their file | Plans the removal of everything they still have | +| After merging | Carry out the steps, then start the verify workflow with the post-merge run ID | Confirms each step is done, or says what is not | + +Until the person has a Google account, `people/NAME.yaml` can't name their +identity ID: add the file once their directory account exists. Plans, verification +and the weekly check are described in [access plans](access-plans.md) and +[scheduled checks](check.md). To be told in chat or by email, add notifications to +the `check` section ([notifications](notifications.md)). + +## A second administrator + +A colleague clones the repository, installs the same provider versions, trusts and +approves them on their machine, and logs in with their own credentials. Approvals +and credentials are never shared through Git. See [team workflows](team-workflows.md). From 37667bf5e75f585ba632caa53915bf30bc217522 Mon Sep 17 00:00:00 2001 From: Nicholas Petrasek Date: Fri, 23 Oct 2026 20:42:11 +0200 Subject: [PATCH 09/10] fix: verify accepts finished plan runs; starter passes Google refresh credentials; guide uses env references and defines every profile --- CHANGELOG.md | 7 +- crates/permesh-cli/tests/starter.rs | 26 +++++++- docs/starter.md | 5 +- docs/team-setup.md | 65 ++++++++++++++----- .../.github/workflows/permesh-check.yml | 3 + .../.github/workflows/permesh-plan.yml | 6 ++ .../.github/workflows/permesh-verify.yml | 12 ++-- examples/starter/.gitlab-ci.yml | 3 +- examples/starter/README.md | 5 +- 9 files changed, 103 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ad29299..2c15d6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,14 +5,15 @@ User-visible changes are recorded here. Alpha releases are for evaluation; stabl ## Unreleased - Add a [team setup guide](docs/team-setup.md) for a small team on GitHub and - Google Workspace. + Google Workspace. The starter workflows pass `PERMESH_GOOGLE_CLIENT_SECRET` and + `PERMESH_GOOGLE_REFRESH_TOKEN`, so Google can run in refresh-token mode in CI. - Fix: someone with `status: left` is expected to hold nothing in `profile check`, even when a directory group still assigns them profiles. - `access review` refuses existing plan files before discovery, escapes Markdown in `--detail` summaries, and names the folder and the CI remedy when a reviewed `index.json` has an old format. -- The starter verify workflow accepts only a successful post-merge plan run from - this repository's default branch, and exits like the CLI for the worst plan +- The starter verify workflow accepts only a finished post-merge plan run from + this repository's default branch (also one that exited 4 or 6), and exits like the CLI for the worst plan (errors, then 3, 4, 6). - `permesh mcp`: a failure to reach the client exits 5 instead of blaming the protocol version; symlinks that leave the workspace read as "File not found", so diff --git a/crates/permesh-cli/tests/starter.rs b/crates/permesh-cli/tests/starter.rs index 65425cd..4ee7d3a 100644 --- a/crates/permesh-cli/tests/starter.rs +++ b/crates/permesh-cli/tests/starter.rs @@ -399,11 +399,15 @@ fn verify_workflow_accepts_only_post_merge_plan_runs() { "path": ".github/workflows/permesh-plan.yml", "event": "push", "head_branch": "main", + "status": "completed", "conclusion": "success", "repository": {"id": 7}, "head_repository": {"id": 7} }); - let mut cases = vec![(good.clone(), true)]; + // A run that exits 4 or 6 still keeps the plans that matched their review. + let mut failed = good.clone(); + failed["conclusion"] = serde_json::json!("failure"); + let mut cases = vec![(good.clone(), true), (failed, true)]; for (field, value) in [ ( "path", @@ -411,7 +415,7 @@ fn verify_workflow_accepts_only_post_merge_plan_runs() { ), ("event", serde_json::json!("pull_request")), ("head_branch", serde_json::json!("feature")), - ("conclusion", serde_json::json!("failure")), + ("status", serde_json::json!("in_progress")), ("head_repository", serde_json::json!({"id": 8})), ] { let mut run = good.clone(); @@ -432,3 +436,21 @@ fn verify_workflow_accepts_only_post_merge_plan_runs() { assert_eq!(dir.path().join("downloaded").exists(), accepted, "{run}"); } } + +#[test] +fn workflows_pass_google_refresh_credentials() { + // A Google access token expires within the hour; weekly CI needs refresh mode. + for file in [ + ".github/workflows/permesh-check.yml", + ".github/workflows/permesh-plan.yml", + ".github/workflows/permesh-verify.yml", + ] { + let text = String::from_utf8(example(file)).unwrap(); + for name in ["PERMESH_GOOGLE_REFRESH_TOKEN", "PERMESH_GOOGLE_CLIENT_SECRET"] { + assert!( + text.contains(&format!("{name}: ${{{{ secrets.{name} }}}}")), + "{file}: {name}" + ); + } + } +} diff --git a/docs/starter.md b/docs/starter.md index bc371b2..97a6f63 100644 --- a/docs/starter.md +++ b/docs/starter.md @@ -60,8 +60,9 @@ before carrying out any step. A push that changes people files without a pull request fails, because nothing was reviewed. When the steps are done, start `permesh-verify.yml` from the Actions tab with the -post-merge run ID. It only accepts a successful post-merge `permesh-plan.yml` run on -the default branch of this repository. It verifies each execution plan, writes +post-merge run ID. It only accepts a finished post-merge `permesh-plan.yml` run on +the default branch of this repository; a run that exited 4 or 6 still counts, +because it keeps the plans that matched their review. It verifies each execution plan, writes counts to the step summary and, after all plans ran, exits like the CLI would for the worst one: an error first, then 3, 4 and 6. diff --git a/docs/team-setup.md b/docs/team-setup.md index d10d43f..4909632 100644 --- a/docs/team-setup.md +++ b/docs/team-setup.md @@ -27,32 +27,58 @@ permesh init --starter --organization "Example Ltd" This writes `permesh.yaml`, the CI workflows, a review policy and empty `profiles/` and `people/` folders ([starter layout](starter.md)). -## 2. Connect GitHub and Google Workspace +## 2. Get read-only credentials + +CI runs unattended every week, so use credentials that last: + +- **GitHub:** a token with read access to the organization + ([GitHub permissions](providers/github.md)). +- **Google Workspace:** an OAuth client and a refresh token for an administrator + allowed to read users, with the read-only directory scope + ([Google permissions](providers/google.md) and the provider's + [setup guide](https://github.com/NIPE-Solutions/permesh-providers/blob/main/docs/google.md)). + An access token expires within an hour, which is too short for CI. Make the OAuth + app internal to your organization: Google expires refresh tokens of apps in + testing after seven days. + +`permesh.yaml` names each credential by reference, never by value, and the same +file serves your machine and CI. Use environment variables, which work in both +places: + +| Secret | Environment variable | +| --- | --- | +| GitHub token | `PERMESH_GITHUB_TOKEN` | +| Google OAuth client secret | `PERMESH_GOOGLE_CLIENT_SECRET` | +| Google refresh token | `PERMESH_GOOGLE_REFRESH_TOKEN` | + +Set them in your shell (or your password manager's shell integration) before +running Permesh. Never put the values in the repository. + +## 3. Connect GitHub and Google Workspace ```bash permesh provider add github --portable permesh provider add google --portable --authoritative -permesh auth login github-main -permesh auth login google-main --browser permesh doctor ``` +When `provider add` asks for a credential reference, answer with the variables +above: `env://PERMESH_GITHUB_TOKEN` for GitHub; for Google, choose refresh-token +mode, give the OAuth client ID (it isn't secret), then +`env://PERMESH_GOOGLE_CLIENT_SECRET` and `env://PERMESH_GOOGLE_REFRESH_TOKEN`. +`permesh doctor` then checks that both providers answer. + - `--portable` pins the provider for every platform, so CI and your colleague's machine run exactly the same binary. - `--authoritative` makes the Google directory the source of truth for who is employed: a suspended or archived Google account counts as departed. GitHub only shows access; it can't say whether someone still works for you. -- Use read-only credentials: see [GitHub permissions](providers/github.md) and - [Google permissions](providers/google.md). The Google login runs a local browser - sign-in ([browser login](browser-auth.md)); an access token or refresh - credentials work too. Credentials stay in your keychain - locally and in CI secrets in CI, never in the repository. The Google provider reads directory users only, not Google groups or Drive sharing. Expected access below is therefore described with GitHub teams and repository roles. -## 3. Link each person's GitHub account +## 4. Link each person's GitHub account Each person's ID is their Google directory identity, written `google:CUSTOMER_ID:USER_ID`. It stays the same when their email changes. GitHub @@ -82,7 +108,7 @@ To look up someone's identity ID by email: permesh user anna@example.com --json | jq -r .result.identity.id ``` -## 4. Describe the roles +## 5. Describe the roles A profile lists what one role should have. Most teams need two to four, for example everyone, developers and admins. Let Permesh draft one from a team that @@ -95,7 +121,14 @@ permesh profile suggest --group github-main:4712 --name developer > profiles/dev The first command shows the GitHub team IDs Anna is in; `4712` is one of them. Review the draft before committing: it describes what the team has today, not what -it should have. A finished profile looks like this: +it should have. Finished profiles look like this: + +```yaml +# profiles/everyone.yaml +profile: everyone +grants: + - {instance: github-main, group: "4711"} # team everyone +``` ```yaml # profiles/developer.yaml @@ -107,7 +140,7 @@ grants: - {instance: github-main, role: Write, resource: "98765"} # repository api ``` -## 5. Add one file per person +## 6. Add one file per person ```yaml # people/anna.yaml @@ -123,11 +156,11 @@ the access until the result is what you expect. Profile findings are report-only in the weekly check until you set `profiles: {enforce: true}` in `permesh.yaml`. -## 6. Turn on CI +## 7. Turn on CI -1. List the provider versions from step 2 in `ci/providers.txt`. -2. Add repository secrets for each `env://` reference in `permesh.yaml`, such as - `PERMESH_GITHUB_TOKEN` and `PERMESH_GOOGLE_TOKEN`. +1. List the provider versions from step 3 in `ci/providers.txt`. +2. Add the three variables from step 2 as repository secrets with the same names. + The starter workflows already pass them to Permesh. 3. In each workflow file, set `PERMESH_VERSION` and `PERMESH_ARCHIVE_SHA256` (the `x86_64-unknown-linux-gnu` archive digest from the release page). 4. Run `permesh check --out out` once locally, then commit and push. diff --git a/examples/starter/.github/workflows/permesh-check.yml b/examples/starter/.github/workflows/permesh-check.yml index bcd972d..6aac2c0 100644 --- a/examples/starter/.github/workflows/permesh-check.yml +++ b/examples/starter/.github/workflows/permesh-check.yml @@ -68,6 +68,9 @@ jobs: # Map each credential reference in permesh.yaml (env://NAME) to a repository secret. PERMESH_GITHUB_TOKEN: ${{ secrets.PERMESH_GITHUB_TOKEN }} PERMESH_GOOGLE_TOKEN: ${{ secrets.PERMESH_GOOGLE_TOKEN }} + # Google refresh mode (weekly CI): client secret and refresh token. + PERMESH_GOOGLE_CLIENT_SECRET: ${{ secrets.PERMESH_GOOGLE_CLIENT_SECRET }} + PERMESH_GOOGLE_REFRESH_TOKEN: ${{ secrets.PERMESH_GOOGLE_REFRESH_TOKEN }} PERMESH_SLACK_WEBHOOK: ${{ secrets.PERMESH_SLACK_WEBHOOK }} run: | set +e diff --git a/examples/starter/.github/workflows/permesh-plan.yml b/examples/starter/.github/workflows/permesh-plan.yml index 01cee8b..be6005e 100644 --- a/examples/starter/.github/workflows/permesh-plan.yml +++ b/examples/starter/.github/workflows/permesh-plan.yml @@ -59,6 +59,9 @@ jobs: # Map each credential reference in permesh.yaml (env://NAME) to a repository secret. PERMESH_GITHUB_TOKEN: ${{ secrets.PERMESH_GITHUB_TOKEN }} PERMESH_GOOGLE_TOKEN: ${{ secrets.PERMESH_GOOGLE_TOKEN }} + # Google refresh mode (weekly CI): client secret and refresh token. + PERMESH_GOOGLE_CLIENT_SECRET: ${{ secrets.PERMESH_GOOGLE_CLIENT_SECRET }} + PERMESH_GOOGLE_REFRESH_TOKEN: ${{ secrets.PERMESH_GOOGLE_REFRESH_TOKEN }} run: | git worktree add ../base "$BASE_SHA" set +e @@ -140,6 +143,9 @@ jobs: BEFORE: ${{ github.event.before }} PERMESH_GITHUB_TOKEN: ${{ secrets.PERMESH_GITHUB_TOKEN }} PERMESH_GOOGLE_TOKEN: ${{ secrets.PERMESH_GOOGLE_TOKEN }} + # Google refresh mode (weekly CI): client secret and refresh token. + PERMESH_GOOGLE_CLIENT_SECRET: ${{ secrets.PERMESH_GOOGLE_CLIENT_SECRET }} + PERMESH_GOOGLE_REFRESH_TOKEN: ${{ secrets.PERMESH_GOOGLE_REFRESH_TOKEN }} run: | git worktree add ../base "$BEFORE" set +e diff --git a/examples/starter/.github/workflows/permesh-verify.yml b/examples/starter/.github/workflows/permesh-verify.yml index b3f357f..74b4882 100644 --- a/examples/starter/.github/workflows/permesh-verify.yml +++ b/examples/starter/.github/workflows/permesh-verify.yml @@ -50,14 +50,15 @@ jobs: case "$RUN_ID" in ''|*[!0-9]*) echo "run_id must be a numeric workflow run ID." >&2; exit 2 ;; esac - # Accept only a successful post-merge run of permesh-plan.yml on the default - # branch of this repository, so no branch or fork can supply the plans. + # Accept only a finished post-merge run of permesh-plan.yml on the default + # branch of this repository, so no branch or fork can supply the plans. A run + # that exited 4 or 6 still holds the plans that matched their review. default_branch=$(gh api "repos/${GITHUB_REPOSITORY}" | jq -r .default_branch) gh api "repos/${GITHUB_REPOSITORY}/actions/runs/${RUN_ID}" > run.json jq -e --arg branch "$default_branch" '.path == ".github/workflows/permesh-plan.yml" - and .event == "push" and .head_branch == $branch and .conclusion == "success" + and .event == "push" and .head_branch == $branch and .status == "completed" and .head_repository.id == .repository.id' run.json > /dev/null || { - echo "Run $RUN_ID is not a successful post-merge Permesh plan run on $default_branch." >&2 + echo "Run $RUN_ID is not a finished post-merge Permesh plan run on $default_branch." >&2 exit 2 } gh run download "$RUN_ID" -n execution-plans -D execution @@ -67,6 +68,9 @@ jobs: RUN_ID: ${{ inputs.run_id }} PERMESH_GITHUB_TOKEN: ${{ secrets.PERMESH_GITHUB_TOKEN }} PERMESH_GOOGLE_TOKEN: ${{ secrets.PERMESH_GOOGLE_TOKEN }} + # Google refresh mode (weekly CI): client secret and refresh token. + PERMESH_GOOGLE_CLIENT_SECRET: ${{ secrets.PERMESH_GOOGLE_CLIENT_SECRET }} + PERMESH_GOOGLE_REFRESH_TOKEN: ${{ secrets.PERMESH_GOOGLE_REFRESH_TOKEN }} run: | # The job exit follows the CLI's precedence: errors, then 3, 4 and 6. rank() { case "$1" in 0) echo 0 ;; 6) echo 1 ;; 4) echo 2 ;; 3) echo 3 ;; *) echo 4 ;; esac; } diff --git a/examples/starter/.gitlab-ci.yml b/examples/starter/.gitlab-ci.yml index b1f0637..a6d3711 100644 --- a/examples/starter/.gitlab-ci.yml +++ b/examples/starter/.gitlab-ci.yml @@ -3,7 +3,8 @@ # Merge requests that touch people/ or profiles/ get an access plan (permesh-plan); # the post-merge execution and verify steps run by hand (docs/access-plans.md). # -# CI/CD variables (masked): PERMESH_GITHUB_TOKEN, PERMESH_GOOGLE_TOKEN, ... for the +# CI/CD variables (masked): PERMESH_GITHUB_TOKEN, PERMESH_GOOGLE_CLIENT_SECRET, +# PERMESH_GOOGLE_REFRESH_TOKEN, ... for the # env:// references in permesh.yaml; optional PERMESH_ISSUE_TOKEN (project access # token with api scope) to keep a review issue updated. Don't mark the provider # variables Protected: merge request pipelines on unprotected branches would not get diff --git a/examples/starter/README.md b/examples/starter/README.md index 2aa86e7..94cd325 100644 --- a/examples/starter/README.md +++ b/examples/starter/README.md @@ -14,11 +14,14 @@ public repositories anyone signed in can download them. ```bash permesh provider add github --portable permesh provider add google --portable --authoritative - permesh auth login github-main permesh doctor ``` `--portable` pins the provider binary for every platform, so CI can verify it. + When asked for credential references, use environment variables, which work + both locally and in CI: `env://PERMESH_GITHUB_TOKEN`, and for Google refresh-token + mode `env://PERMESH_GOOGLE_CLIENT_SECRET` and `env://PERMESH_GOOGLE_REFRESH_TOKEN`. + A keychain reference works only on your machine. 2. **List the provider versions** you just added in `ci/providers.txt`. 3. **Add CI secrets** for each `env://NAME` reference in `permesh.yaml` From 9003e46553436a5fa1a8fc8e651f2e52edec0b14 Mon Sep 17 00:00:00 2001 From: Nicholas Petrasek Date: Fri, 23 Oct 2026 20:42:16 +0200 Subject: [PATCH 10/10] style: cargo fmt --- crates/permesh-cli/tests/starter.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/permesh-cli/tests/starter.rs b/crates/permesh-cli/tests/starter.rs index 4ee7d3a..9e1d4f2 100644 --- a/crates/permesh-cli/tests/starter.rs +++ b/crates/permesh-cli/tests/starter.rs @@ -446,7 +446,10 @@ fn workflows_pass_google_refresh_credentials() { ".github/workflows/permesh-verify.yml", ] { let text = String::from_utf8(example(file)).unwrap(); - for name in ["PERMESH_GOOGLE_REFRESH_TOKEN", "PERMESH_GOOGLE_CLIENT_SECRET"] { + for name in [ + "PERMESH_GOOGLE_REFRESH_TOKEN", + "PERMESH_GOOGLE_CLIENT_SECRET", + ] { assert!( text.contains(&format!("{name}: ${{{{ secrets.{name} }}}}")), "{file}: {name}"