Skip to content

human in the loop - #10514

Merged
briansrls merged 13 commits into
mainfrom
session/crisp-ram-671-access-grants
Sep 5, 2026
Merged

briansrls merged 13 commits into
mainfrom
session/crisp-ram-671-access-grants

Conversation

@gunbai-bot

@gunbai-bot gunbai-bot Bot commented Sep 5, 2026 •

Copy link
Copy Markdown
Contributor

Groundwork for human-in-the-loop access grants. This is phase (a) only — it does not deliver the approval flow, and nothing here consumes approval evidence before a privileged write. See "What this deliberately does not do".

What this changes

The access-grant cell became a request. secret_access_ensure_for could name only one cell shape: its target was already a parameter, but role was a constant inside the desired-cell builder and condition was hardcoded absent. So "which access is being asked for" was a fact about the source rather than about a request. Role and condition now travel on a SecretAccessGrant — one secret, one role, one condition.

Carrying condition is what makes a time-bounded grant ownable. Previously the owned-cell universe was the unconditioned cell, so a conditioned cell was foreign and preserved in silence — a grant could outlive its request with nothing able to count it. conditioned_cells_without_grant partitions observed conditioned cells against the granted set and returns the unmatched ones. It reports and never removes: revocation belongs to whoever owns the request set.

The reconciler moved to gunbc.auth.gcp_secret_access. It lived under gunbc.spark because spark was its first consumer; once the gunbai-ci App key became a second caller in another domain, the path asserted an ownership that was not true (DESIGN §3 homes a fact by its layer). The spark module keeps a wrapper naming its own target, and both entries keep their names.

OrgAdminAppKeyUnreadable and OrgAdminInstallationTokenRefused are now arms of OrgAdminCredentialRefusal. They already existed as refusal text in the CI prelude — named, located, load-bearing, invisible to the type system — so calling them "typed and located" was inflation on a rung whose evidence was an echo.

AccessTokenSource gained WorkloadIdentityToken. Both existing arms were operator-shaped, so every typed gcp.SecretManager effect was unreachable from a workflow run and CI reached Secret Manager through hand-written preludes instead. The token is read in-process and passed as a Secret, never interpolated into a command line, so this removes an argv exposure rather than creating an environment one.

Evidence

required-witnesses-floor executed 25 of 25 witnesses, all passing — 19 gcp_secret_access, 4 org_admin_refusal_prelude_derivation, 2 spark — each recorded standing=planned-and-passed. required-witnesses-build, heal-generated-artifacts and witnesses also pass.

v1_src_dag_parse clean at 4877 files, verified with a planted control: a deliberate parse error in the new module reddens the sweep (rc 1) and restoring it greens (rc 0), so the sweep demonstrably reaches this code.

The drift detector carries a positive control on the plant (the_same_conditioned_cell_is_not_drift_once_a_grant_matches_it) and a both-directions control that unconditioned foreign bindings are never reported.

Known defect, not yet repaired

B1 — the condition axis is not real at the provider boundary. GetSecretIamPolicy sends no options.requestedPolicyVersion, so a v1 read returns conditional bindings with the condition object omitted and the role rendered _withcond_<hash>; a function inspecting cell.condition cannot recover it. reconcile_gcp_policy also copies observed.version into a changed policy, so writing a condition yields a policy claiming version 1. The witness fixtures encode that invalid state. Repair belongs in this PR, not a separate prerequisite: the quoted external-name form query: { "options.requestedPolicyVersion": 3 } already parses (measured — only the bare dotted spelling is refused), so no parser or emitter change has been shown to be needed. What remains is a request capture proving the pair reaches the wire undistorted, the shared version construction in reconcile_gcp_policy (3 when any final cell carries a condition, else observed.version), and fixtures that stop encoding a conditioned v1 policy.

What this deliberately does not do

  • No approval is consumed before the privileged write. secret_access_ensure_for still proceeds from a supplied token to SetSecretIamPolicy. The consent lifecycle anchors on std.scoped_authorization (which already carries request, grant, scope, attempt, intent-hash, expiry and claim machinery, and names this exact Secret Manager grant as in-flight) rather than on a second mechanism.
  • Least privilege is not yet structural. SecretAccessGrant.role is an open String; the production wrappers request accessor only, but nothing prevents a broader role reaching the write. The restriction belongs on the admitted domain request.
  • The refusal arms are declared and rendered, not constructed. Nothing on the CI path builds a value of the coproduct — this is message centralization, not runtime typed-event production. The module annotation says so explicitly.

gunbc-ci-auto-heal and others added 5 commits September 4, 2026 23:06
…nt enrolls the slot grammar, diverged is not unrelated, and refusals name their subject

Four climbs measured against today's fleet, each at its authority:

- Slot provenance carries a registration KIND, not a Bool. srv2-11 and srv2-12 sat refused for
  months because a `.runner` file was present; the file says Ephemeral=True and GitHub unregisters
  an ephemeral runner after one job (extdeps.github.actions_runner
  actions_runner_registration_decode, cited), so a dead ephemeral registration is a retired
  incarnation and needs no org credential to retire. A persistent registration still refuses.
- The retired-tree removal grant enrolls the host's slot grammar up to a declared index bound (64)
  rather than the current desired width, still one exact path per sudoers line with no wildcard, so
  a narrowed host no longer refuses at apply the removals a width change created. The bound is
  pinned above every committed width by witness.
- DivergedHistories is its own arm of DeployRevisionRelation. The srv1 deploy refused with "share no
  ancestry" for a sibling with a merge base two commits back; the neither-ancestor case is now named
  as diverged, the no-common-ancestor case stays unrelated, and the two-probe fold declares it cannot
  tell them apart instead of picking one.
- The plan artifact write refusal names which path refused and why. The srv2 run refused bare on a
  /tmp/fleet-converge-plan owned by another principal from a local run; the shared literal dir is
  the underlying defect and is declared here, not solved.
- The fleet-converge job roster witness counts the four jobs main actually has.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FbXKcgqW4Jx7e78BPWc8ci
… no human token, no repository secret

The interim design asked the operator to mint a fine-grained PAT and paste it into
GUNBC_ORG_ADMIN_TOKEN. It was never taken and was never needed: the gunbai-ci GitHub App is
installed on the organization, its private key is in Secret Manager behind the same WIF read the
fleet key uses, and the runner installer already mints registration tokens with it.

gunbc.ci_spec gunbc_ci_org_admin_app_token_prelude follows the fleet-key lifecycle (WIF token by
env, the SecretRef's own access URL, 0600 under RUNNER_TEMP, trap before the key touches disk),
signs a ten-minute RS256 JWT with openssl, exchanges it for a one-hour installation token at the
cited endpoint (extdeps.github.org_admin_auth github_app_installation_access_token_url), classifies
anything but 201 as OrgAdminInstallationTokenRefused with the status line, wipes the key, and
exports the token into that step's environment only. The workflow step drops its
secrets.GUNBC_ORG_ADMIN_TOKEN rows and takes the WIF access token instead. The acquisition plan's
interim section is marked superseded; the witness asserts the endpoint, the secret version path,
the signing, the refusal name, and the absence of any repository secret reference.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FbXKcgqW4Jx7e78BPWc8ci
…or binding is a converge the control plane runs

The first live org_actions_observe run refused with a JSON decode traceback: Secret Manager answered
403 for ci-github-app-private-key to the fleet-cloud-convergence principal, and `curl -sSf | python3`
collapsed that into "Expecting value" -- the exact status-collapse the administrator prelude's note
records. The read now captures the HTTP code, refuses OrgAdminAppKeyUnreadable naming the version
resource, the principal, the 403 ambiguity, and the remedy, and decodes only a 200 body.

The remedy is modeled rather than a hand gcloud line: gunbc.spark.secret_access_ensure is generalized
over its target secret (secret_access_ensure_for; the spark entry is one caller), and
gunbc.fleet.org_actions_converge gains org_admin_app_key_access_converge_with_supplied_token, which
reconciles roles/secretmanager.secretAccessor on the App key for the workload principal using a
control-plane token from GUNBC_GCP_ACCESS_TOKEN_FILE. The 14 spark access rows still pass against
the generalized ensure; the dispatch witness asserts the classified read.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FbXKcgqW4Jx7e78BPWc8ci
…nd the App-key refusals are types

The reconciler that binds Secret Manager access could name only one cell shape. Its target became a
parameter when the gunbai-ci App key needed the same binding, but role stayed a constant inside the
desired-cell builder and condition was hardcoded absent -- so "which access is being asked for" was
a fact about the source, not about the request, and a human-in-the-loop grant had nowhere to say
anything else. Role and condition are now carried on a SecretAccessGrant, which is the nameable
cell: one secret, one role, one condition.

Carrying condition is what makes a time-bounded grant ownable at all. Before this the owned-cell
universe was the unconditioned cell, so a conditioned cell was foreign and preserved in silence --
a grant could outlive its request with nothing able to count it. conditioned_cells_without_grant
partitions observed conditioned cells against the granted set and RETURNS the unmatched ones.
It reports and never removes: revocation belongs to whoever owns the request set.

The module moves to gunbc.auth.gcp_secret_access. It lived under gunbc.spark because spark was its
first consumer, and once a second domain called it the path asserted an ownership that was not true
(DESIGN section 3). The spark file keeps the wrapper naming its own target, and both entries keep
their names.

OrgAdminAppKeyUnreadable and OrgAdminInstallationTokenRefused become arms of
OrgAdminCredentialRefusal. They already existed as refusal TEXT in the CI prelude -- named, located,
load-bearing, and invisible to the type system -- so calling them typed was inflation on a rung
whose evidence was an echo. The prelude's sentence is now rendered from the renderers beside the
arms, which is the half that matters: with one home for the words, the shell bytes cannot drift
from the type, and the witness executes that join rather than asserting it.

AccessTokenSource gains WorkloadIdentityToken. Both existing arms were operator-shaped, so every
typed gcp.SecretManager effect was unreachable from a workflow run and CI reached Secret Manager
through hand-written preludes instead -- one fact with two realizations, only one of which ran.
The token is read in-process and passed as a Secret, never interpolated into a command line, so
this removes an argv exposure rather than creating an environment one.

Verified: v1_src_dag_parse clean at 4877 files, with a planted parse error in the new module
confirming the sweep reaches it (RC 1 planted, RC 0 restored). Witness EXECUTION is not yet
demonstrated -- BuildBuddy runners expose no cgroup memory.max so claim_batch refuses with
HostBudgetUnreadable, and declaring GUNBC_MEMORY_BUDGET_BYTES to get past that arm is the escape
hatch DESIGN section 5 forbids. The local build died on host memory pressure. The floor run on this
PR is the first execution of these witnesses.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UBLsJSnmB8ZdDTh1RRVTe7
fleet-converge-gaps (#10485) landed on main, so the three commits this branch was stacked on are
now upstream and the diff reduces to the widening this lane owns.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UBLsJSnmB8ZdDTh1RRVTe7

# Conflicts:
#	dag/gunbc/ci/ci_spec.dag
#	dag/gunbc/fleet/org_actions_converge.dag
#	dag/gunbc/spark/secret_access_ensure.dag
@gunbai-bot
gunbai-bot Bot marked this pull request as ready for review September 5, 2026 05:05
gunbc-ci-auto-heal and others added 3 commits September 5, 2026 05:20
The merge with main resolved the conflicted hunk to this branch's grant-based wrapper and then
CLEANLY auto-merged main's trailing copy of the same func below it, calling the pre-widening
`secret_access_ensure_for(target: ...)` signature. Two definitions of one name, one of them
against an argument that no longer exists.

Caught in review, not by me, and the reason is worth recording: v1_src_dag_parse reported the
merged tree parse-clean at 4877 files, because a duplicate declaration is a RESOLUTION failure and
not a parse failure. The sweep was doing its job; I read a green from it as coverage it does not
provide. A merge-resolved file is a MIX of both sides, and resolving the marked hunks is not the
same as checking the file that results -- the unmarked auto-merge is exactly where the second copy
landed.

Swept the rest of the merge for the same class: no other duplicate fn/func/data/type declaration in
any changed file, and no remaining call site on the old `target:` signature.

The comment describing the zero-argument entry convention now sits above
spark_secret_access_converge, which is the declaration it actually describes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UBLsJSnmB8ZdDTh1RRVTe7
Picks up the #10324 world-convergence transition admissions the wave-admission roster gained on
main, so this branch's roster is the same object CI adjudicates on the merge ref.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UBLsJSnmB8ZdDTh1RRVTe7
…er's owed deletion

The required floor refused this branch at the namespace-wave-admission phase: 8 unadjudicated
deltas, every one a TargetChanged binding produced by moving the Secret Manager access ensure from
gunbc.spark.secret_access_ensure to gunbc.auth.gcp_secret_access. The leaf segments are identical
on both sides -- secret_access_ensure_for, read_supplied_access_token, SuppliedTokenReady,
SuppliedTokenUnavailable -- and only the declaring module differs, which is exactly the motion the
wall adjudicates rather than auto-admits. The wall was right to refuse and the roster is where the
answer goes, so the TWENTY-SEVENTH TRANSITION entry states what moved, why the layer changed, and
that this is one change class and not a requalification bundled with a move.

The same run reported the two gunbc#10324 host_converge_for_identity rows as CONSUMED. That entry's
own trigger named this condition and said deletion was owed by whoever next touches the roster.
This change touches it, so the THIRTY-FIFTH DISSOLUTION pays that rather than deferring it, and the
obligation ledger advances to a sixth at 64 rows.

The floor also executed the witnesses for the first time: 25 of 25 passed
(19 gcp_secret_access, 4 org_admin_refusal_prelude_derivation, 2 spark), each recorded
standing=planned-and-passed. The build and heal-generated-artifacts jobs passed on the same run.
So the widening, the drift detector with its positive control, and the identity join between the
typed refusals and the CI prelude bytes are green by execution, not by typecheck.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UBLsJSnmB8ZdDTh1RRVTe7

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed exact head 36d2a94. REQUEST_CHANGES.

The shared reconciler is useful groundwork: preserve unrelated cells, retain the observed etag, do not write an already-converged policy, refuse an unresolved member, and check the policy returned by the write. Those properties deserve to survive. They do not establish the operator's requested human-in-the-loop authorization flow. This review separates a concrete conditional-IAM defect from that acceptance gap and the remaining representation claims.

B1 — P1: the newly supported conditional grants cannot be faithfully read/written through the current IAM boundary

Locations: dag/gunbc/auth/gcp_secret_access.dag (SecretAccessGrant.condition, secret_access_ensure_for); dependencies dag/extdeps/cloud/gcp/secret_manager.dag (GetSecretIamPolicy) and dag/extdeps/cloud/gcp/iam.dag (reconcile_gcp_policy).

GetSecretIamPolicy has no requested-policy-version input/query. Reconciliation copies version: observed.version even when adding a conditioned cell. Counterexample: observe an ordinary version-1 policy, request an expiring accessor binding, reconcile; the policy passed to SetSecretIamPolicy still has version 1 and now contains a condition. Google's contract requires version 3 for operations affecting conditional bindings. Requesting the old representation also cannot establish faithful observation of existing conditions. The new policy_with_expiring_grant fixture actually constructs a conditioned policy with version 1, so the pure witnesses do not test a valid provider representation.

Repair the dependency boundary along with the new condition capability: request options.requestedPolicyVersion=3, derive a version-3 output when conditions require it, retain the existing etag discipline, and witness both first-condition v1-to-v3 promotion and round-tripping an existing conditional policy while preserving foreign cells. An unchanged unconditional policy must still take the no-write arm. This is a correctness blocker even for a preparatory refactor.

Primary contracts: https://docs.cloud.google.com/iam/docs/reference/rest/v1/Policy and https://docs.cloud.google.com/iam/docs/reference/rest/v1/GetPolicyOptions ; Secret Manager exposes these options at https://docs.cloud.google.com/secret-manager/docs/reference/rest/v1/projects.secrets/getIamPolicy .

B2 — P1 acceptance gap: the privileged write consumes no human authorization evidence

Location: secret_access_ensure_for and its Spark/org-actions entrypoints.

The executable chain is currently grant + usable token -> get policy -> reconcile -> set policy. There is no request identity, authenticated approver, approval receipt, request-revision match, or std.access policy decision consumed before the write. The modeled principal-ceiling helpers are not called by this dispatch path; their tests establish the declared role partition, not the identity/authority of the supplied token. The module's own convention-only identity qualification is accurate and should not be upgraded to an enforced guarantee.

This ungated operator path predates part of this refactor; I am not claiming the move introduced every missing safeguard. It does mean the PR does not deliver the requested authorization-question -> contextual email -> approve/deny/clarify -> resume flow. Preserve one kernel: project a typed request through std.authorization_profile into std.access; collect authenticated human evidence; re-evaluate at the actual privileged effect boundary. Bind evidence to the exact beneficiary, action, resource, bounded lifetime, workflow/plan identity and request revision. Missing, denied, clarification-pending, expired, replayed or mismatched evidence must produce no IAM mutation. A successful notification or a caller-supplied bare Permit is not that evidence.

At this head dag/std/access.dag exists, but dag/std/auth.dag does not; the existing companion is dag/std/authorization_profile.dag. The requested std.auth work should compose authentication/challenge/response evidence with that kernel, not introduce another authorization decision engine.

B3 — P1 acceptance gap/new widening: arbitrary role and ambient beneficiary are not minimum-access admission

Locations: SecretAccessGrant, secret_access_desired_cells, grant_member_resolution, secret_accessor_grant, and a_grant_naming_a_different_role_produces_a_cell_for_that_role.

The former fixed accessor role is now an unchecked String copied into the policy. The new test deliberately proves that roles/secretmanager.admin reaches the desired cell. With a token authorized to edit that secret's policy, this generalized function has no local check preventing that grant to the fleet workload principal. The two current production wrappers still select accessor: this is not a claim that they currently request admin. The issue is that the new general grant surface has no enforced minimum-privilege contract before it can accept requests.

Likewise, the beneficiary is resolved from global fleet standing rather than carried and matched to the approved request, and the helper produces an unconditional grant. A resource-scoped binding can still be too broad in operations, beneficiary or duration.

Keep a generic provider binding representation below an admitted request, but derive/check permitted roles against the exact effect's required permissions and principal ceiling before any write. Bind the real workload identity to the approved beneficiary; do not label a shared service-account grant as run-only access unless the identity/broker boundary actually isolates that run. Make permanent service grants a separately explicit authorization rather than the fallback for an ad-hoc request. Add negative dispatch witnesses for an admin-role substitution, wrong member/resource, and missing or enlarged expiry. Merely adding an expiring binding alongside an existing unconditional grant does not bound effective access; report that overbreadth rather than silently claiming it is fixed or deleting foreign grants.

Primary scope/permission contract: https://docs.cloud.google.com/secret-manager/docs/access-control .

B4 — P2: refusal-message centralization has not made the runtime refusal typed

Locations: dag/gunbc/auth/org_admin_credential.dag, gunbc_ci_org_admin_app_token_prelude, and org_admin_refusal_prelude_derivation_witness_test.dag.

The two added refusal constructors are not constructed by the CI path. Its helpers accept strings and return strings, and the consumer still emits an echo followed by exit. The new containment tests join the prelude to those same string helpers; they do not join a runtime OrgAdminCredentialRefusal value to a consumer. In particular, editing a message helper changes both the rendered command and the expected substring, contrary to the comment claiming such an edit necessarily makes the test red.

Credit this as single-source message rendering, not a produced typed authorization event. Either narrow that claim explicitly for the preparatory change, or make the observed failure produce a structured refusal/request that the human-question workflow actually consumes and whose renderer matches the typed carrier. Transport/unknown failures must not automatically become requests for wider IAM access.

Required delivery shape for the operator's request

Use an operator-configured Workspace recipient. A send-only Gmail adapter plus authenticated Google sign-in for decisions is sufficient for the first delivery; inbox-reading and domain-wide delegation are not required by this use case. Keep provider contracts in extdeps, provider-neutral auth/access semantics in std, and routing/approver policy in gunbc configuration. Email should present the exact request and contextual reason without secret values. Approve/Deny/Clarify should open an authenticated request view; a GET, forwarded link, or email preview must never itself grant access. Approval must be a server-validated, replay-safe state transition over the immutable request. Clarification leaves the operation blocked and revised requests invalidate prior approval. Notification retries must not duplicate grants or lose pending questions. Verify the exact grant, then resume only the bound workflow; cleanup/revocation may remove only explicitly owned grants.

Review method: inspected changed code, witness sources and relevant dependency implementations; checked Google primary contracts. No local execution or live IAM mutation performed. The head-exact witnesses run 33948802693 was still in progress at final inspection; commit-message reports of prior witness passes are not substituted for that run's result. The PR body remains the generated TODO summary/test-plan template and needs to state whether it delivers the full flow or only groundwork. No merge authorization.

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Implementation-direction ruling at unchanged head 36d2a94. This clarifies review 5120220828; it does not approve the PR, close B1, or authorize IAM changes.

B1 fork: (b), as a separately reviewable prerequisite

By repairing the query boundary I mean preserving the upstream query parameter through the structured query representation, not putting it in the path to avoid a reported carrier deficit. Take (b) in that sense. A language-layer prerequisite need not be bundled into the access-grant change: isolate the smallest required repair and make this PR depend on it.

The extdeps.cloud.gcp.sts / gcp.Metadata.GetIdentityToken path really contains ?audience=...; I accept the cited production-consumer precedent. This is not a claim that (a) cannot produce valid HTTP. The distinction is that production use is not admission of a new workaround. DESIGN §§5–6 explicitly distinguish declaring a dissolution trigger from authorizing debt and require root-causing substrate obstacles. I am not granting a scaffold exception for another embedded query to bypass the reported structured-query gap.

Do not infer an emitter-only defect from the absence of dotted or quoted keys in extdeps. First use a minimal source-to-request reproducer to locate the actual unsupported stage. If an existing supported representation already preserves the exact query name and value, use it; no gratuitous grammar change is required. Otherwise repair the smallest deficient parser/carrier/lowering/emitter seam. No GCP-specific compiler case, dot-to-underscore rewrite, or silent dropping/stringification. The acceptance fact is that the real request builder produces exactly the query parameter options.requestedPolicyVersion=3 with the resource still in the path. A .query( containment test or operation mock alone does not establish that fact.

Shared reconciliation: proposed rule accepted, with explicit non-downgrade coverage

For a valid observed policy, in the existing delta-required arm:

output.version = 3 when any FINAL cell has a condition
                 otherwise observed.version

Evaluate the complete final population, including preserved foreign cells, not just desired/added cells. Retain observed.etag and the no-write arm. Your otherwise arm correctly preserves version 3 when removing the last conditional cell; do not replace it with a hard-coded version 1. This is a shared provider invariant, not a Spark-only policy rule.

Every Secret Manager policy read feeding this reconciler must request version 3 even when the desired grant is unconditional: foreign cells can be conditional. Google may legitimately return version 1 when the policy contains no conditions. Request version and returned policy version are different facts.

Precision correction to the finding discussion: Google does not simply omit conditional bindings from its documented version-1 compatibility response. It omits their condition objects and renders the affected role names with _withcond_ plus a hash. Thus your condition-based observer still cannot observe those conditions, but the regression fixture must model the actual lossy representation. Never relabel such a read as faithful merely by changing the output version to 3; re-read with the proper request option.

Required coverage: emitted request key/value fidelity; valid v1 unconditional policy plus first condition -> v3; v3 foreign conditioned cell preserved during another grant; removal of final owned conditioned cell keeps write version 3; unconditional delta preserves observed version; unchanged unconditional policy performs no setIamPolicy. Preserve etag conflict failure and unrelated bindings throughout. A local transport stub/request capture is sufficient to test the request boundary; no live IAM mutation is needed for this review.

Primary contracts:
https://docs.cloud.google.com/secret-manager/docs/reference/rest/v1/projects.secrets/getIamPolicy
https://docs.cloud.google.com/iam/docs/reference/rest/v1/GetPolicyOptions
https://docs.cloud.google.com/iam/docs/reference/rest/v1/Policy
https://docs.cloud.google.com/iam/docs/allow-policies#specifying-policy-version

Earlier type-layer escalation: anchor on std.scoped_authorization

Use the existing AuthorizationRequest<Subject>, OperatorGrant<Subject>, scoped-consent validation and claim lifecycle. Do not extend gunbc.auth.org_admin_credential.AcquisitionRequirement.ApprovalReceipt into a competing operator-grant system. My earlier implementation guidance failed to account for the existing scoped carrier; that part is corrected here.

Keep the Secret Manager capability/beneficiary/resource/access-window subject at its domain layer. Derive enforcing scopes, the intent hash, role and condition from that same immutable request. The generic scoped module deliberately does not inspect Subject, so putting the member/role in Subject is not itself admission. A caller-supplied unchanged hash is likewise not proof that a changed payload agrees. Bind the payload actually dispatched back to the canonical approved intent.

Compose, do not fork: std.auth supplies authenticated response evidence; scoped_authorization validates scoped consent and owns its claim transitions; the typed authorization profile projects the exact effect into std.access, whose AccessPolicy/decision_meet remain the final access-decision seam. Do not make two alternative permit paths or duplicate the scope/expiry comparison. Reuse existing CAS machinery but actually execute and consume its durable result before the effect: authorize permits an Unclaimed grant and a CasAttempt value is not an exclusive claim receipt.

B2 is the outstanding end-to-end acceptance requirement, not a claim that phase (a) introduced the old operator bypass or that every email phase must be forced into this diff. State the phase boundary in the currently TODO PR body. B3 remains an executable widening issue for the new arbitrary-role surface; deferring the UI does not admit that widening. B4 may be handled as an honest message-rendering scope correction, but the source annotation must stop claiming runtime typed-event production or that changing a shared renderer necessarily breaks a test derived from it. No finding is marked fixed by this direction-only comment.

The eventual notification recipient remains briansrls@gunb.ai with Approve/Deny/Clarify; this comment does not configure mail or change permissions. No local tests or live cloud writes were performed by this re-review.

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Follow-up adjudication of the author's two escalations, at exact head 36d2a94. This supplements review 5120220828; it is not approval or merge authorization. The head has not changed, so the acknowledged repairs are not yet present.

Decisions

  1. Reuse std.scoped_authorization for the operator-consent request/grant/claim lifecycle. Do NOT grow AcquisitionRequirement.ApprovalReceipt into a competing consent engine. I missed this existing authority in my initial design prescription; any instruction from me to extend that ApprovalReceipt into the authorization mechanism is superseded.
  2. Repair the canonical query representation/handling if it actually cannot represent the required external key. Do NOT add another path-embedded query on the strength of the sts.dag precedent. Establish the smallest executable reproducer first: absence of dotted/quoted examples in extdeps is not proof of a parser limitation, much less proof that an emitter change is the necessary repair. Use an existing faithful external-name representation if supported; otherwise repair the actual rejecting/lossy parser, lowering, carrier or emission boundary. The key is the literal HTTP parameter name options.requestedPolicyVersion, not a DAG namespace reference. No Google-specific special case.
  3. Yes: fix policy-version construction in the shared extdeps.cloud.gcp.iam.reconcile_gcp_policy, together with GetSecretIamPolicy's request contract. Do not duplicate version repair in BMC/Spark wrappers.

B1: exact repair contract

The precedent exists specifically in gcp.Metadata.GetIdentityToken in extdeps.cloud.gcp.sts, whose path includes ?audience=. That establishes an existing spelling, not its admission as a permanent alternative to the modeled query surface. DESIGN section 5 says a dissolution condition does not authorize creating debt; I am not granting a scaffold exception here.

One correction to the defect description: a v1 view does not necessarily omit the entire conditional binding. Google documents role names with a withcond suffix while omitting the condition itself. It is a lossy policy representation, not evidence that the binding or condition is absent. This makes the observation defect real without relying on the stronger, inaccurate omission claim.

Request representation version 3 on the actual GET; retain the observed etag on writes. Cover first-condition v1-to-v3 promotion, preservation of foreign conditional cells, and removing the LAST condition: that mutation still requires version 3 even though its resulting cell set has no condition. Do not derive the write version only from final_cells. An unchanged unconditional policy must remain AlreadyConverged/no-write. A GET asking for version 3 can legitimately return version 1 when the policy has no conditional bindings; do not turn that response into a false refusal.

Keep valid provider fixtures distinct from deliberately invalid inputs. Add a witness that observes the generated request's actual query name/value, not merely the presence of .query( in emitted Rust. The current pipeline_transport_emit_rest_shell_witness.w_rest_includes_query_params only checks that substring. Also drive a valid conditional wire response through decoding into conditioned_cells_without_grant and observe the located discrepancy. Pure fixtures alone cannot prove the read boundary exposes it.

Primary contracts: https://docs.cloud.google.com/iam/docs/troubleshooting-withcond ; https://docs.cloud.google.com/iam/docs/reference/rest/v1/GetPolicyOptions ; https://docs.cloud.google.com/iam/docs/reference/rest/v1/Policy ; https://docs.cloud.google.com/secret-manager/docs/reference/rest/v1/projects.secrets/getIamPolicy .

Consent architecture: reuse, but consume its actual guarantees

std.scoped_authorization already carries AuthorizationRequest, OperatorGrant, AuthorizationDecision, scope/attempt/intent/expiry checks, and claim lifecycle. Its documentation explicitly names the Secret Manager instance. gunbc.workflow_escalation already projects AuthorizationNotGranted into StepAwaitingApproval and other refusal causes into StepBlocked. Use those, preserving the distinction between a missing approval and a disagreement/replay/expired grant.

ApprovalReceipt in gunbc.auth.org_admin_credential is an AcquisitionRequirement arm containing approver and approved_request strings. It is credential-genealogy vocabulary, not an equivalent authorization mechanism. Where genealogy needs to record approval, reference the canonical issuance rather than adding another scope/expiry/replay implementation.

Keep std.access as the final access-policy decision seam through std.authorization_profile's effect projection. Treat scoped consent validation as evidence consumed by that policy, not a second route around it. std.auth work, where needed for the requested flow, should establish authenticated responder/challenge evidence, not duplicate requests, grants or an allow/deny engine. The actual privileged operation must consume the resulting decision for its exact intent, not accept a caller-supplied bare Permit.

Three limits of the existing module matter at the new consumer:

  • Subject is deliberately not inspected by authorize_granted; it compares supplied intent hashes and scopes. Derive the canonical intent hash and enforcing scopes from the SAME executable domain intent at issuance and consumption. Bind beneficiary, full resource identity, capability, requested access lifetime and attempt/revision. Swapping subject/member/role while retaining a copied hash must not pass at the effect boundary. Prefix scope coverage also does not by itself prove an exact-secret grant rather than project-wide authority.
  • claim_authorization returns a CasAttempt; it does not execute a durable store mutation. A pure AuthorizationPermitted is not proof of winning that claim. Require the corresponding durable committed result before dispatch; lost or undecided claims cause no write. Bind the receipt to the right slot/request/attempt. Crash recovery must re-observe/reconcile the same operation, not mint another grant or assume exactly-once external effects.
  • granted_by is a NonEmptyStr, not authentication evidence. Verify the approver and their authority separately. Also distinguish the deadline to redeem an approval from the lifetime of access being granted: one-time approval consumption does not make an IAM binding or retrieved secret one-use.

Finding dispositions and PR scope

B1 remains an immediate correctness blocker, including for groundwork.

B2 is an outstanding acceptance requirement for the end-to-end feature, not an allegation that this refactor newly introduced every existing ungated operator path. I accept the author's stated staging. I am NOT requiring Gmail delivery to be crammed into a sound preparatory PR. State that boundary in the PR and its modeled work; do not describe or close the contextual-email/approve-deny-clarify requirement as delivered. Existing privileged bootstrap access remains explicitly outside the new human-approval guarantee until that path is wired.

B3 remains a blocker on the newly generalized executable grant surface. The right repair is at the typed domain request/admission layer, not a closed enum of every Google IAM role and not an unrelated allowlist beside an unchecked String. Reuse SecretManagerEffect and secret_manager_effect_required_permissions. Keep provider role strings at the wire layer, derive the permitted binding from the admitted capability and policy, and require that admission on the route to SetSecretIamPolicy. A role that includes the requested read permission can still include excess authority; a includes-required-permission test alone does not prove minimum privilege. Test admin substitution, beneficiary/resource substitution and missing/enlarged access lifetime as zero-write outcomes. This request/admission construction can be built before the email transport.

B4 may remain message centralization in a preparatory change, but correct the current overclaims now. The annotation still says the sentence is rendered from the refusal arms and cannot drift from the type, while the helpers accept strings and never construct/match the coproduct. Declaring the variants and deriving text from shared helpers is not runtime structured-event production. The later workflow must consume a produced, located refusal/request, not parse these messages as authorization authority.

The operator-facing completion criterion remains contextual notification to briansrls@gunb.ai and authenticated Approve/Deny/Clarify over the exact request. Denial/clarification remain non-authorizing, revisions invalidate prior approval, notification retries do not duplicate authority, and only the bound workflow resumes after verification. No secret values in the notification.

Evidence/status

I independently checked the four exact-head check runs: required-witnesses-build, required-witnesses-floor, heal-generated-artifacts and witnesses all completed/success in run 33948802693. I did not rerun the reported 25 witnesses or verify the dashboard's cache. The last normalized PR snapshot returned mergeable:false at the unchanged head, so the earlier CLEAN claim should not be carried forward without refresh. The PR body still contains its autogenerated TODO summary and test plan.

Source/dependency inspection and Google contract verification only; no code push, local test execution, GCP IAM mutation or Workspace configuration performed. The existing REQUEST_CHANGES remains in force for the immediate defects described above.

…ot a typed refusal path

The annotation claimed the shell text "cannot drift from the type". It can. The renderers take
String and return String; nothing constructs OrgAdminAppKeyUnreadable or
OrgAdminInstallationTokenRefused and nothing matches on them, so renaming an arm, changing its
fields or deleting it leaves every renderer and every witness green. No value of the coproduct is
built anywhere on the CI path. The arms are declared and the message is centralized; the two are
adjacent, not joined.

That overclaim sat one paragraph after the same file warned that an echo establishes nothing a
program can act on -- the inflation it was written to catch, in the text that caught it. Found in
review, not by me.

What is actually true is narrower and now stated as such: the WORDS have one home, so the shell
text and any future reader of these arms cannot disagree about what the condition is called. The
annotation also names what would join them -- the read classifying its own outcome INTO an arm,
with the renderer taking the arm rather than its fields -- and the honest trigger, which is the
org-actions read moving off the shell prelude onto the typed gcp.SecretManager effect that the
WorkloadIdentityToken arm exists to make reachable.

No behaviour changes; this corrects a claim, which is the point.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UBLsJSnmB8ZdDTh1RRVTe7

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Scope correction and focused re-review at 262eea1, whose parent is the previously reviewed 36d2a94. This supersedes the separate-prerequisite requirement in my direction comments 5120338177 / 5120340132 insofar as it was premised on a missing query-language capability. It is not approval, B1 closure, merge authorization, or permission for live IAM writes.

Scope ruling: use the existing quoted-key candidate and prove its actual request; no separate compiler PR is required

The author's controlled parser report is decisive for its stated stage: pageSize parses; the quoted key "options.requestedPolicyVersion" parses; the bare dotted spelling refuses. That withdraws the claimed inability to express the external name at the parser. I have not independently rerun that probe, and it does not establish lowering, emission, or wire fidelity.

My review no longer requires a separate prerequisite PR merely to use this syntax. Adding the quoted query binding to GetSecretIamPolicy, exercising the real request builder, fixing shared policy-version construction, and repairing its fixtures are ordinary B1 work within #10514. A focused commit is useful for review; a separate PR is not a correctness requirement. The transport check is a separate proof obligation, not inherently a separate project. This removes the reviewer-imposed scope obstacle; it does not override an independent operator instruction to pause.

Target binding: query: { "options.requestedPolicyVersion": 3 }, with the existing resource path retained. Use the normal structured-query route, not a path-embedded query. No parser/emitter change is justified merely by the rejected bare spelling. If the actual request drops or transforms the accepted quoted name, locate that demonstrated failure and scope only that repair; do not pre-author a language change or a missing-capability/rung-drop claim now.

I inspected the interpreter: it collects query values using field_init_node_name_at / field_init_node_value and then calls request.query(name, val). That is relevant implementation evidence, not proof that the accepted quoted source reaches those pairs correctly. I did not establish Rust-emitter wire fidelity or execute a request capture in this review.

B1 acceptance evidence remains at the real transport boundary

Drive the modeled GetSecretIamPolicy operation through the production request-construction path into a local HTTP capture or equivalent request-builder observation. Assert GET; the intended /v1/projects//secrets/:getIamPolicy resource path; and exactly one decoded query pair whose key is options.requestedPolicyVersion and whose value is 3. There must be no quote characters in the external name, dot-to-underscore conversion, duplicate shadow parameter, or name moved into the path. Use synthetic identifiers and a dummy token; no production credentials or live cloud mutation are needed.

A mock that returns a policy before constructing the HTTP request does not settle this boundary. Neither does manually calling the HTTP library with the desired pair. A source-to-request regression must exercise the model being changed. Deleting or changing the parameter should make that request assertion fail. Capture the execution route this feature actually uses; an interpreter-only receipt must not be called evidence for generated Rust, or vice versa. Keep the evidence scoped to whichever path was executed.

Then feed a valid v3 conditional response through the actual decoder and into the condition observer. This checks the other half of B1: a faithful request alone does not establish that the condition reaches conditioned_cells_without_grant. No general query-framework redesign is being requested.

Correction to my version wording: the author's exact proposed rule already handles final-condition removal

For a valid, faithfully observed policy, in the delta-required arm:

version = 3 if any FINAL cell is conditional
          else observed.version

This rule is accepted. With observed.version == 3, removing the final condition takes the otherwise arm and still writes 3. My warning about a final-state-only downgrade applies to an else-1 rule, not to this proposed else-observed.version rule. No extra scan of observed conditions is required merely to fix a counterexample this rule already excludes.

Preserve foreign final cells and observed.etag. Retain first-condition v1 -> v3 promotion, v3 foreign-condition preservation, final-condition removal remaining v3, unconditional-delta version preservation, and AlreadyConverged/no-write coverage. A GET requesting version 3 can validly return version 1 when there are no conditions.

Primary contracts: https://docs.cloud.google.com/secret-manager/docs/reference/rest/v1/projects.secrets/getIamPolicy ; https://docs.cloud.google.com/iam/docs/reference/rest/v1/GetPolicyOptions ; https://docs.cloud.google.com/iam/docs/reference/rest/v1/Policy .

B4: substantial correction credited, but the existing witness overclaim remains

The only code-file delta in 262eea1 is the annotation change in gunbc.auth.org_admin_credential. It now explicitly says the coproduct is not constructed/matched and that renaming an arm leaves renderers and witnesses green. Credit that correction; it is not a runtime behavior change.

B4 is not fully closed because test.claim.org_admin_refusal_prelude_derivation_witness_test still says the arms are load-bearing and cannot drift, and explicitly claims: "change either message function and the corresponding test fails." Its two prelude-inclusion tests derive the expected string from the same message helper used by the producer. A shared wording change can update both and remain green. The later comment still calls the message "rendered from the arm."

Correct those existing witness claims to shared-message inclusion coverage. A valid discriminating mutation is to stop including or independently corrupt the relevant rendered message in the prelude, not to change the shared helper used on both sides. Do not add an independently maintained copy of the message just to force such a test red. Also narrow the new module sentence about "any future reader ... cannot disagree": only actual callers sharing the helper have that property; a future consumer of an unjoined arm does not. A concise correction is sufficient; no typed runtime producer is required to close the groundwork's representation overclaim.

The PR body is now substantive and explicitly states phase (a), missing approval consumption, open-role exposure, and unrepaired B1. Credit that scope correction. Update its 'separate prerequisite' sentence to this ruling; prefer 'arms declared; messages centralized' to 'arms ... rendered.' B2 stays a later delivery obligation, B3 remains the existing immediate admission/widening issue, and the std.scoped_authorization direction is unchanged. No new architecture work is added by this comment.

Status and limits

The fresh PR snapshot returns mergeable:true at 262eea1. My previous mergeable:false observation is not a current conflict claim; I cannot establish its historical cause from these snapshots. The head is no longer 36d2a94, so old exact-head green evidence is not carried over to 262eea1. I did not independently verify a new full CI run.

No local parser run, generated client execution, request capture, source modification, merge, GCP IAM mutation, or Workspace setup was performed by this review. The standing REQUEST_CHANGES remains, narrowed as above; the quoted-key B1 investigation and repair do not need another review-imposed scope escalation.

gunbc-ci-auto-heal and others added 4 commits September 5, 2026 08:07
The header claimed "change either message function and the corresponding test fails". It does not.
Both the expected string and the prelude bytes are computed from the SAME helper, so editing the
helper's wording moves both sides together and every test in the file stays green. A check whose two
sides share their only authority cannot detect a change to that authority; what it detects is the
two sides coming apart -- the prelude ceasing to echo the rendered sentence, or corrupting it
through the shell quoting. That is real coverage and it is worth executing; it is just narrower than
the sentence claimed.

Deliberately NOT repaired by manufacturing the missing red: an independently authored expected
string would reintroduce the duplicated sentence this arrangement removed and would rot the moment
either copy moved. The honest move is to state the coverage and stop citing more.

Also narrowed the neighbouring claim in gunbc.auth.org_admin_credential. It said the words having
one home means the shell text and "any future reader of these arms" cannot disagree. Only callers of
the RENDERERS have that relationship; a consumer that matches on the coproduct instead shares no
authority with the sentence the runner prints, because nothing joins the two.

Second claim correction in this lane found by review rather than by me, same class as the first: the
overclaim was in the prose about the evidence, not in the evidence.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UBLsJSnmB8ZdDTh1RRVTe7
… not copied forward

Half of B1. reconcile_gcp_policy copied observed.version into the assembled document, so adding the
first conditional cell to an ordinary version-1 policy produced a policy that asserted version 1 and
carried a condition -- invalid by construction, and rejected by Google, which requires schema
version 3 for any setIamPolicy whose document contains a conditional binding. Nothing in the module
had an opinion about the field, so the condition axis this branch added could not have been written
successfully even once.

The rule is: 3 when any FINAL cell carries a condition, else observed.version.

Both halves are load-bearing. The population is final_cells, which includes PRESERVED FOREIGN cells,
because a foreign conditional binding makes the document conditional even when the cell we author
carries none -- a desired-only scan would write version 1 over someone else's condition. And the
otherwise arm is observed.version rather than a literal 1, which is what makes removal of the last
conditional binding correct without a second scan: that removal is still an operation on a
conditional policy and must stay at version 3.

The fix is in the shared reconciler rather than in its callers. Its shared use by the BMC and spark
lanes is the reason to fix it there -- policy reconstruction already lives in that function beside
the etag it preserves, and duplicating the version rule per caller is the fork the module's own
header argues against.

FIXTURE CORRECTION IN THE SAME DIFF: policy_with_expiring_grant declared version 1 while carrying a
condition, so every test reading it asserted over a document the provider cannot issue. It is now
version 3. A fixture encoding an impossible observation proves nothing about the real round trip
however green it goes.

Four new witnesses drive the rule in both directions: first-condition promotion with the etag
preserved, a preserved FOREIGN condition holding the write at 3 while our own grant is
unconditioned, last-condition removal still writing 3, and -- the negative half, without which
"always 3" would pass everything above -- an unconditioned delta carrying observed.version through
untouched.

STILL OUTSTANDING ON B1, AND NOT CLAIMED HERE: the read. GetSecretIamPolicy still sends no
options.requestedPolicyVersion, so a v1 read returns conditional bindings with the condition object
omitted and the role rendered _withcond_<hash>. Until that is repaired and its request capture
reviewed, conditioned_cells_without_grant cannot observe a real conditioned cell, and a correct
write version does not repair an already lossy read.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UBLsJSnmB8ZdDTh1RRVTe7
…ing no ordinal

Main moved 20 commits ahead and the only conflict was the wave-admission roster -- the
serialization point that file's own header predicts every merging branch must pass through. Its
recipe was followed rather than improvised: main's file taken WHOLE and this branch's delta
re-derived at ROW IDENTITY grain, because hand-editing the markers splices one cohort's label onto
another's body.

NO ORDINAL IS CLAIMED, following gunbc#10459 immediately above and for the reason it gives: the
numbered entries count against a sequence other lanes append to concurrently, so a number chosen on
a branch is wrong by the time it merges. This branch is that argument's own evidence -- it authored
a TWENTY-SEVENTH TRANSITION and main moved underneath it before the merge.

THE gunbc#10324 DISSOLUTION ENTRY IS DROPPED, NOT RENUMBERED. This branch had authored one, deleting
both host_converge_for_identity rows, adjudicated by its own required run reporting them CONSUMED
2 of 2. Main discharged that deletion first. The deletion happened ONCE, and two entries would leave
two authorities describing one event -- the same disposition this roster records for four earlier
collisions.

The eight TargetChanged rows are unchanged in content: main touched neither
gunbc.spark.secret_access_ensure nor gunbc.fleet.org_actions_converge, so the delta this branch
authors is the same one. The wall re-adjudicates it on the merge ref regardless, which is the check
that matters rather than this sentence.

Also reworded the drop note so no line begins "THIRTY-FIFTH DISSOLUTION": wrapped that way it read
as a live heading for an entry that deliberately does not exist.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UBLsJSnmB8ZdDTh1RRVTe7
…ching change

The required namespace-wave phase admitted this branch's eight rows -- 0 unadjudicated deltas, so
the re-derivation across the merge was correct -- and then refused on a different obligation: seven
CONSUMED admissions due for deletion on a roster-touching change. gunbc#10459 landed in main while
this branch was held open by the merge conflict, so its parsed-item-kind vocabulary rows now match
no delta.

All seven are deleted, with PARSED_ITEM_KIND_VOCABULARY_LABEL.

ADJUDICATED BY A RUN, NOT BY THE TRIGGER SENTENCE, which is the standard that entry set for itself:
it asked for the deletion to be decided by joining each row against main's tree on its own
(module, in_declaration, spelling, target) tuple rather than by trusting its own prose. The wall did
exactly that and reported all seven CONSUMED by identity, 7 of 7. So the evidence is the join the
entry demanded, not the sentence it wrote.

FOURTH COHORT PAID BY THIS BRANCH FOR WORK IT DID NOT DO, after gunbc#10439's six, gunbc#10300's
two, and gunbc#10324's two. The roster's own note that the toll is proportional to how long a branch
stays open is not something this branch can dispute: it has now paid on two separate touches, and
the second cohort came due only because the first merge conflict held it open long enough for
gunbc#10459 to land.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UBLsJSnmB8ZdDTh1RRVTe7
@briansrls
briansrls merged commit d1bd0b1 into main Sep 5, 2026
4 checks passed
@briansrls
briansrls deleted the session/crisp-ram-671-access-grants branch September 5, 2026 15:26
@briansrls
briansrls restored the session/crisp-ram-671-access-grants branch September 5, 2026 15:27

Copy link
Copy Markdown
Contributor

Next-PR delivery ruling after #10514 merged as d1bd0b1. Advisory implementation review, not authority for live IAM mutation. The operator's new deliverable is the working approval loop, preferably the next PR; not another independently landed model/transport layer.

Delivery scope

Ntfy first, email second. Google sign-in is NOT a prerequisite and my earlier recommendation must not be treated as one. Deliver one real consumer: blocked org-actions secret-access requirement -> durable exact request -> protected ntfy notification -> deliberate Approve/Deny -> durable decision -> admitted, serialized IAM action -> verified result -> re-drive only the bound workflow. Use std.scoped_authorization and std.durable_compare_and_set; keep std.access/authorization_profile on the actual effect path. A minimal std.auth should supply the authentication evidence this slice actually uses, not a new consent engine or a broad account system.

There is one unresolved requirement difference, not a cryptography equivalence: the quoted brief requires SIGNED links; the proposed opaque random capability is not signed. A stateful random capability can be a sound authorization method under the boundaries below, but naming it accurately does not waive the operator's signature requirement. Under the brief as quoted, implement a narrowly consumed sign/verify or MAC realization using a vetted existing crypto implementation; do not invent an algorithm or a standalone crypto project. An explicit operator amendment can select opaque capabilities instead. 'ASAP' is not itself that amendment. Both forms are transferable bearer authority unless additional recipient authentication is used; a server signature does not prove which human clicked.

Q1: B1 is not only a drift-observer issue

I do NOT accept the blanket claim that an unconditional desired cell makes the read half irrelevant. SetIamPolicy is a whole-policy read-modify-write. Another existing binding on that secret may be conditional; a v1 view omits condition objects and exposes withcond role names. Google documents an unconditional-addition counterexample that defeats an existing condition. Etag protects concurrency, not fidelity of the policy representation.

An exact all-unconditional policy can support an unconditional change using v1, but 'our desired binding is unconditional' does not establish that precondition for the current whole policy. I have not observed ci-github-app-private-key's live policy. Notification and decision persistence can work independently, but the automatic IAM write must have faithful input or refuse.

Smallest path to a WORKING next PR: include the already-supported quoted query binding { "options.requestedPolicyVersion": 3 }, exercise the real request builder with a synthetic local capture, and retain the shared reconciliation rule: final conditional cell -> 3, otherwise observed.version, preserving the etag and no-write arm. No separate compiler prerequisite is required. This also preserves v3 on removal of the last conditional cell. Do not create a v1 compatibility bypass to defer this small repair.

Separate three deadlines: link redemption, authorization-to-dispatch, and resulting IAM access. An expiring link that authorizes an unconditioned cell creates ongoing access, not expiring access. The first slice may represent explicitly approved persistent service access, clearly rendered as such; it must not substitute that for a request for temporary access. Temporary access requires an enforced conditional binding and both B1 halves. A binding to a shared service account is not run-isolated access, and expiry cannot retract already disclosed secret bytes.

Google contracts: https://docs.cloud.google.com/iam/docs/troubleshooting-withcond ; https://docs.cloud.google.com/secret-manager/docs/reference/rest/v1/projects.secrets/getIamPolicy ; https://docs.cloud.google.com/iam/docs/reference/rest/v1/Policy .

Q2: review-refusing failures and their minimal closures

  1. REQUESTER CAN SELF-APPROVE. The requesting worker must not mint/read its redemption credential, see it in a request-status response or ntfy history, write the decision/CAS store, or read the grant administrator's credential. A separate trusted broker mints and publishes; the requester receives a non-authorizing request id. Broker policy fixes the operator channel/callback origin; neither is request-controlled. The control-plane IAM credential, ntfy publisher credential and any link signing key must be available independently of the very secret being requested. An in-process typed boundary without OS/account/storage isolation does not protect against arbitrary code under the same principal.

  2. NOTIFICATION DISCLOSURE BECOMES AUTHORITY DISCLOSURE. The complete capability URL is a secret, even when it does not contain the GCP payload. Protect subscription/read access, cached history, publish responses, server/proxy logs, tracing, and request serialization. Ntfy defaults are not private merely because auth is configured: anonymous default access must be denied and the selected topic ACL verified. Identify the broker/push intermediaries trusted with the message. The source ReadBytes operation returns a String from stdout: casting to Secret after a log has captured it is too late. Secure state/outbox material is not a user-visible request record. For opaque tokens, use cryptographically strong random bytes (32 bytes is my concrete recommendation), validate actual decoded length, use canonical URL-safe encoding, and refuse entropy/decoding/storage failures. Do not use content_hash_of_value for token hashing: it returns FNV1a64Structural, not a cryptographic hash. Store a cryptographic verifier; any retained raw notification material requires private/encrypted custody. Invalid attempts do not consume the valid credential. Bound/rate-limit request and redemption traffic.

  3. GET OR PREVIEW DECIDES. GET, HEAD, OPTIONS, link preview, and page load must not approve, deny, or consume the shared decision right. Smallest portable implementation: links open a first-party confirmation view; only an explicit POST decides. No Google login required. Alternatively ntfy supports deliberate HTTP action buttons with POST and headers/body, but verify the actual operator client's support; do not replace unsupported POST with GET. Browser confirmation needs appropriate CSRF protection, escaped request text, no third-party assets, no-store and no-referrer handling. Derive URLs from trusted configuration, not Host or requester-provided redirect targets.

  4. APPROVE AND DENY BOTH WIN. Bind each credential to its action and the same immutable request revision. Separate approve/deny tokens must contend on ONE authoritative request-decision slot, not two token-used slots. Atomically transition Pending -> ApprovedWithWorkPending OR Denied, validating credential, expiry and immutable identity against the same state. One commit wins; the other is AlreadyDecided and causes no IAM effect. The store owns read-and-conditional-write; a call to pure cas_decide or a constructed CasAttempt is not durable exclusion. The module explicitly leaves wrong-slot observation binding to its realization. Protect terminal state through restart, re-notification and reissue; never delete the spent state and recreate the same request as Pending. No automatic new request id to evade Denied.

  5. DECISION AUTHORIZES DIFFERENT WORK. Load canonical server-owned intent; never accept member, resource, role, expiry or workflow replacement from the link/POST. Bind domain/instance, request revision, intended action, beneficiary, fully qualified secret, effect/role observation, access lifetime and workflow attempt. Recompute from the dispatched intent, not a client-supplied copied hash. Important concrete reuse limit: content_hash_of_value returns a noncryptographic structural fingerprint; do not use it as the sole hostile-input integrity proof. Use exact canonical payload agreement or a genuine cryptographic digest. Approval must still pass B3/principal policy through std.access at the privileged boundary; a valid bearer credential is not permission to grant admin or use arbitrary callback targets. Audit 'capability redeemed for designated operator channel', not 'Google-authenticated Brian'.

  6. ONE-TIME DECISION IS MISTAKEN FOR EXACTLY-ONCE EXTERNAL EXECUTION. Commit approval and its work obligation together; execute from durable work state. Reuse scoped authorization's claim lifecycle, but actually claim execution, including competition between duplicate deliveries with the SAME attempt identity. A same-attempt ClaimedBy value is not independent proof of exclusive ownership. Crash after decision but before IAM must leave runnable work; timeout after IAM is an unknown result requiring readback, not a fresh approval or assumed failure. Retry the same fixed binding and absolute access deadline with etag protection. Do not slide the deadline on retries. Do not claim arbitrary downstream effects are exactly-once; carry a stable resume identity into the existing workflow. Only verified operation readiness permits that workflow to continue. Store-unreadable and indeterminate effect results do not become success.

  7. EXPIRY OR DENIAL FAILS OPEN. Use broker-observed time with a defined canonical timestamp/comparison contract, never caller time or lexical comparison of arbitrary timestamp spellings. The existing grant_not_expired_at uses <=: define/test the equality boundary rather than assuming it is exclusive. Enforce at redemption and revalidate authorization before dispatch; expiry races must not produce an executable stale authorization. Missing/invalid time refuses. Deny records a terminal human decision while preserving the original located operational refusal; it cannot be represented as generic retryable absence. Notification failure, timeout, unknown HTTP failure, invalid credentials and policy uncertainty do not mint grants. Delivery retries reuse the same durable question and cannot reset expiry or reopen it. A publish acknowledgment is not approval or proof the operator saw it.

Ntfy contracts: https://docs.ntfy.sh/config/#access-control ; https://docs.ntfy.sh/publish/#action-buttons ; https://docs.ntfy.sh/publish/#message-caching ; https://docs.ntfy.sh/subscribe/web/ . Authentication/HTTP guidance: https://cheatsheetseries.owasp.org/cheatsheets/Forgot_Password_Cheat_Sheet.html ; https://cheatsheetseries.owasp.org/cheatsheets/Transaction_Authorization_Cheat_Sheet.html ; https://www.rfc-editor.org/rfc/rfc9110.html#section-9.2.1 .

B3 positive-path caveat

The reported effect-based request and excess-authority refusal are the right direction; I have not reviewed the fresh branch or executed its six witnesses. Do not accidentally make the legitimate path permanently refuse. Google's actual secretAccessor role contains resourcemanager.projects.get, resourcemanager.projects.list and secretmanager.versions.access. A literal equality between its raw permission list and the one required operation permission would reject it. Google's scope rule says permissions apply only at the bound resource or below, and only where applicable. Ground the effective-permission comparison at the exact secret scope; do not filter arbitrary extras by naming convention or fabricate a one-permission upstream role fixture. Revalidate the observed role identity and bind the actual beneficiary, not just nonemptiness.

Contracts: https://docs.cloud.google.com/secret-manager/docs/access-control ; https://docs.cloud.google.com/iam/docs/viewing-grantable-roles .

Definition of done for the next PR

A deployed, exercised vertical slice for the named consumer: the refusal creates a durable question, the real operator receives ntfy actions, the actual endpoint commits a decision, approval drives the existing admitted IAM workflow with verified readback and the bound continuation, and denial keeps it blocked. No pasted admin token/manual grant as the advertised normal path. In addition to the positive execution, exercise actual store concurrency (approve/deny and duplicate same-attempt execution), replay, wrong action/request/payload, invalid or expired token, non-mutating GET, inaccessible store, crash windows, lossy/conditional-policy preservation, and notification redaction. Deterministic stubs can cover destructive/fault cases; never test destructive failures against production. No requirement for a generalized notification bus, OAuth/Workspace, or a broad auth rewrite before this slice. Email to briansrls@gunb.ai stays the next transport, not part of the first delivery claim.

Inspection used the merge snapshot plus primary provider contracts. No live IAM policy read, cloud write, notification, deployment, local witness run or exact-head review of the unprovided follow-up branch occurred here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant