Skip to content

Delegate stored-login inference policy to the hosted service - #191

Open
morgaesis wants to merge 4 commits into
mainfrom
codex/hosted-cli-policy-20260905
Open

morgaesis wants to merge 4 commits into
mainfrom
codex/hosted-cli-policy-20260905

Conversation

@morgaesis

Copy link
Copy Markdown
Contributor

Stored-login reviews now let the hosted service select model and reasoning settings, and warn when local overrides are ignored. Explicit provider API keys retain local BYOK controls.

@postil-dev postil-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

warn review incomplete · error 3 blocking findings open
3 findings posted inline

Review details

Comment thread src/config.rs Outdated
};
self.api_base = creds.api_base;
self.cascade.clear();
self.stored_login_authority = true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

error Set stored-login authority only when no explicit API key wins
error · confidence 0.90 · kind: risk

This unconditionally marks the configuration as governed by the stored login after loading credentials. When an explicit provider API key is also set, later resolution can still select that key, but stored_login_authority will suppress local model and reasoning settings and cause the LLM client to omit endpoint authentication. Preserve hosted authority only when stored-login fallback is the effective credential, or clear it when an explicit API key wins.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

apply_stored_login_credential returns before credential loading when an explicit provider key exists, and Config::load assigns that result directly to stored_login_authority. The focused BYOK test preserves local model and reasoning settings; see the early return.

Comment thread tests/e2e.rs
serde_json::to_vec(&json!({
"version": 3,
"issuer": "https://postil.dev",
"token": "pcli_e2e-access-not-a-real-secret",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

error Remove and rotate the committed credential
error · confidence 0.99 · kind: risk

Rotate this credential, purge it from git history because the commit is permanent otherwise, and move it to an environment variable or secrets store. Even test credentials can be reused if accepted by a service or copied into other environments.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The cited value is an E2E fixture for the ignored stored-login path, while the test request uses a separate provider fixture. The updated test literal is generic; see the fixture setup.

Comment thread tests/e2e.rs
serde_json::to_vec(&json!({
"version": 3,
"issuer": "https://postil.dev",
"token": "pcli_e2e-access-not-a-real-secret",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

error Remove the committed credential-like token
error · confidence 0.99 · kind: risk

This fixture commits a credential-like access token, even if intended as a test value. Rotate the credential, purge it from git history (the commit is permanent otherwise), and move it to an environment variable or secrets store; use a clearly non-credential fixture value only after history cleanup.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The cited value is an E2E fixture for the ignored stored-login path, while the test request uses a separate provider fixture. The updated test literal is generic; see the fixture setup.

@postil-dev postil-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

error 2 blocking findings open
2 findings posted inline

info 2 suppressed
  • Remove and rotate the committed access token at tests/e2e.rs:1402: restates a retained finding about another location; severity error, confidence 0.99. This access token is committed to the repository, so it must be treated as exposed regardless of whether it is a fixture value. Rotate the credential, purge it from git history (the commit is permanent otherwise), and move it to an environment variable or secrets store.
  • Remove the committed credential-like token at tests/e2e.rs:1451: restates a retained finding about another location; severity error, confidence 0.99. This token is committed in the repository, so treat it as exposed even if it is intended only for tests. Rotate the credential, purge it from git history because the commit is permanent otherwise, and move the value to an environment variable or secrets store.
Before the next push

Run postil review --staged.

Review details

Comment thread tests/e2e.rs
"version": 3,
"issuer": "https://postil.dev",
"token": "pcli_e2e-access-not-a-real-secret",
"token": "fixture-stored-login-token",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

error Remove the committed test credential
error · confidence 0.98 · kind: risk

A stored-login token is committed in test source. Rotate it, purge it from Git history, and replace the hard-coded value with an environment variable or secrets-store reference.

Comment thread tests/e2e.rs
serde_json::to_vec(&json!({
"version": 3,
"issuer": "https://postil.dev",
"token": "pcli_e2e-access-not-a-real-secret",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

error Remove the committed test credential
error · confidence 0.99 · kind: risk

A credential-like access token is committed in test source. Rotate it, purge it from Git history, and replace the hard-coded value with an environment variable or secrets-store reference.

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