fix: certify protected clarification S4 objects - #310
Conversation
edfdf17 to
10c2154
Compare
…covery fix: recover timestamp-only legacy claims
…isher-reuse fix: reuse the production task event publisher
…d-answers fix: make protected answer batches atomic
9439644
into
codex/pr198-f1-legacy-clarification
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f55289591d
ℹ️ 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".
| const exactClarificationRoutineValues = EXACT_CLARIFICATION_ROUTINES.map((routine) => `( | ||
| ${literal(routine.identity)}, | ||
| ${literal(routine.name)}, | ||
| ${literal(routine.grantee)}::pg_catalog.regrole | ||
| )`).join(',') |
There was a problem hiding this comment.
When this script is rerun after the parent migration has completed, the newly listed clarification tables already belong to forge_s4_routines_owner, so transferComplete is true and this entire CREATE OR REPLACE path is skipped. Existing deployments therefore retain the old 70-routine finalizer and never run the new clarification signature/grant checks, allowing precisely the drift this change is intended to reject; install or version the updated finalizer independently of the ownership-transfer branch.
AGENTS.md reference: AGENTS.md:L141-L143
Useful? React with 👍 / 👎.
| where observed.routine_oid is null | ||
| or observed.proowner <> '${OWNER}'::regrole | ||
| or observed.acl_count <> 2 | ||
| or observed.owner_execute_count <> 1 | ||
| or observed.expected_execute_count <> 1 |
There was a problem hiding this comment.
If an administrator or migration step changes one of these routines to SECURITY INVOKER or resets its fixed search_path, every attribute counted here remains unchanged and finalization still succeeds. For example, an invoker-mode clarification append then accesses protected tables as forge_architect_plan_history_reader, which has no direct table privileges, so protected answer submission fails at runtime; include prosecdef and the expected proconfig in this certificate, as the existing bridge check does.
AGENTS.md reference: AGENTS.md:L141-L143
Useful? React with 👍 / 👎.
1 participant
Source Issue
Issue: #179
Summary
EXECUTE, with no PUBLIC, extra grantee, extra overload, or missing grantThis stacked child addresses PR #198 finding
PRRT_kwDOS9dWe86U9jdz/discussion_r3679390472, including the follow-up exact-signature and closed-ACL review.Exact stack
362e41939e3028d891e9d75898b9333c48b902ab(PR fix: preserve legacy Architect clarification storage #309)10c2154b33f5d4fe55597be54a47eeefa3c609d41f98c5ebb59bc3acc653a3cc3693aa0a863e9044Files
web/scripts/bootstrap-epic-172-s4-roles.tsweb/__tests__/epic-172-s4-context.test.tsweb/__tests__/epic-172-s4-postgres.test.tsValidation
npx tsc --noEmit: passedgit diff --check: passedHosted evidence
30557711845, job90922356044— success30558228863, job90924210052— success90922387523— successNo parent review thread was replied to or resolved. This PR remains draft.