Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pkg/workflow/awf_config_conformance_registry_formal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,11 @@ func formalConformanceRegistrySeriesDisjoint(id string) bool {

func TestFormalConformanceRegistry_P1_TestIDMonotonicity(t *testing.T) {
next := formalConformanceRegistryNextPlainID(formalConformanceRegistryBaselineRows(t))
assert.Equal(t, "T-DR-011", next)
assert.Equal(t, "T-DR-012", next)

nextValue, ok := formalConformanceRegistryParsePlainID(next)
require.True(t, ok)
assert.Equal(t, 11, nextValue)
assert.Equal(t, 12, nextValue)
}

func TestFormalConformanceRegistry_P1_EmptyRegistryStartsAtOne(t *testing.T) {
Expand Down Expand Up @@ -228,7 +228,7 @@ func TestFormalConformanceRegistry_P8_SpecCrossReferenceRequired(t *testing.T) {
assert.True(t, formalConformanceRegistryHasSpecCrossReference(specContent, row.TestID), row.TestID)
}

assert.False(t, formalConformanceRegistryHasSpecCrossReference(specContent, "T-DR-011"))
assert.False(t, formalConformanceRegistryHasSpecCrossReference(specContent, "T-DR-012"))
}

func TestFormalConformanceRegistry_P9_DriftSeriesVsSafeguardSeriesDisjoint(t *testing.T) {
Expand Down
6 changes: 3 additions & 3 deletions pkg/workflow/awf_config_safeguards_formal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,18 +105,18 @@ func TestAWFConfigSafeguard_TDRSAFE001_SnapshotStoragePathSelection(t *testing.T
assert.NotEqual(t, filepath.Clean(formalSnapshotStoragePath(false)), filepath.Clean(formalSnapshotStoragePath(true)))
}

func TestFormalP13_EscalationOwnerAssignmentFallbackChain(t *testing.T) {
func TestAWFConfigSafeguard_TDR011_EscalationOwnerAssignmentFallbackChain(t *testing.T) {
assert.Equal(t, "@last-maintainer", formalEscalationOwner("@last-maintainer", "@on-call"))
assert.Equal(t, "@on-call", formalEscalationOwner("", "@on-call"))
}

func TestFormalP14_EscalationOwnerMustNotBeUnassigned(t *testing.T) {
func TestAWFConfigSafeguard_TDR011_EscalationOwnerMustNotBeUnassigned(t *testing.T) {
assert.True(t, formalEscalationOwnerNonEmpty(formalEscalationOwner("@last-maintainer", "@on-call")))
assert.True(t, formalEscalationOwnerNonEmpty(formalEscalationOwner("", "@on-call")))
assert.False(t, formalEscalationOwnerNonEmpty(formalEscalationOwner("", "")))
}

func TestFormalP15_EscalationAcknowledgementWindow(t *testing.T) {
func TestAWFConfigSafeguard_TDR011_EscalationAcknowledgementWindow(t *testing.T) {
assignedFriday := time.Date(2026, 8, 7, 9, 0, 0, 0, time.UTC)
mondayDeadline := time.Date(2026, 8, 10, 9, 0, 0, 0, time.UTC)

Expand Down
6 changes: 4 additions & 2 deletions scratchpad/github-mcp-access-control-specification.md
Original file line number Diff line number Diff line change
Expand Up @@ -1026,11 +1026,11 @@ approval-labels: [true] # All elements must be strings
**Validation**:
```yaml
# INVALID
repos: [] # Empty array blocks all access
repos: [] # Empty allowlist would block all access; omit `repos` for no repository restriction
roles: [] # Empty array blocks all access

# VALID
# (omit field entirely if you want no restrictions)
# Omitted `repos` means no repository restriction; an empty array is rejected to prevent accidental deny-all.
repos:
- "*/*" # Explicit all-access
```
Expand Down Expand Up @@ -1852,6 +1852,7 @@ This subsection specifies normative failure-mode behavior for the case where gua
- Implementations MUST fail closed on malformed guard-policy configuration: until the configuration error is fixed, the affected GitHub MCP tools MUST NOT be enabled, rather than falling back to an unrestricted ("all repos", `min-integrity: none`) policy.
- Compilation error messages for malformed guard-policy configuration SHOULD identify the offending field, the value received, and the set of valid values or formats, so the misconfiguration can be corrected without consulting this specification.
- Implementations MUST NOT partially apply a malformed guard policy (for example, enforcing `min-integrity` while ignoring an invalid `allowed-repos` value); validation MUST treat the guard policy as a single unit that either fully passes validation or causes compilation to fail.
- When a username appears in both `blocked-users` and `trusted-users`, the blocked-user decision MUST take precedence and the item MUST be denied. This deterministic rule prevents a trusted-user grant from weakening an explicit deny list (T-GH-094).

### 9.7 Open Questions

Expand Down Expand Up @@ -2124,6 +2125,7 @@ Additional blocked-user validation tests in `TestValidateGitHubGuardPolicy`:
- **T-GH-091**: When both `blocked-users` and `min-integrity` are configured, access is the conjunction of P5_NotBlocked AND P6_IntegrityMet; a non-blocked user with content at or above the threshold is allowed
- **T-GH-092**: Non-blocked user with content exceeding the configured threshold is allowed; P5 and P6 both pass
- **T-GH-093**: Blocked user whose content also fails the integrity threshold is denied with `-32005` (P5_NotBlocked fires before P6_IntegrityMet per §8.5 combined evaluation order), not `-32006`
- **T-GH-094**: A user in both `blocked-users` and `trusted-users` is denied because `blocked-users` takes precedence

### 11.2 Compliance Checklist

Expand Down
15 changes: 13 additions & 2 deletions scratchpad/guard-policies-specification.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ tools:

> **Note**: The field was originally named `repos` and renamed to `allowed-repos` in PR #22331. The old name is retained as a deprecated alias; run `gh aw fix` to migrate automatically.

Runtime precedence is defined by the canonical [Evaluation Order](#41-evaluation-order) in Operations.

## Operations

### 4. MCP Gateway Configuration Flow
Expand All @@ -127,6 +129,16 @@ tools:
- Enforces policies on all tool invocations
- Blocks unauthorized repository access

### 4.1 Evaluation Order

The MCP Gateway MUST evaluate access in this order:

1. `lockdown: true` takes absolute precedence and denies the invocation; `allowed-repos` and `min-integrity` MUST NOT be evaluated.
2. When lockdown is not enabled, `allowed-repos` determines whether the target repository is in scope.
3. When the repository is in scope, `min-integrity` determines whether the content meets the required integrity level. Both checks MUST pass to allow the invocation.

Lockdown is an emergency security stop and MUST NOT be weakened by guard policies; those policies narrow access in an otherwise-open tool session but never grant access revoked by lockdown.

### 5. Safe Outputs Integration

When GitHub guard policies are configured, the compiler automatically derives a linked guard-policy for the safe-outputs MCP server. This ensures that safe output operations work correctly with guard policies by creating a write-sink configuration.
Expand Down Expand Up @@ -403,8 +415,7 @@ tools:

3. **How should conflicts between lockdown and guard policies be resolved?**

**Decision**: `lockdown: true` takes **absolute precedence** over guard policies. When `lockdown: true` is set, all tool invocations are blocked regardless of any `allowed-repos` or `min-integrity` configuration. Guard policies are not evaluated when lockdown is active.
*Rationale*: Lockdown is an emergency/security stop; it MUST NOT be weakened by other configuration. Guard policies narrow access within an otherwise-open tool session; they do not grant access that lockdown has revoked. The compiler SHOULD warn operators at compilation time when both `lockdown: true` and guard-policy fields (`allowed-repos`, `min-integrity`, `blocked-users`, `trusted-users`, `approval-labels`) are present, as the combination is likely a misconfiguration. This warning is now implemented in `pkg/workflow/tools_validation_github.go`, where `validateGitHubGuardPolicy()` detects the conflict and `emitGitHubLockdownGuardPolicyWarning()` surfaces the compiler warning.
**Decision**: The canonical [Evaluation Order](#41-evaluation-order) applies. The compiler SHOULD warn operators at compilation time when `lockdown: true` and guard-policy fields are both present; this is implemented in `pkg/workflow/tools_validation_github.go`, where `validateGitHubGuardPolicy()` detects the conflict and `emitGitHubLockdownGuardPolicyWarning()` surfaces the warning.

4. **Should we add a "dry-run" mode to test policies before enforcement?**

Expand Down
19 changes: 19 additions & 0 deletions scratchpad/safe-outputs-specification.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ This subsection clarifies how the Requirements Notation (§2.2) is applied consi
- When a requirement uses "SHOULD" or "RECOMMENDED", implementations that deviate MUST document the deviation and its rationale (for example in release notes or an architecture decision record).
- Conflicting requirements MUST NOT appear for the same conformance class; if a later section appears to narrow an earlier "MUST", the later section is normative and the earlier section MUST be read as superseded.
- Requirements scoped to a specific conformance class (§2.1) apply only to implementations claiming that class or higher; a Standard Conforming Implementation MUST also satisfy all Basic Conformance requirements.
- Changes to the Layer 3 and Layer 4 requirements MUST be reviewed using the implementation mappings in [Sync Notes](#sync-notes).

---

Expand Down Expand Up @@ -1266,6 +1267,24 @@ The system does NOT protect against:

---

## Sync Notes

| Specification area | Implementation mapping |
|---|---|
| §3.4 Layer 3 validation guardrails | `pkg/workflow/safe_outputs_validation.go`, `pkg/workflow/safe_outputs_validation_config.go` |
| §3.5 Layer 4 execution handlers | `pkg/workflow/safe_output_handlers.go`, `pkg/workflow/safe_outputs_actions.go` |
| §2.4 Norms | Review the Layer 3 and Layer 4 mappings above whenever a normative requirement changes. |

## Sync Follow-ups

When changing Layer 3 or Layer 4 behavior:

1. Verify the mapped `pkg/workflow/` implementation and its focused tests reflect the updated requirement.
2. Update this specification's conformance requirements when the implementation changes behavior.
3. Record any intentional implementation deviation and its rationale as required by §2.4.

---

## Change Log

### Version 1.1.0 (Recommendation)
Expand Down
3 changes: 2 additions & 1 deletion specs/awf-config-sources-compliance/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ The following test IDs cover the `DriftRecord` schema and its usage requirements
| T-DR-008 | §7.5.1 — corrective PR embeds records | The corrective PR description MUST embed the full `DriftRecord` list as JSON. | `pkg/workflow/awf_config_drift_test.go` |
| T-DR-009 | §7.5.1 — empty list is valid | An empty `DriftRecord` list (no drift detected) is a valid output and MUST NOT trigger corrective PR or escalation actions. | `pkg/workflow/awf_config_drift_test.go` |
| T-DR-010 | §7.2 Step 5 integration | The drift detection procedure Step 5 MUST produce a list of zero or more `DriftRecord` objects; the output format MUST be a JSON array conforming to the §3.1 schema. | `pkg/workflow/awf_config_drift_test.go` |
| T-DR-011 | §6 CR-06a — escalation-owner acknowledgement | Escalations select a non-empty owner using the documented fallback and require acknowledgement within one business day. | `pkg/workflow/awf_config_safeguards_formal_test.go` |

---

Expand Down Expand Up @@ -55,7 +56,7 @@ Conformance tests that validate `DriftRecord` schema compliance are implemented

```
pkg/workflow/awf_config_drift_test.go — DriftRecord schema validation and usage (T-DR-001 through T-DR-010; T-DR-005: TestDriftRecord_TDR005_NoAdditionalProperties)
pkg/workflow/awf_config_safeguards_formal_test.go — unavailable-source safeguards (T-DR-SAFE-001 through T-DR-SAFE-004)
pkg/workflow/awf_config_safeguards_formal_test.go — unavailable-source safeguards (T-DR-SAFE-001 through T-DR-SAFE-004) and CR-06a escalation-owner acknowledgement (T-DR-011)
```

To run related tests:
Expand Down
12 changes: 6 additions & 6 deletions specs/awf-config-sources-spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ This subsection identifies the normative behavior that conforming AWF config sou

**CR-06**: Drift categorized as "missing in gh-aw" or "spec mismatch" MUST be remediated (merged or explicitly waived with rationale) within **5 business days** of detection. For this requirement, business days are Monday-Friday in UTC, excluding weekends. If this SLA is missed, maintainers MUST open (or update) an escalation tracking issue within 1 business day. The escalation issue MUST include an owner, unblock plan, and revised ETA.

**CR-06a (Escalation Owner Assignment)**: When opening or updating an escalation tracking issue under CR-06, the assignee **SHOULD** be determined as follows: (a) the maintainer who merged the last change to the drifted property's corresponding implementation file in `pkg/workflow/` or `actions/setup/` is the **default escalation owner** (implementation guidance: this can be determined via `git log` on the relevant file, or through PR merge history); (b) if no such maintainer is identifiable (e.g., the property has never been implemented), the escalation owner **SHOULD** default to the on-call maintainer for the `github/gh-aw` repository at the time of escalation; (c) the assigned owner **MUST** be recorded in the `Owner` field of the escalation issue template and **MUST** acknowledge the assignment by commenting on the issue within 1 business day of assignment. The escalation issue **MUST NOT** be left unassigned.
**CR-06a (Escalation Owner Assignment)**: When opening or updating an escalation tracking issue under CR-06, the assignee **SHOULD** be determined as follows: (a) the maintainer who merged the last change to the drifted property's corresponding implementation file in `pkg/workflow/` or `actions/setup/` is the **default escalation owner** (implementation guidance: this can be determined via `git log` on the relevant file, or through PR merge history); (b) if no such maintainer is identifiable (e.g., the property has never been implemented), the escalation owner **SHOULD** default to the on-call maintainer for the `github/gh-aw` repository at the time of escalation; (c) the assigned owner **MUST** be recorded in the `Owner` field of the escalation issue template and **MUST** acknowledge the assignment by commenting on the issue within 1 business day of assignment. The escalation issue **MUST NOT** be left unassigned. The formal conformance fixture is [T-DR-011](awf-config-sources-compliance/README.md#driftrecord-conformance-tests), implemented in `pkg/workflow/awf_config_safeguards_formal_test.go`; production issue assignment and comment-based acknowledgement enforcement are not yet automated.

---

Expand Down Expand Up @@ -251,7 +251,7 @@ diff -u /tmp/schema-keys.txt /tmp/ghaw-refs.txt || true

A scheduled GitHub Actions workflow in `github/gh-aw` SHOULD automate this procedure. The workflow SHOULD:

- Run on a weekly schedule and on pull requests that touch AWF config handling.
- Run on a daily schedule and on pull requests that touch AWF config handling.
- Fail the check (non-zero exit) when any "missing in gh-aw" drift is found.
- Post a summary comment on PRs with the drift report.
- Create a tracking issue when drift is detected on the scheduled run.
Expand Down Expand Up @@ -308,7 +308,7 @@ The drift detection procedure (Section 7.2, Step 5) **MUST** produce a list of z

Every invocation (scheduled, manual, or ad hoc) **MUST** attempt to refresh the canonical sources and evaluate snapshot freshness. When canonical sources in `github/gh-aw-firewall` are unavailable (GitHub outage, auth failure, transient fetch errors), agents and automation MUST apply the following safeguards:

1. The workflow **MUST** attempt to use the last-known validated local snapshot (for example cached schema/spec artifacts from the previous successful run) to keep checks deterministic. The snapshot **MUST** be stored at a stable, well-known path: `~/.cache/gh-aw/schema-consistency/last-known-snapshot/` on self-hosted runners or `/tmp/gh-aw/agent/schema-consistency/last-known-snapshot/` when the runner is ephemeral. The workflow **MUST** record the UTC refresh time when, and only when, all canonical sources refresh successfully. Snapshots older than **7 days** (168 hours from that recorded successful refresh) **MUST** be treated as expired and **MUST NOT** be used to suppress drift warnings; when a snapshot is expired, the run **MUST** be marked degraded even if the snapshot files are physically present. Implementations **SHOULD** delete snapshots older than 14 days to prevent unbounded disk use. (T-DR-SAFE-001)
2. The workflow **SHOULD** emit a warning that canonical source retrieval failed, including the failing source path(s) and timestamp. (T-DR-SAFE-002)
3. The workflow **MUST** skip destructive validation actions (for example failing required checks, auto-opening corrective PRs, or auto-creating drift issues from stale snapshots) when canonical data cannot be refreshed, and mark the run as degraded instead of silently passing. (T-DR-SAFE-003)
4. The workflow **SHOULD** open or update a tracking issue when canonical source unavailability persists from one scheduled cron invocation through the next scheduled cron invocation. Manual reruns and ad hoc invocations still perform the refresh and freshness checks above, but do not advance this consecutive-scheduled-run threshold. (T-DR-SAFE-004)
1. The workflow **MUST** attempt to use the last-known validated local snapshot (for example cached schema/spec artifacts from the previous successful run) to keep checks deterministic. The snapshot **MUST** be stored at a stable, well-known path: `~/.cache/gh-aw/schema-consistency/last-known-snapshot/` on self-hosted runners or `/tmp/gh-aw/agent/schema-consistency/last-known-snapshot/` when the runner is ephemeral. The workflow **MUST** record the UTC refresh time when, and only when, all canonical sources refresh successfully. Snapshots older than **7 days** (168 hours from that recorded successful refresh) **MUST** be treated as expired and **MUST NOT** be used to suppress drift warnings; when a snapshot is expired, the run **MUST** be marked degraded even if the snapshot files are physically present. Implementations **SHOULD** delete snapshots older than 14 days to prevent unbounded disk use. (T-DR-SAFE-001; see the [fixture index](awf-config-sources-compliance/README.md#safeguards-conformance-tests))
2. The workflow **SHOULD** emit a warning that canonical source retrieval failed, including the failing source path(s) and timestamp. (T-DR-SAFE-002; see the [fixture index](awf-config-sources-compliance/README.md#safeguards-conformance-tests))
3. The workflow **MUST** skip destructive validation actions (for example failing required checks, auto-opening corrective PRs, or auto-creating drift issues from stale snapshots) when canonical data cannot be refreshed, and mark the run as degraded instead of silently passing. (T-DR-SAFE-003; see the [fixture index](awf-config-sources-compliance/README.md#safeguards-conformance-tests))
4. The workflow **SHOULD** open or update a tracking issue when canonical source unavailability persists from one daily scheduled cron invocation through the next. Manual reruns and ad hoc invocations still perform the refresh and freshness checks above, but do not advance this consecutive-scheduled-run threshold. This persistence threshold applies only to unavailable-source reporting; the daily cadence in §7.4 applies to the full drift check. (T-DR-SAFE-004; see the [fixture index](awf-config-sources-compliance/README.md#safeguards-conformance-tests))
Loading