Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/482ee193-e20d-4cde-afed-7480840c2e50 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…y (Section 11, Threat T6) Agent-Logs-Url: https://github.com/github/gh-aw/sessions/482ee193-e20d-4cde-afed-7480840c2e50 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…fix, script normativity, legacy key format Agent-Logs-Url: https://github.com/github/gh-aw/sessions/482ee193-e20d-4cde-afed-7480840c2e50 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates the Safe Outputs specification to v1.15.0 by documenting integrity-aware cache-memory behavior (policy-scoped keys + git-backed integrity branching) to prevent cross-integrity cache poisoning, and refreshes generated workflow lockfiles.
Changes:
- Add §11 “Cache Memory Integrity” with CI1–CI12 requirements (key format, policy hash, git branch isolation, merge-down, commit/gc, migration).
- Add Threat T6 (cache integrity poisoning) and new terminology entries related to integrity and caching.
- Regenerate workflow
.lock.ymlfiles reflecting updated resolved-manifest ordering.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/src/content/docs/reference/safe-outputs-specification.md | Adds cache-memory integrity section, Threat T6, terminology, and bumps spec version/date. |
| .github/workflows/typist.lock.yml | Regenerated lockfile (resolved imports ordering). |
| .github/workflows/terminal-stylist.lock.yml | Regenerated lockfile (resolved imports ordering). |
| .github/workflows/smoke-copilot.lock.yml | Regenerated lockfile (resolved imports ordering). |
| .github/workflows/smoke-copilot-arm.lock.yml | Regenerated lockfile (resolved imports ordering). |
| .github/workflows/smoke-codex.lock.yml | Regenerated lockfile (resolved imports ordering). |
| .github/workflows/smoke-claude.lock.yml | Regenerated lockfile (resolved imports ordering). |
| .github/workflows/sergo.lock.yml | Regenerated lockfile (resolved imports ordering). |
| .github/workflows/semantic-function-refactor.lock.yml | Regenerated lockfile (resolved imports ordering). |
| .github/workflows/repository-quality-improver.lock.yml | Regenerated lockfile (resolved imports ordering). |
| .github/workflows/q.lock.yml | Regenerated lockfile (resolved imports ordering). |
| .github/workflows/mcp-inspector.lock.yml | Regenerated lockfile (resolved imports ordering). |
| .github/workflows/go-fan.lock.yml | Regenerated lockfile (resolved imports ordering). |
| .github/workflows/glossary-maintainer.lock.yml | Regenerated lockfile (resolved imports ordering). |
| .github/workflows/duplicate-code-detector.lock.yml | Regenerated lockfile (resolved imports ordering). |
| .github/workflows/developer-docs-consolidator.lock.yml | Regenerated lockfile (resolved imports ordering). |
| .github/workflows/daily-testify-uber-super-expert.lock.yml | Regenerated lockfile (resolved imports ordering). |
| .github/workflows/daily-function-namer.lock.yml | Regenerated lockfile (resolved imports ordering). |
| .github/workflows/daily-file-diet.lock.yml | Regenerated lockfile (resolved imports ordering). |
| .github/workflows/daily-compiler-quality.lock.yml | Regenerated lockfile (resolved imports ordering). |
| .github/workflows/cloclo.lock.yml | Regenerated lockfile (resolved imports ordering). |
| .github/workflows/archie.lock.yml | Regenerated lockfile (resolved imports ordering). |
Comments suppressed due to low confidence (1)
docs/src/content/docs/reference/safe-outputs-specification.md:4314
- The migration text claims legacy cache entries “will never match the new restore keys,” but CI2 currently specifies a
memory-restore-key fallback that would match legacy caches. Please reconcile CI2 and §11.10 (either remove the legacy restore fallback, or revise the migration section to describe when/why legacy restores may still occur and how they remain safe).
Existing deployments using the pre-integrity cache format MUST expect a **cache miss** on the first run after upgrading to an implementation supporting this section.
**Legacy key format** (before this section):
memory-{workflowID}-{runID}
Example: memory-my-workflow-12345678
**New key format** (this section):
memory-{integrityLevel}-{policyHash}-{workflowID}-{runID}
Example (with policy): memory-unapproved-7e4d9f12-my-workflow-12345678
Example (without policy): memory-none-nopolicy-my-workflow-12345678
The integrity level and policy hash prefixes are new components not present in legacy keys. Because the key formats differ, legacy cache entries will never match the new restore keys, resulting in a one-time cache miss.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| trusted-users:{canonicalTrustedUsers} | ||
| ``` | ||
|
|
There was a problem hiding this comment.
CI3’s canonical policy string construction is ambiguous about trailing newlines: step (2) says each field is followed by a newline, but the example only shows \n on some lines and not after the final field. Please explicitly specify whether the canonical string includes a terminal newline after the last field to avoid hash mismatches between implementations.
| trusted-users:{canonicalTrustedUsers} | |
| ``` | |
| trusted-users:{canonicalTrustedUsers}\n |
The canonical policy string MUST include the trailing newline after the final trusted-users field.
| ▼ | ||
| commit_cache_memory_git.sh [if: always()] | ||
| 1. git add -A | ||
| 2. git commit --allow-empty -m "run-{run_id}" |
There was a problem hiding this comment.
The lifecycle diagram uses run-{run_id} for the commit message, while CI10 specifies run-{GITHUB_RUN_ID}. Please align the placeholder naming (and ideally match the earlier github.run_id terminology) to avoid confusion for implementers.
| 2. git commit --allow-empty -m "run-{run_id}" | |
| 2. git commit --allow-empty -m "run-{github.run_id}" |
| **Threat T6: Cache Integrity Poisoning** | ||
|
|
||
| *Attack Vector*: A `none`-integrity agent writes malicious or attacker-controlled data to a shared cache-memory directory. A subsequent `merged` or `approved`-integrity run blindly restores and consumes that data, violating the Bell-LaPadula write-up property (lower-integrity subjects MUST NOT write where higher-integrity subjects read). | ||
|
|
There was a problem hiding this comment.
Section 3.2 still states the spec addresses “five primary threat scenarios”, but a new Threat T6 has been added. Please update the threat count language (and any other references to “five threats”, e.g. in the appendices) so it remains consistent with the numbered threat list.
| restore-keys: | | ||
| memory-{integrityLevel}-{policyHash}-{workflowID}- | ||
| memory-{integrityLevel}-{policyHash}- | ||
| memory- | ||
| ``` | ||
|
|
||
| The final fallback `memory-` entry exists solely to allow migration from legacy (non-scoped) caches and MUST be removed in a future major version. |
There was a problem hiding this comment.
CI2’s restore-key cascade includes a final fallback of memory-, which allows restoring legacy (non-integrity-scoped) caches across integrity boundaries. That undermines CI1’s isolation guarantees and can reintroduce cache poisoning for higher-integrity runs. Consider removing this fallback, or tightly scoping it to an explicit, one-time migration path that cannot restore data produced by lower-integrity runs into higher-integrity contexts.
This issue also appears on line 4298 of the same file.
Documents the integrity-aware cache-memory architecture from #23425, which closes a Bell-LaPadula write-up hole where a
none-integrity agent could poison a cache later consumed by amerged-integrity run.Changes
New §11 "Cache Memory Integrity" — 12 normative requirements (CI1–CI12) covering:
memory-{integrityLevel}-{policyHash}-{workflowID}-{runID}nopolicysentinel for unconfigured workflows)-X theirs; reverse never occurs)git gc --automergedbranch)New Threat T6 in §3.2 — cache integrity poisoning with mitigation table
4 new terminology entries — Integrity Level, Policy Hash, Integrity Branch, Cache Poisoning
Key format
Merge semantics
mergedmergedonlyapprovedapproved+mergedunapprovedunapproved+approved+mergednone