Repository navigation
Wire Conduit into the protected static-inference path - #2485
freeqaz-openai wants to merge 10 commits into
Conversation
GitHub Actions
Failure
GitHub Actions
Failure
freeqaz-openai
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| argumentsDone: boolean; | ||
| } | ||
|
|
||
| function protectedEvents( |
There was a problem hiding this comment.
this function is insane. needs to get refactored. what are the unknown values? why all the let statements? the .split("\n\n") is wild. this needs serious love
There was a problem hiding this comment.
Reworked in 8a344f168. The provider now owns release, a typed stream owns sequence/item/argument state, and framing is parsed by lines rather than splitting the payload wholesale. I also reproduced a real hole: with zero argument deltas, two contradictory completion events were accepted. They now fail, while the matching control releases the original bytes. I compared 48 actual release decisions and reran the static suite.
| @@ -15,7 +15,15 @@ import { createRequire } from "node:module"; | |||
| import { tmpdir } from "node:os"; | |||
There was a problem hiding this comment.
this file could use a dedicated PR to clean it up. Wayyyy too long. Probably could be sliced up and greatly reduced in size/scope. Ideally is made more modular. Split out helpers from test logic. Make the test setup more declarative and 'drive' a test engine that handles the heavy lifting/logic. This feels insane right now.
There was a problem hiding this comment.
Moved the provider-Harness test restructuring to standalone #2491 against main. It now checks the Deployment actually submitted and follows one revision through starting, ready, and unready; deliberately breaking either behavior fails the relevant assertion. I rehearsed the overlap and will rebase this stack after #2491 lands, keeping protected enrollment and TLS cases here.
| @@ -1,6 +1,6 @@ | |||
| import assert from "node:assert/strict"; | |||
There was a problem hiding this comment.
same for this file. way too big. needs to be sliced up. we should try to share more logic between our tests if we can. cleaner abstraction to make adding new tests more concise.
There was a problem hiding this comment.
The Codex startup family is now in standalone #2491 against main: the same 50 named scenarios, a small fixture around the real entrypoint, and a readiness assertion that fails if stale state survives until the probe. After that lands, I will rebase the four Egress-specific startup cases into this structure here.
| skip: selected ? false : "Select OCC_TEST_EGRESS_CONNECTED=1 with a prepared protected stack.", | ||
| timeout: 30 * 60_000, | ||
| }, | ||
| async (t) => { |
There was a problem hiding this comment.
this is a mega function. same test treatment here. very gnarly.
There was a problem hiding this comment.
Reworked in 8a344f168. The 30-line test body now shows the lifecycle in order, with separate ownership, revocation, rotation, hostile-response, and withdrawal phases. The meaningful distinction is explicit: a revoked grant expects zero provider sends; rejecting a hostile response expects exactly one send and no retry. Parser and control preflight passed; the installed test was correctly skipped without a prepared stack. This test originates here, unlike the independent cleanup in #2491.
# Conflicts: # apps/controller/src/drivers/compute/kubernetes/index.ts # docs/flows/docker-compose-development.md # docs/flows/openshell-sandbox-provisioning.md # docs/reference/backends.md # docs/reference/drivers/openshell-credential-gateway.md # docs/reference/settings/development.md # internal/occdev/openshell_k3d.go # internal/occdev/state.go
…teway configuration
freeqaz-openai
changed the title
freeqaz-openai
left a comment
freeqaz-openai
left a comment
There was a problem hiding this comment.
Storage prerequisite for either scope in the description: preserve released 0054_credential_source_pat and 0055_compute_revision_admission. Moving this branch’s SQL to the next filename without merging the constraint is not compatible with released data.
I checked the exact Git objects: this head (03390eed) and the migration I tested on #2486 contain the same 0054_static_egress.sql blob, 2b0809de. On PostgreSQL 18.6, a revision containing the released compute.admission field was valid before the rename-only migration and was rejected by its older admitted-snapshot constraint (23514). The forward candidate that preserved both releases and merged their validation preserved that revision and all 55 prior receipts. That is evidence about these exact SQL bytes, not a claim to have tested this full PR or its installed protected path.
For this first layer, take only the static-egress forward migration and its populated upgrade regression; leave the send-receipt migration in #2486. Choose the actual append slot at landing (0056 on the tested base, not reserved), and fail closed on divergent migration history rather than rewrite its receipts. Please keep the draft and the separate workload-release, response-binding and installed-path qualifications in the description. No bot rerun or merge requested.
freeqaz-openai
left a comment
freeqaz-openai
left a comment
There was a problem hiding this comment.
First-layer upgrade now exercised, without #2486’s send-receipt migration. Following the storage note above, I built a five-file migration candidate against main 975f7448 and verified the equivalent reconciliation applies to the published migration files at 03390eed. It is not in this branch yet.
On disposable PostgreSQL 18.6, with the repository’s actual migration commands:
- Populated released database: installed released 0054/0055, then admitted a revision containing the released
compute.admissionfield. The rename-only control failed atagent_revisions_admitted_snapshot(23514); rollback preserved the revision and ledger. The corrected candidate recognized the existing catalog, preserved the entire admitted revision and all 55 receipts, appended exactly one expected receipt, and final check reportedcompleted. - Already divergent database: installed this branch’s actual conflicting 0054 receipt separately. Both check and apply refused
MIGRATION_HISTORY_UNSUPPORTED; neither rewrote history. If that ledger needs support, it needs an intentional separate transition. - Owning regressions: fresh absent / owner-only schemas, populated released upgrade, Provider-lineage continuation, and prefix-55 rollback/retry all passed (5 cases; TAP reported 8 passed, 0 skipped). This is local migration evidence, not full-PR CI or installed Conduit qualification.
Storage-owner handoff: keep both released SQL files unchanged; append static egress with the merged admitted-snapshot constraint; update the journal, measured catalog, history classifier and populated regression together. Recheck the append slot at landing (0056 on this base, not reserved). Keep the draft and the separate protected-path acceptance in the description.
Folded into #2487. Do not land this historical checkpoint.
The active native stack is RFC #2484 followed by the complete implementation in #2487. The contracts, storage, and enforcement share types that made these intermediate review cuts unsuitable as independently checked PRs. The complete implementation retains this history and the installed qualification covers the stack tip.
First implementation layer above #2484. This draft registers the Conduit Driver, passes revision-specific wiring to Sandbox, exposes the two-phase request authorization contract, and provides connected qualification tooling. Selecting the retired protected Driver fails configuration rather than falling back to direct access.
To land the protected path: On the installed Compute → OpenShell → Agent path, show that Compute controls workload release, an execution with no allowed routes is denied, and replacing an installation cannot transfer an in-flight request or its reply. The published controller currently fails that last check in a two-recipient fixture. The acceptance test should show that the replacement causes no credential read, provider request, or response delivery; the original recipient gets only its own accepted response; and a forbidden final trailer releases no provider bytes. Observe the Agent’s socket through the end of the response, not just the supervisor’s reply stream.
A smaller change reviewers could consider: Land the wiring with protected admission deliberately unavailable. A test must show that a protected request creates no runnable revision or workload, missing protection metadata cannot become unprotected execution, an older reader refuses the protected revision, and an explicitly unprotected workload still runs. Default-off configuration alone does not establish this boundary.
Either scope also needs a real PostgreSQL upgrade:
mainalready owns0054_credential_source_pat.sql, while this branch uses0054_static_egress.sql. Preserve the applied migration onmainand exercise the upgrade from that catalog, not only a fresh install.The independent provider-Harness and Codex startup test restructuring is in #2491.