security: enforce repository auth invariants - #7
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 860437c469
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // Role numbers are ordered from most to least privileged (OPERATOR=0). | ||
| // Delegating account creation must never permit privilege escalation. | ||
| if (role < creator->role) { |
There was a problem hiding this comment.
Validate the creator against authoritative auth state
This comparison trusts the caller-owned creator->role; because qihse_user_t is a public, mutable struct and this function never confirms that the context matches an authenticated entry in users, a caller can pass a forged stack object (or change an Analyst's role to QIHSE_ROLE_OPERATOR) and successfully create an Operator. Resolve the creator's privileges from authoritative authenticated state before applying this boundary.
AGENTS.md reference: AGENTS.md:L17-L21
Useful? React with 👍 / 👎.
|
|
||
| // Role numbers are ordered from most to least privileged (OPERATOR=0). | ||
| // Delegating account creation must never permit privilege escalation. | ||
| if (role < creator->role) { |
There was a problem hiding this comment.
Preserve the creator's hardware-token requirement
When a delegated creator has requires_hardware_token == true, this role-only check still permits creating a same-role account with requires_hw_token == false; that child can authenticate and access classified data without the control imposed on its creator. The privilege comparison must reject weakening the hardware-token policy, not only role escalation.
AGENTS.md reference: AGENTS.md:L17-L21
Useful? React with 👍 / 👎.
|
|
||
| // Role numbers are ordered from most to least privileged (OPERATOR=0). | ||
| // Delegating account creation must never permit privilege escalation. | ||
| if (role < creator->role) { |
There was a problem hiding this comment.
Reject passwordless delegated account creation
A delegated principal that passes this role check can supply plaintext_password == NULL; the later branch hashes an empty string, so qihse_auth_authenticate_id accepts the publicly known empty password even though non-null passwords must be at least 12 characters. This allows creation of an equally ranked account with an authentication bypass and must be denied or assigned a non-authenticatable state.
AGENTS.md reference: AGENTS.md:L17-L21
Useful? React with 👍 / 👎.
Adds hard repository security invariants in root
AGENTS.md, fixes delegated account creation so a principal cannot create a more privileged role than itself, adds regression coverage for Analyst→Operator and Guest→Analyst escalation attempts, and makes that regression a CI gate.Security invariants: