Record the verified principal type on approver entries (schema v4)
An entry stored subject_id, assurance and evidence_ref but nothing about what kind of principal bound the approval, and subject_id is a naming convention rather than a verified claim. /entries is not restricted by principal type — only /consume is — and the approval-engine-operator client holds approval:approve, so a service can supply approver evidence today. Whether it may is gate-house doctrine; that it is legible is ours. Add entries.principal_type, populate it from the verified token, surface it on the object and the audit evidence path (not the claim, which stays least-disclosure), and migrate v3 stores leaving legacy rows null rather than back-filling a claim nobody made. Also corrects two statements in the requirements issued to informed-decision: agent tokens are not barred from approval:approve, and an empty assurance object is accepted rather than refused. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HyybaE7DUXrWYrhbnESCTe Assistant: claude-code Assistant-Model: opus Assistant-Process: 1275879@bnt-lap001 Assistant-Session: eb464208-f821-41b2-bc5a-a6c33d92a8ad
This commit is contained in:
parent
8b8ada6c4a
commit
31da1af5e4
6 changed files with 160 additions and 20 deletions
|
|
@ -145,6 +145,7 @@ class App:
|
|||
identity.subject,
|
||||
assurance=json.dumps(identity.assurance, sort_keys=True),
|
||||
evidence_ref=identity.evidence_ref,
|
||||
principal_type=identity.principal_type,
|
||||
)
|
||||
return 200, obj.as_dict()
|
||||
if rest == ["revoke"] and method == "POST":
|
||||
|
|
|
|||
|
|
@ -35,7 +35,7 @@ AUDIT_SCHEMA = "audit-core.event.v1alpha1"
|
|||
SOURCE = "approval-engine"
|
||||
SCOPE = "netkingdom-approvals"
|
||||
EVENT_CLASSES = ("issuance", "use", "supersession", "revocation", "heartbeat")
|
||||
LATEST_SCHEMA_VERSION = 3
|
||||
LATEST_SCHEMA_VERSION = 4
|
||||
|
||||
SCHEMA = """
|
||||
CREATE TABLE IF NOT EXISTS approvals (
|
||||
|
|
@ -66,6 +66,7 @@ CREATE TABLE IF NOT EXISTS entries (
|
|||
approved_at TEXT NOT NULL,
|
||||
assurance TEXT,
|
||||
evidence_ref TEXT,
|
||||
principal_type TEXT,
|
||||
PRIMARY KEY (approval_id, subject_id)
|
||||
);
|
||||
CREATE TABLE IF NOT EXISTS outbox (
|
||||
|
|
@ -230,6 +231,15 @@ class Engine:
|
|||
conn.execute(
|
||||
"ALTER TABLE approvals ADD COLUMN pdp_path INTEGER NOT NULL DEFAULT 0"
|
||||
)
|
||||
entry_columns = {
|
||||
row["name"] for row in conn.execute("PRAGMA table_info(entries)").fetchall()
|
||||
}
|
||||
if "principal_type" not in entry_columns:
|
||||
# v4. Legacy entries stay NULL. An entry recorded before this
|
||||
# column existed carries no verified statement about what kind
|
||||
# of principal bound it, and defaulting it to 'human' would
|
||||
# manufacture approver evidence that was never presented.
|
||||
conn.execute("ALTER TABLE entries ADD COLUMN principal_type TEXT")
|
||||
outbox_columns = {
|
||||
row["name"] for row in conn.execute("PRAGMA table_info(outbox)").fetchall()
|
||||
}
|
||||
|
|
@ -423,10 +433,11 @@ class Engine:
|
|||
"approved_at": e["approved_at"],
|
||||
"assurance": e["assurance"],
|
||||
"evidence_ref": e["evidence_ref"],
|
||||
"principal_type": e["principal_type"],
|
||||
}
|
||||
for e in self._conn()
|
||||
.execute(
|
||||
"SELECT subject_id, approved_at, assurance, evidence_ref "
|
||||
"SELECT subject_id, approved_at, assurance, evidence_ref, principal_type "
|
||||
"FROM entries WHERE approval_id=? ORDER BY approved_at",
|
||||
(row["id"],),
|
||||
)
|
||||
|
|
@ -485,6 +496,7 @@ class Engine:
|
|||
*,
|
||||
assurance: str | None = None,
|
||||
evidence_ref: str | None = None,
|
||||
principal_type: str | None = None,
|
||||
) -> Approval:
|
||||
if not subject_id:
|
||||
raise Unprocessable("subject_id is required")
|
||||
|
|
@ -501,9 +513,10 @@ class Engine:
|
|||
raise Conflict(f"cannot add entries in status {row['status']}")
|
||||
try:
|
||||
conn.execute(
|
||||
"INSERT INTO entries (approval_id, subject_id, approved_at, assurance, evidence_ref) "
|
||||
"VALUES (?,?,?,?,?)",
|
||||
(approval_id, subject_id, now, assurance, evidence_ref),
|
||||
"INSERT INTO entries "
|
||||
"(approval_id, subject_id, approved_at, assurance, evidence_ref, principal_type) "
|
||||
"VALUES (?,?,?,?,?,?)",
|
||||
(approval_id, subject_id, now, assurance, evidence_ref, principal_type),
|
||||
)
|
||||
except sqlite3.IntegrityError as exc:
|
||||
conn.rollback()
|
||||
|
|
@ -767,7 +780,7 @@ class Engine:
|
|||
a future schema that relaxes it.
|
||||
"""
|
||||
rows = conn.execute(
|
||||
"SELECT subject_id, approved_at, assurance, evidence_ref "
|
||||
"SELECT subject_id, approved_at, assurance, evidence_ref, principal_type "
|
||||
"FROM entries WHERE approval_id=? ORDER BY approved_at, subject_id",
|
||||
(approval_id,),
|
||||
).fetchall()
|
||||
|
|
@ -780,6 +793,7 @@ class Engine:
|
|||
"approved_at": r["approved_at"],
|
||||
**({"assurance": r["assurance"]} if r["assurance"] else {}),
|
||||
**({"evidence_ref": r["evidence_ref"]} if r["evidence_ref"] else {}),
|
||||
**({"principal_type": r["principal_type"]} if r["principal_type"] else {}),
|
||||
}
|
||||
for r in seen.values()
|
||||
]
|
||||
|
|
|
|||
|
|
@ -53,11 +53,22 @@ the deployed issuer's `/jwks` (RS256) and carry:
|
|||
The **access** token is what we validate. `id_token` appears nowhere in this
|
||||
codebase; the ID token belongs to the login client and is never evidence here.
|
||||
|
||||
Agents may assemble a memo but must never complete a binding act — that is
|
||||
`informed-decision`'s own design principle 10, and it matches this engine: an
|
||||
agent token cannot hold `approval:approve` under the requested registrations,
|
||||
and an entry recorded under an agent subject would be indistinguishable from a
|
||||
human's in the evidence chain.
|
||||
**Correction to our 2026-09-09 message.** We said an agent token cannot hold
|
||||
`approval:approve` under the requested registrations. That is wrong twice: the
|
||||
`approval-engine-operator` service client holds `approval:approve`, and this
|
||||
engine does not restrict `/entries` by principal type at all — only `/consume`
|
||||
is restricted, to `service`/`agent`. So a non-human principal can supply
|
||||
approver evidence today.
|
||||
|
||||
Whether it *should* be able to is approval doctrine and belongs to
|
||||
`gate-house`, not to us and not to you. What we have done instead is make it
|
||||
visible: schema v4 records the verified `principal_type` on every entry
|
||||
(`tests/test_auth.py::test_entry_records_the_verified_principal_type`,
|
||||
`::test_non_human_approver_is_recorded_as_such`). Your design principle 10
|
||||
("humans bind, agents draft") is therefore enforceable in the evidence chain
|
||||
rather than assumed — read `entries[].principal_type` and do not infer the
|
||||
answer from the shape of `subject_id`. Entries written before v4 are `null` and
|
||||
must not be read as `human`.
|
||||
|
||||
## 3. Open question A — the human client cannot read the approval it renders
|
||||
|
||||
|
|
@ -100,9 +111,13 @@ on the human client — the requirement here is that the resulting token's
|
|||
(authentication method, `acr`/`amr`, `auth_time` at minimum), rather than
|
||||
leaving MFA as a property of the login that nothing downstream can see.
|
||||
|
||||
`informed-decision` and `key-cape` own that shape between them. This engine will
|
||||
store whatever they agree on and will not silently accept an empty object as
|
||||
evidence of anything.
|
||||
`informed-decision` and `key-cape` own that shape between them. To be exact
|
||||
about what this engine does and does not do: an empty `assurance` object
|
||||
(`{}`) is *accepted* — validation checks only that the claim is present and is
|
||||
an object. So an empty assurance is not refused here; it simply produces an
|
||||
entry that evidences nothing about how the person authenticated. If MFA has to
|
||||
be *provable* from the approval record, the shape has to carry it, because
|
||||
nothing downstream reconstructs it.
|
||||
|
||||
## 5. Open question C — `view_hash` has nowhere to go today
|
||||
|
||||
|
|
|
|||
|
|
@ -20,8 +20,17 @@ cross-tenant reads and mutations are rejected before object lookup.
|
|||
| explicit heartbeat | `approval:emit` |
|
||||
|
||||
Create additionally requires `binding.actor == sub`. Approval-entry subject,
|
||||
assurance, and evidence reference are derived from the verified JWT, never the
|
||||
request body. KeyCape owns client registration and scope grants; approval-engine
|
||||
assurance, evidence reference, and **principal type** are derived from the
|
||||
verified JWT, never the request body.
|
||||
|
||||
`principal_type` is recorded on the entry (schema v4) because `subject_id`
|
||||
alone cannot answer what kind of principal bound the approval — `user:alice` is
|
||||
a naming convention, not a verified claim. This engine does not restrict
|
||||
`/entries` to human principals: whether a service or agent may supply approver
|
||||
evidence is approval doctrine and belongs to `gate-house`, and the
|
||||
`approval-engine-operator` registration holds `approval:approve` today. What
|
||||
this engine owes is that the evidence chain says which it was. Entries written
|
||||
before v4 stay `null` rather than being back-filled into a claim nobody made. KeyCape owns client registration and scope grants; approval-engine
|
||||
only verifies and enforces them. Requested registrations are:
|
||||
|
||||
- audience/resource server `approval-engine` with the scopes above;
|
||||
|
|
|
|||
|
|
@ -346,3 +346,80 @@ def test_human_principal_cannot_consume(engine):
|
|||
assert status == 200
|
||||
assert claim["consumed"] is False
|
||||
assert claim["valid_now"] is True
|
||||
|
||||
|
||||
def test_entry_records_the_verified_principal_type(engine):
|
||||
"""Approver evidence must say what kind of principal bound the approval.
|
||||
|
||||
`subject_id` alone cannot answer it: `user:alice` is a naming convention,
|
||||
not a verified claim. The recorded value comes from the token and from
|
||||
nowhere else, so an evidence reader can tell a human bind from a service
|
||||
one without trusting a string's shape.
|
||||
"""
|
||||
from approval_engine.api import App
|
||||
|
||||
creator = Identity(
|
||||
subject="agt-secrets-engine",
|
||||
issuer="test",
|
||||
audiences=("approval-engine",),
|
||||
principal_type="service",
|
||||
tenant="tenant:platform",
|
||||
roles=frozenset(),
|
||||
scopes=frozenset({"approval:create", "approval:read"}),
|
||||
assurance={"level": "aal1"},
|
||||
evidence_ref="service",
|
||||
)
|
||||
human = Identity(
|
||||
subject="user:alice",
|
||||
issuer="test",
|
||||
audiences=("approval-engine",),
|
||||
principal_type="human",
|
||||
tenant="tenant:platform",
|
||||
roles=frozenset(),
|
||||
scopes=frozenset({"approval:approve"}),
|
||||
assurance={"level": "aal2", "amr": ["pwd", "otp"]},
|
||||
evidence_ref="human",
|
||||
)
|
||||
app = App(engine, StaticTokenAuthenticator({"creator": creator, "human": human}))
|
||||
_, created = call(
|
||||
app,
|
||||
"POST",
|
||||
"/v1/approvals",
|
||||
{"binding": binding(), "validity": validity()},
|
||||
authorization="Bearer creator",
|
||||
)
|
||||
_, approved = call(
|
||||
app,
|
||||
"POST",
|
||||
f"/v1/approvals/{created['id']}/entries",
|
||||
{"principal_type": "service"},
|
||||
authorization="Bearer human",
|
||||
)
|
||||
entry = approved["entries"][0]
|
||||
assert entry["subject_id"] == "user:alice"
|
||||
assert entry["principal_type"] == "human"
|
||||
|
||||
_, claim = call(
|
||||
app, "GET", f"/v1/approvals/{created['id']}/claim", authorization="Bearer creator"
|
||||
)
|
||||
assert "approvers" not in claim
|
||||
|
||||
|
||||
def test_non_human_approver_is_recorded_as_such(app):
|
||||
"""A service principal holding `approval:approve` is not refused — but it
|
||||
is not silently indistinguishable from a human either.
|
||||
|
||||
Whether a non-human may supply approver evidence at all is approval
|
||||
doctrine and belongs to gate-house; the `approval-engine-operator`
|
||||
registration holds `approval:approve` today. This engine's obligation is
|
||||
that the evidence chain records which it was.
|
||||
"""
|
||||
_, created = call(
|
||||
app,
|
||||
"POST",
|
||||
"/v1/approvals",
|
||||
{"binding": binding(), "validity": validity()},
|
||||
)
|
||||
_, approved = call(app, "POST", f"/v1/approvals/{created['id']}/entries", {})
|
||||
assert approved["status"] == "approved"
|
||||
assert approved["entries"][0]["principal_type"] == "service"
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ from pathlib import Path
|
|||
import pytest
|
||||
|
||||
from approval_engine.errors import Conflict, Unprocessable
|
||||
from approval_engine.store import Engine
|
||||
from approval_engine.store import LATEST_SCHEMA_VERSION, Engine
|
||||
from tests.conftest import FROZEN, approve, binding, validity
|
||||
|
||||
|
||||
|
|
@ -174,8 +174,8 @@ def test_successor_inherits_the_pdp_path_declaration(engine):
|
|||
assert successor.pdp_path is True
|
||||
|
||||
|
||||
def test_v2_database_migrates_to_v3_preserving_approvals():
|
||||
"""Schema v3 (GH-DEC-2026-008) must not disturb approvals issued under v2."""
|
||||
def test_v2_database_migrates_forward_preserving_approvals():
|
||||
"""A v2 store must upgrade to the current schema without disturbing approvals."""
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
path = Path(tmp) / "v2.sqlite"
|
||||
eng = Engine(path, clock=lambda: FROZEN)
|
||||
|
|
@ -189,7 +189,7 @@ def test_v2_database_migrates_to_v3_preserving_approvals():
|
|||
|
||||
upgraded = Engine(path, clock=lambda: FROZEN)
|
||||
version = int(upgraded._conn().execute("PRAGMA user_version").fetchone()[0])
|
||||
assert version == 3
|
||||
assert version == LATEST_SCHEMA_VERSION
|
||||
survivor = upgraded.get(obj.id)
|
||||
assert survivor.status == "approved"
|
||||
assert survivor.pdp_digest == "sha256:" + "12" * 32
|
||||
|
|
@ -197,4 +197,28 @@ def test_v2_database_migrates_to_v3_preserving_approvals():
|
|||
# a digest that happens to be present
|
||||
assert survivor.pdp_path is False
|
||||
assert upgraded.claim(obj.id)["binding"]["pdp_path"] is False
|
||||
# v4: a legacy entry carries no verified statement about the kind of
|
||||
# principal that bound it, and must not be back-filled into one
|
||||
assert survivor.entries[0]["principal_type"] is None
|
||||
upgraded.close()
|
||||
|
||||
|
||||
def test_v3_entries_migrate_to_v4_without_inventing_a_principal_type():
|
||||
"""The v4 column is added; pre-v4 approver entries stay unclassified."""
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
path = Path(tmp) / "v3.sqlite"
|
||||
eng = Engine(path, clock=lambda: FROZEN)
|
||||
obj = eng.create(binding(), validity())
|
||||
eng.add_entry(obj.id, "user:alice", principal_type="human")
|
||||
eng._conn().execute("ALTER TABLE entries DROP COLUMN principal_type")
|
||||
eng._conn().execute("PRAGMA user_version=3")
|
||||
eng._conn().commit()
|
||||
eng.close()
|
||||
|
||||
upgraded = Engine(path, clock=lambda: FROZEN)
|
||||
assert int(upgraded._conn().execute("PRAGMA user_version").fetchone()[0]) == 4
|
||||
survivor = upgraded.get(obj.id)
|
||||
assert survivor.status == "approved"
|
||||
assert survivor.entries[0]["subject_id"] == "user:alice"
|
||||
assert survivor.entries[0]["principal_type"] is None
|
||||
upgraded.close()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue