[REVIEW] rbac-design: add ABAC attribute freshness and PDP failure-mode gates

[abulasa1]

Skill Being Reviewed

Skill name: rbac-design
Skill path: skills/identity/rbac-design/

False Positive Analysis

Benign code/config that triggers a false positive:

package authz

default allow := false

# Department is not read from a user-controlled JWT claim at decision time.
# The API gateway passes only subject_id; OPA resolves department from an
# authoritative HR directory bundle that is refreshed every 5 minutes and signed.
allow if {
  input.action == read
  input.resource.type == finance_report
  subject := data.hr.subjects[input.subject_id]
  subject.department == input.resource.owning_department
  data.bundle_metadata.hr.subjects.signature_verified == true
  time.now_ns() - data.bundle_metadata.hr.subjects.loaded_at_ns < 300000000000
}

Why this is a false positive:
The current skill warns on stale or inconsistent PIP attributes, which is correct, but it can over-report any cached attribute source as stale. A short-lived, signed PDP-local bundle can be safer than making every authorization decision depend on a live HR or directory lookup, because it avoids fail-open behavior caused by upstream outages. The review should distinguish unsafe stale attributes from bounded, signed, monitored attribute caches with explicit maximum age and fail-closed behavior.

Coverage Gaps

Missed variant 1: user-controlled token claims are treated as authoritative ABAC attributes

app.get('/reports/:id', async (req, res) => {
  const report = await reports.get(req.params.id);

  // department and clearance come directly from the access token payload.
  // The service does not rehydrate them from an authoritative PIP or check
  // whether the user's department changed after token issuance.
  if (req.user.department === report.department &&
      req.user.clearance >= report.classification) {
    return res.json(report);
  }

  res.status(403).end();
});

Why it should be caught:
This is an ABAC authorization failure even if the policy expression looks correct. Long-lived access tokens, stale session claims, or claims minted by a less-trusted IdP can preserve access after transfer, termination, risk-state changes, or clearance revocation. The skill mentions PIP authority in one checklist line, but it does not require reviewers to record attribute provenance, token max age, rehydration strategy, or revocation freshness for ABAC-critical attributes.

Missed variant 2: PDP/PIP outage falls back to permit or cached allow decision without a freshness bound

def can_export_customer_data(user_id, customer_id):
    try:
        attrs = pip.fetch_subject_attrs(user_id, timeout_ms=250)
        return policy.evaluate(attrs, {customer_id: customer_id, action: export})
    except TimeoutError:
        # Designed for availability, but unsafe for sensitive data export.
        return last_known_decision_cache.get((user_id, customer_id, export), True)

Why it should be caught:
Hybrid RBAC/ABAC systems often fail in the plumbing rather than the policy syntax. If the PIP, PDP, or policy bundle service is unavailable, a fail-open default or unbounded cached allow decision can bypass SoD, tenant isolation, clearance, device posture, or risk-adaptive restrictions. The current skill requires deny-by-default for ABAC policies, but it does not explicitly test PDP/PIP failure mode, cache TTL, last-known-good policy behavior, or stale decision invalidation.

Edge Cases

• Some attributes are safe to cache longer than others. Department and manager hierarchy may tolerate minutes, while risk score, device compliance, break-glass activation, employment status, legal hold, or tenant membership often need much shorter freshness bounds.
• Offline/edge deployments may intentionally use signed policy and attribute bundles. That is acceptable only when bundle age, source, signature, and fail-closed behavior are logged and enforced.
• OAuth scopes can be stable permission boundaries, but ABAC-critical claims such as department, tenant_id, clearance, or device_compliant need provenance and freshness evidence before they are trusted.
• Attribute-conflict resolution matters when HR, IdP, device posture, and app-local ownership sources disagree. The skill should require source precedence rules instead of assuming all PIPs agree.

Remediation Quality

• Fix resolves the vulnerability
• Fix doesn't introduce new security issues
• Fix doesn't break functionality
• Issues found: Add an attribute provenance and decision freshness gate for ABAC/RBAC hybrid systems.

Suggested gates:

RBAC-ATTR-01: For every ABAC-critical attribute, record authoritative source, trust tier, refresh cadence, maximum acceptable age, and whether it is user-controlled, token-derived, or server-side rehydrated.
RBAC-ATTR-02: Require token/session claim freshness evidence for department, tenant, clearance, employment status, device posture, risk score, and break-glass activation attributes.
RBAC-ATTR-03: Require explicit PDP/PIP failure-mode evidence: fail-closed for sensitive actions, bounded last-known-good cache only where approved, and no implicit permit on timeout.
RBAC-ATTR-04: Log policy version, attribute source/version, attribute age, and decision reason for each authorization decision that depends on ABAC attributes.
RBAC-ATTR-05: Define precedence and conflict handling when multiple PIPs provide the same attribute or when app-local ownership conflicts with directory/HR data.

Comparison to Other Tools

Tool	Catches this?	Notes
Semgrep	Partial	Can flag direct trust in JWT claims or fail-open timeout handlers, but it cannot prove authoritative attribute lineage without review evidence.
CodeQL	Partial	Can trace token claims into authorization decisions in supported languages, but policy freshness and PIP outage behavior are architectural.
OPA/Rego tests	Partial	Can test policy decisions and deny-by-default behavior, but separate tests are needed for bundle freshness, PIP source precedence, and outage behavior.
Cedar policy validation	Partial	Can validate policy shape and entity schemas, but token/session claim freshness and entity-store synchronization are deployment concerns.

Overall Assessment

Strengths:
The skill gives a strong RBAC/ABAC design workflow: role hierarchy, SoD constraints, permission boundaries, ABAC policy structure, role mining, and NIST-grounded output. It correctly calls out stale PIP data as a risk.

Needs improvement:
The ABAC section should move from a single PIP-authority checklist item to a concrete evidence model. In real systems, authorization failures often come from stale JWT/session claims, directory synchronization lag, cached allow decisions, fail-open PDP/PIP outages, or conflicting attribute sources. The policy can look correct while the attribute pipeline is untrustworthy.

Priority recommendations:

1. Add an Attribute Provenance and Freshness subsection to Step 5 with an evidence table for each ABAC-critical attribute.
2. Add PDP/PIP outage and cache TTL checks, including fail-closed guidance for sensitive actions.
3. Extend the output format with policy version, attribute source/version, attribute age, and decision rationale fields for auditability.

Bounty Info

• I have read and agree to the CONTRIBUTING.md bounty terms
• Preferred payment method: GitHub Sponsors; payment details can be provided privately after maintainer acceptance.

[Errordog2]

/claim #2550

Implemented in #2588.

Summary:

• adds ABAC attribute provenance and freshness gates for authoritative source, trust tier, owner, max age, token-derived status, and server-side rehydration
• adds PDP/PIP failure-mode checks for fail-closed sensitive actions, bounded last-known-good caches, cache invalidation, and attribute conflict precedence
• extends the output format with attribute evidence, PDP/PIP resilience, policy version, attribute age/source, and decision auditability fields

Validation:

• git diff --check
• Markdown fence balance check
• targeted marker check for RBAC-ATTR gates, PDP/PIP failure modes, attribute evidence matrix, decision auditability, and v1.0.1

Payment details can be provided privately after maintainer acceptance.