Skip to content

docs: Update the threat model and associated reporting skill - #3355

Merged
Benjamin Elder (BenTheElder) merged 1 commit into
agent-substrate:mainfrom
destijl:threat-model-status-accuracy
Oct 31, 2026
Merged

docs: Update the threat model and associated reporting skill#3355
Benjamin Elder (BenTheElder) merged 1 commit into
agent-substrate:mainfrom
destijl:threat-model-status-accuracy

Conversation

@destijl

@destijl Greg Castle (destijl) commented Oct 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

The security-status-report skill passes every column of docs/threats.json to its evaluators, so implementation-status claims and stale notes in the threat model were steering the scores instead of the code. This rewrites the suggested mitigations and notes as short design guidance with no status, and keeps threats and invariants unchanged except:

  • New Trust Boundaries section: the sandbox is the primary boundary, the worker Pod is trusted with the actors it hosts (its own confinement is defense in depth for the worker-to-node boundary), and a threat that needs another control to fail first is High priority at most
  • T-12 removed (documentation alone could never close it); other IDs keep their numbers
  • T-01 to T-03 lowered to High, since they require the outer network layer to fail first
  • T-11 invariant: the official path for secrets keeps them out of the sandbox
  • T-21 lowered to High, since it requires a sandbox escape first. Each worker permission beyond the container defaults needs a documented reason; the ones the sandbox runtime needs while workers share the host user namespace (root, runtime capabilities, AppArmor Unconfined) are accepted. A worker must not be able to modify or execute anything atelet or other workers trust
  • T-25 reworded to a malicious, not corrupt, snapshot, and lowered to Medium, since it requires control of snapshot storage first (same attackers as T-39)
  • T-26: enforcement points may cache policy as long as changes take effect within a short time, and must deny rather than serve stale policy indefinitely. Open connections and already-issued credentials are out of scope
  • T-30 lowered to Medium with the TBD invariant replaced, since nodes are not a boundary between actors

Skill changes: evaluators score against the invariants rather than treating suggested mitigations as a checklist, don't penalize items a threat puts outside Substrate's scope or that its notes accept with a documented reason, and use a defined 0.0 to 1.0 scale. Each evaluator also writes a short mitigation summary and remaining gap; the new render_table.py renders them as a markdown table sorted by priority, and the chart now groups threats by priority.

AI use: AI-assisted. I reviewed each section, and validated the rewrite with before/after evaluator runs and a full skill run.

  • Tests pass
  • Docs updated

@destijl
Greg Castle (destijl) force-pushed the threat-model-status-accuracy branch from 495217a to a0ad550 Compare October 29, 2026 00:57
@destijl
docs: remove status claims from the threat model and shorten it
6fdbc98 
The security-status-report skill feeds every column of docs/threats.json
to its evaluators, so status claims and stale notes in the threat model
steered the scores away from what the code does. Rewrite the suggested
mitigations and notes as short design guidance with no implementation
status, and keep the threats and invariants unchanged except:

- add a Trust Boundaries section: the sandbox is the primary boundary,
  the worker is trusted with the actors it hosts, and a threat that
  needs another control to fail first is High priority at most
- remove T-12, which no amount of documentation could close; other IDs
  keep their numbers
- T-01 to T-03: lower to High, since they need the outer network layer
  to fail first
- T-11: the official path for secrets keeps them out of the sandbox
- T-21: lower to High, since it needs a sandbox escape first. Require a
  documented reason for each worker permission beyond the container
  defaults, accept the ones the sandbox runtime needs while workers
  share the host user namespace, and require that a worker can't modify
  or execute anything atelet or other workers trust
- T-25: the threat is a malicious snapshot, not a corrupt one. Lower to
  Medium, since it needs control of snapshot storage first
- T-26: allow enforcement points to cache policy if changes take effect
  within a short time, and require them to deny rather than serve stale
  policy indefinitely. Open connections and already-issued credentials
  are out of scope
- T-30: lower to Medium and replace the TBD invariant, since nodes are
  not a boundary between actors

In the skill, tell the evaluators to score against the invariants
rather than treating the suggested mitigations as a checklist, not to
penalize items a threat puts outside Substrate's scope or that its
notes accept with a documented reason, and what the ends of the 0.0 to
1.0 scale mean. Each evaluator also writes a short mitigation summary
and remaining gap, which a new render_table.py turns into a markdown
table sorted by priority. The chart groups threats by priority too.

Signed-off-by: Greg Castle <[email protected]>
@destijl
Greg Castle (destijl) force-pushed the threat-model-status-accuracy branch from a0ad550 to 6fdbc98 Compare October 29, 2026 23:31
BenTheElder
Benjamin Elder (BenTheElder) approved these changes Oct 31, 2026
View reviewed changes

@BenTheElder Benjamin Elder (BenTheElder) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nits but nothing blocking, merge and iterate.

Comment thread .agents/skills/security-status-report/scripts/report_test.py

"""Tests for compile_report.py and render_table.py.

Run with: python3 -m unittest discover -s .agents/skills/security-status-report/scripts

@BenTheElder Benjamin Elder (BenTheElder) Oct 31, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't being run by CI, how often do we expect to modify it? (since CI time is not free and targeted CI jobs aren't easy to make blocking in GHA)

Comment thread docs/security/threat-model.md
| T-18 | Critical | Malicious actor gains access to the underlying node or other actors via filesystem | Filesystem access is limited to the actor's local fs and remote filesystems the actor is directly, explicitly authorized to access. | Isolate actor filesystems with user namespaces or per-actor UIDs. Guard filesystem setup and APIs against symlink, mount, and path traversal. | Vectors: file-read bugs, symlink and path traversal, OCI image unpacking, shared directories on root or privileged workers. |
| T-19 | Critical | Malicious actor gains access to the underlying Worker Pod or node via local Substrate services (atelet, ateworker, etc). | Actor access to system services must be denied or explicitly scoped to the actor. Local services must run with the fewest privileges possible to limit the ability for an actor to escalate privilege. | Limit actor access to local services. Local services authenticate the actor and authorize by its identity. | |
| T-20 | Critical | Malicious actor gains access to the underlying node or other actors via Substrate APIs (remote ate-apiserver) | Either actors are completely unable to access Substrate APIs, or actor access is authenticated and authorized such that the actor cannot escalate beyond its intended scope. Especially prevent self-modification, which was a common escalation path in K8s. | Don't let actors or workers modify themselves, e.g. read or write their own snapshots or change their own labels. Keep resource-modifying logic in the control plane. | |
| T-21 | High | Attacker who has escaped the sandbox escalates to the node, atelet, or other workers via excessive Worker Pod permissions | Worker Pods hold only the permissions sandbox lifecycle needs, and each permission beyond the container defaults has a documented reason. A worker can't modify or execute anything that atelet or other workers trust. | Run worker Pods non-root or in a user namespace, unprivileged, with only required capabilities and devices. A tailored seccomp profile instead of Unconfined. No ServiceAccount token. Mount only the worker's own host paths, read-only where it doesn't write. Verify shared assets before every use. | Rated High priority because it requires a sandbox escape first (T-15). Until workers run in their own user namespace, they may run as root, hold the capabilities the sandbox runtime needs (for example, `SYS_ADMIN` for mounts and `pivot_root`, and `NET_ADMIN` to build the actor network), and run with AppArmor Unconfined, because the default profile denies `mount`. By design, a worker can observe and modify the actors it hosts (such as pause containers that share a cgroup). |

@BenTheElder Benjamin Elder (BenTheElder) Oct 31, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 non-blocking: the worker also runs seccomp Unconfined, because the sandbox child calls pivot_root and the default profile denies it (workerSecurityContext, cmd/atecontroller/internal/controllers/workerpool_apply.go:405-450). Only AppArmor Unconfined is accepted here though, and the tailored seccomp profile is still listed as a mitigation.

Both tailored profiles are follow-ups today (#2749 for seccomp). Should we accept both or neither? Otherwise every run penalizes one and not the other.

Comment thread docs/security/threat-model.md
| T-18 | Critical | Malicious actor gains access to the underlying node or other actors via filesystem | Filesystem access is limited to the actor's local fs and remote filesystems the actor is directly, explicitly authorized to access. | Isolate actor filesystems with user namespaces or per-actor UIDs. Guard filesystem setup and APIs against symlink, mount, and path traversal. | Vectors: file-read bugs, symlink and path traversal, OCI image unpacking, shared directories on root or privileged workers. |
| T-19 | Critical | Malicious actor gains access to the underlying Worker Pod or node via local Substrate services (atelet, ateworker, etc). | Actor access to system services must be denied or explicitly scoped to the actor. Local services must run with the fewest privileges possible to limit the ability for an actor to escalate privilege. | Limit actor access to local services. Local services authenticate the actor and authorize by its identity. | |
| T-20 | Critical | Malicious actor gains access to the underlying node or other actors via Substrate APIs (remote ate-apiserver) | Either actors are completely unable to access Substrate APIs, or actor access is authenticated and authorized such that the actor cannot escalate beyond its intended scope. Especially prevent self-modification, which was a common escalation path in K8s. | Don't let actors or workers modify themselves, e.g. read or write their own snapshots or change their own labels. Keep resource-modifying logic in the control plane. | |
| T-21 | High | Attacker who has escaped the sandbox escalates to the node, atelet, or other workers via excessive Worker Pod permissions | Worker Pods hold only the permissions sandbox lifecycle needs, and each permission beyond the container defaults has a documented reason. A worker can't modify or execute anything that atelet or other workers trust. | Run worker Pods non-root or in a user namespace, unprivileged, with only required capabilities and devices. A tailored seccomp profile instead of Unconfined. No ServiceAccount token. Mount only the worker's own host paths, read-only where it doesn't write. Verify shared assets before every use. | Rated High priority because it requires a sandbox escape first (T-15). Until workers run in their own user namespace, they may run as root, hold the capabilities the sandbox runtime needs (for example, `SYS_ADMIN` for mounts and `pivot_root`, and `NET_ADMIN` to build the actor network), and run with AppArmor Unconfined, because the default profile denies `mount`. By design, a worker can observe and modify the actors it hosts (such as pause containers that share a cgroup). |

@BenTheElder Benjamin Elder (BenTheElder) Oct 31, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"pause containers that share a cgroup" -- we have no pause containers now.

Comment thread docs/security/threat-model.md
| T-22 | Critical | Malicious actor gains access to the underlying node or other actors via Kubernetes APIs | Either actors are completely unable to access the Kubernetes APIs, or actor access is authenticated and authorized such that the actor cannot escalate beyond its intended scope. **Strong preference on blocking actor access to Kubernetes.** | Block network access to the Kubernetes API. Give worker Pods no service account token, or one with no privileges. | |
| T-23 | Critical | Malicious actor workload gains access to snapshots of other actors and steals data from them. | Worker Pods and actors must not have direct access to snapshots. | Make snapshot credentials usable only by atelet, via extra claims or binding to atelet's mTLS. Envelope encrypt snapshots, keyed to actor identity. Avoid snapshotting credentials. | |
| T-24 | Critical | Malicious actor workload overrides snapshots of other actors with malicious snapshots. | Worker Pods and actors must not have direct access to snapshots. | Make snapshot credentials usable only by atelet, via extra claims or binding to atelet's mTLS. | |
| T-25 | Medium | Malicious snapshot is restored | Cryptographically verify each snapshot before restoration | Check snapshots against a trusted digest or signature before restore. Deliver the digest or key over a trusted channel. | Rated Medium priority because it requires control of snapshot storage, or of the path to it, which is a failure of other controls (T-23, T-24, T-36). Those with that access are mostly storage administrators, as in T-39. |

@BenTheElder Benjamin Elder (BenTheElder) Oct 31, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 non-blocking: storage admins aren't the only ones who can supply the snapshot. A template author can set storage_location (api/ateapi/v1/ateapi.proto:1269, behind PerTemplateSnapshotLocation), and T-32 is the actor writing its own.

On micro-VM the restore also reads the VM config from the snapshot dir in cloud-hypervisor, outside the guest (cmd/ateworker-microvm/internal/ch/restorefds.go:91), so a malicious snapshot is a way out of the sandbox:
https://github.com/cloud-hypervisor/cloud-hypervisor/blob/9ed824d6d08df3e96f7d5f50795d9449ac99f431/vmm/src/lib.rs#L2354-L2356

Under the new Trust Boundaries rule shouldn't this be High rather than Medium?

Comment thread .agents/skills/security-status-report/scripts/render_table.py
scored = [t["quality"] for t in rows if "error" not in t and "quality" in t]
lines = []
if scored:
lines += [f"Mean score: {statistics.mean(scored):.2f}", ""]

@BenTheElder Benjamin Elder (BenTheElder) Oct 31, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 nit: the mean skips ERROR rows without saying how many, while the chart draws them as 0. Something like Mean score: 0.90 (38 of 42 scored) would keep a run with timeouts from reading better than it is.

@BenTheElder Benjamin Elder (BenTheElder) added kind/docs Improvements or additions to documentation area/security Security related issue/pr labels Oct 31, 2026
@BenTheElder
Benjamin Elder (BenTheElder) added this pull request to the merge queue Oct 31, 2026
Merged via the queue into agent-substrate:main with commit 8a2c015 Oct 31, 2026
7 checks passed
@EItanya Eitan Yarmush (EItanya) mentioned this pull request Nov 2, 2026
Merged
1 of 2 tasks
Dmitry Berkovich (dberkov) pushed a commit that referenced this pull request Nov 2, 2026
@destijl @dberkov
docs: Update the threat model and associated reporting skill (#3355)
f70502d 
The security-status-report skill passes every column of
`docs/threats.json` to its evaluators, so implementation-status claims
and stale notes in the threat model were steering the scores instead of
the code. This rewrites the suggested mitigations and notes as short
design guidance with no status, and keeps threats and invariants
unchanged except:

- New Trust Boundaries section: the sandbox is the primary boundary, the
worker Pod is trusted with the actors it hosts (its own confinement is
defense in depth for the worker-to-node boundary), and a threat that
needs another control to fail first is High priority at most
- T-12 removed (documentation alone could never close it); other IDs
keep their numbers
- T-01 to T-03 lowered to High, since they require the outer network
layer to fail first
- T-11 invariant: the official path for secrets keeps them out of the
sandbox
- T-21 lowered to High, since it requires a sandbox escape first. Each
worker permission beyond the container defaults needs a documented
reason; the ones the sandbox runtime needs while workers share the host
user namespace (root, runtime capabilities, AppArmor Unconfined) are
accepted. A worker must not be able to modify or execute anything atelet
or other workers trust
- T-25 reworded to a malicious, not corrupt, snapshot, and lowered to
Medium, since it requires control of snapshot storage first (same
attackers as T-39)
- T-26: enforcement points may cache policy as long as changes take
effect within a short time, and must deny rather than serve stale policy
indefinitely. Open connections and already-issued credentials are out of
scope
- T-30 lowered to Medium with the TBD invariant replaced, since nodes
are not a boundary between actors

Skill changes: evaluators score against the invariants rather than
treating suggested mitigations as a checklist, don't penalize items a
threat puts outside Substrate's scope or that its notes accept with a
documented reason, and use a defined 0.0 to 1.0 scale. Each evaluator
also writes a short mitigation summary and remaining gap; the new
`render_table.py` renders them as a markdown table sorted by priority,
and the chart now groups threats by priority.

AI use: AI-assisted. I reviewed each section, and validated the rewrite
with before/after evaluator runs and a full skill run.

- [x] Tests pass
- [x] Docs updated

Signed-off-by: Greg Castle <[email protected]>
(cherry picked from commit 8a2c015)
Signed-off-by: Eitan Yarmush <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BenTheElder Benjamin Elder (BenTheElder) BenTheElder approved these changes

Assignees

No one assigned

Labels

area/security Security related issue/pr kind/docs Improvements or additions to documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@destijl @BenTheElder