Skip to content

PLAT-711: Require a non-empty permissions table in installation-policy - #65

Merged
nresare merged 1 commit into
mainfrom
require-installation-policy-permissions
Aug 18, 2026
Merged

nresare merged 1 commit into
mainfrom
require-installation-policy-permissions

Conversation

@nresare

@nresare nresare commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

What

permissions is now a required, non-empty table on every [[installation-policy]].

  • Removed #[serde(default)] from permissions on RawInstallationPolicyConfig, so a policy without the table fails to parse with missing field `permissions` .
  • Added a validate() check rejecting an explicitly empty table: installation-policy for github-app '…' repository '…' role '…' must define at least one permission.

Why

The table down-scopes what a minted token can do. With default, omitting it meant the token carried the App's full installation permissions — so the least-privilege path was opt-in, and the broad one was what you got by forgetting.

I made it non-empty rather than merely present: a required-but-empty table would still mean "full installation permissions", preserving exactly the silent behaviour this change is meant to remove.

Breaking change

This is a breaking config change. Any existing [[installation-policy]] without a permissions table will fail to start with a parse error naming the missing field, and will need a table added before deploying this version.

Notes for reviewers

  • rejects_installation_policy_without_permissions and rejects_installation_policy_with_empty_permissions replace installation_policy_permissions_default_empty_when_absent; every other config fixture gained a permissions entry.
  • Unrecognised permission names/values are unchanged — still a startup warning, still forwarded to GitHub.
  • Left alone deliberately: the authorize_github_app_* tests in src/service.rs build InstallationPolicyConfig structs directly with permissions: BTreeMap::new(). That state is no longer reachable from config, but the tests exercise the authorization path and pass as-is. Happy to update them if you'd rather they reflect the new invariant.
  • cargo test (63 passed), cargo clippy --all-targets and cargo fmt --check are clean.

Every [[installation-policy]] must now declare a permissions table naming
at least one GitHub permission. Previously the table defaulted to empty,
which meant the minted token carried the App's full installation
permissions — the opposite of least privilege, and easy to get by
omission rather than by choice.

A missing table is now a deserialization error; an explicitly empty one
is rejected by validate(), since allowing it would preserve exactly the
silent full-permissions behaviour this change removes.

Config fixtures, idcat.toml.example and the README are updated to match.
@nresare
nresare force-pushed the require-installation-policy-permissions branch from 43b1ce5 to 11da3d7 Compare August 18, 2026 15:11
@nresare nresare changed the title Require a non-empty permissions table in installation-policy PLAT-711: Require a non-empty permissions table in installation-policy Aug 18, 2026
@nresare
nresare merged commit 36019c1 into main Aug 18, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant