Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

many: support mixed outcomes for permissions in prompting constraints #14581

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Commits on Nov 14, 2024

  1. many: support mixed outcomes for permissions in prompting constraints

    Let rule content constraints have heterogeneous outcomes and lifespans
    for different permissions in the constraints. As such, convert the list
    of permissions to a map from permission to permission entry, where the
    entry holds the outcome, lifespan, and duration/expiration for that
    particular permission, where previous those were global to the
    containing rule, rule contents, or patch contents.
    
    However, the existing concept of replying "allow"/"deny" to a particular
    set of requested permisisons is clear and simple. We want to keep
    outcome, lifespan, and duration as reply-scoped values, not
    permission-specific, when accepting prompt replies. So we need different
    types of constraints for prompt replies vs. rule contents.
    
    The motivation behind this is so that we can have only a single rule for
    any given path pattern. We may have a situation where the user
    previously replied with "allow read `/path/to/foo`" and they're now
    prompted for write access, they need to be able to respond with "deny
    read `/path/to/foo`". If we only support a single outcome for any given
    rule, then we'd need two rules for the same path `/path/to/foo`. Thus,
    we need rules to support different outcomes for different permissions.
    
    The same logic applies for lifetimes and expirations, though this adds
    additional complexity now that the concept of rule expiration is shifted
    to being permission-specific. We care about expired rules in two primary
    places: when loading rules from disk, we want to discard any expired
    rules, and when adding a new rule, we want to discard any expired
    permisison entry for a rule which shares a pattern variant with the new
    rule. For cases where that expired permission entry had a conflicting
    outcome, we clearly can't have that, and we want to remove the expired
    permission entry from its containing rule as well, so as to avoid
    confusion for the user without them needing to check expiration
    timestamps. Even if the outcome of the expired entry matches that of the
    new rule's entry for the same permission, we still want to prune the
    expired permission from the old rule to avoid confusion. The complexity
    is around when a notice is recorded for a rule for which some
    permissions have expired. At the moment, the logic is that a notice is
    recorded in these cases:
    
    - when a rule is loaded from disk
        - data may be `"removed": "expired"` if all permissions are expired
    - when a rule is added
    - when a rule is patched
    - when a rule is removed (with data `"removed": "removed"`)
    - when a rule is found to be expired when attempting to add a new rule
    
    Notably, a notice is not recorded automatically when a permission entry
    expires. Nor is a notice recorded when a permission is found to be
    expired, so long as its associated rule still has at least one
    non-expired permission. Neither pruning an expired permission entry from
    the rule tree nor from the entry's containing rule results in a notice,
    even though technically the rule data has changed, since the expired
    permission has been erased. The rationale is that the semantics of the
    rule have not changed, since the expiration of that permission was part
    of the semantics of the rule.
    
    Since durations are used when adding/patching a rule and expirations are
    used when retrieving a rule, in addition to the differences for prompt
    replies vs. rule contents, we now need several different variants of
    constraints:
    - `promptConstraints`:
        - path, requested permissions list, available permissions list
        - internal to `requestprompts`, unchanged
    - `ReplyConstraints`:
        - path pattern, list of permissions
        - containing `PromptReply` holds outcome/lifespan/expiration
        - unchanged from before, though under a new name
        - converted to a `Constraints` if reply warrants a new rule
    - `Constraints`:
        - path pattern, map from permission to outcome, lifespan, duration
        - used when adding rule to the rule DB
        - converted to `RuleConstraints` when the new rule is created
    - `RuleConstraints`:
        - path pattern, map from permisison to outcome, lifespan, expiration
        - used when retrieving rules from the rule DB
        - never used when POSTing to the API
    - `PatchConstraints`:
        - identical to `Constraints`, but with omitempty fields
        - converted to `RuleConstraints` when the patched rule is created
    
    To support this, we define some new types, including `{,Rule}PermissionMap`
    and `{,Rule}PermissionEntry`. The latter of these is used in the leaves
    of the rule DB tree in place of the previous set of rule IDs of rules
    whose patterns render to a given pattern variant.
    
    Whenever possible, logic surrounding constraints, permissions, and
    expiration is pushed down to methods on these new types, thus
    simplifiying the logic of their callers.
    
    Signed-off-by: Oliver Calder <[email protected]>
    olivercalder committed Nov 14, 2024
    Configuration menu
    Copy the full SHA
    364e903 View commit details
    Browse the repository at this point in the history
  2. i/prompting: expire entry on (not after) timestamp and improve unit t…

    …ests
    
    Signed-off-by: Oliver Calder <[email protected]>
    olivercalder committed Nov 14, 2024
    Configuration menu
    Copy the full SHA
    aedf157 View commit details
    Browse the repository at this point in the history
  3. i/prompting: unexport PermissionMap.ToRulePermissionMap

    Signed-off-by: Oliver Calder <[email protected]>
    olivercalder committed Nov 14, 2024
    Configuration menu
    Copy the full SHA
    b9b63d8 View commit details
    Browse the repository at this point in the history
  4. i/p/errors: added tests for errors.Join

    Signed-off-by: Oliver Calder <[email protected]>
    olivercalder committed Nov 14, 2024
    Configuration menu
    Copy the full SHA
    b450fcd View commit details
    Browse the repository at this point in the history
  5. i/p/requestprompts: adjust comments and naming to clarify behavior ar…

    …ound handling new rules
    
    Signed-off-by: Oliver Calder <[email protected]>
    olivercalder committed Nov 14, 2024
    Configuration menu
    Copy the full SHA
    c62e9e4 View commit details
    Browse the repository at this point in the history
  6. i/p/requestprompts: expand and simplify tests of rule with mixed outc…

    …omes
    
    Signed-off-by: Oliver Calder <[email protected]>
    olivercalder committed Nov 14, 2024
    Configuration menu
    Copy the full SHA
    46b5c71 View commit details
    Browse the repository at this point in the history