feat(sdk): surface trusted cyber access status in scans - #236
feat(sdk): surface trusted cyber access status in scans#236mldangelo-oai wants to merge 2 commits into
Conversation
|
@codex review Please review the current head commit |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
![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: fec6d43b6c
ℹ️ 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".
|
|
||
| try { | ||
| const stored = JSON.parse( | ||
| await readFile(join(codexHome, "auth.json"), "utf8"), |
There was a problem hiding this comment.
When a stored API key uses the supported system-keyring backend (including a managed-device policy), there may be no auth.json to read, so this failure path returns stored_credentials without a credentialType. trustedAccessWarning consequently treats that API-key scan as a ChatGPT account and directs the user to chatgpt.com/cyber instead of the organizational enrollment form. Determine the stored authentication mode through a backend-independent runtime/status mechanism rather than relying exclusively on the file credential backend.
Useful? React with 👍 / 👎.
| result["schemaVersion"] !== 1 || | ||
| !Array.isArray(result["grants"]) || | ||
| typeof result["checkedAt"] !== "string" || | ||
| Number.isNaN(Date.parse(result["checkedAt"])) || |
There was a problem hiding this comment.
When the provider returns a malformed checkedAt such as "1" or "0", JavaScript's permissive Date.parse interprets it as a date instead of rejecting it. A result carrying status: "granted", a valid grant, and stale: false is therefore surfaced as verified even though its timestamp does not satisfy the structured status contract, producing a false Trusted Access confirmation. Validate the expected ISO/RFC 3339 representation rather than accepting every string that Date.parse can coerce.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fec6d43b6c
ℹ️ 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".
| if (!tacStatusReported) { | ||
| const tacStatus = trustedAccessStatusFromEvent(event); | ||
| if (tacStatus !== null) { | ||
| tacStatusReported = true; |
There was a problem hiding this comment.
When the first get_tac_status call fails or returns malformed/stale data, this sets tacStatusReported immediately after classifying it as unknown. If the agent retries the tool and receives a valid granted result later in the same stream, that result is ignored, so users receive a false unverified-access warning and the status callback never reports the verified grant. Allow a later verified result to supersede an unverified attempt, or defer the final notification until retries can no longer occur.
Useful? React with 👍 / 👎.
| errorOutput.write( | ||
| "codex-security: ✓ Your account has Trusted Access for Cyber.\n", | ||
| ); |
There was a problem hiding this comment.
On an interactive TTY, onScanStarted has already started a progress timer whose current line has no trailing newline. Writing the granted notice directly therefore appends it to the timer text, and the next timer refresh uses \r to overwrite part of the notice, producing garbled or hidden output. Stop the timer before emitting this line and restart it afterward, as the other mid-scan progress callbacks do.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19c6a55d83
ℹ️ 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".
| errorOutput.write( | ||
| "codex-security: ✓ Your account has Trusted Access for Cyber.\n", | ||
| ); |
There was a problem hiding this comment.
When a scan uses an environment API key (or a stored API key whose type was detected), the TAC result applies to the API organization, but this unconditional message tells the operator that their personal account has access. Branch on selectedAuthentication, as the warning path already does, so API-key scans report that the organization has Trusted Access.
Useful? React with 👍 / 👎.
| Some cybersecurity requests and protected findings require approval through | ||
| Trusted Access for Cyber. To apply or check your access, visit | ||
| [chatgpt.com/cyber](https://chatgpt.com/cyber). |
There was a problem hiding this comment.
This authentication section covers environment and stored API keys immediately above, but directs every reader to the personal ChatGPT enrollment page. API-key access is organization-scoped and the new runtime guidance correctly uses the enterprise Trusted Access form, so API-key users following this documentation are sent to the wrong application flow; document both routes and identify which credential type uses each.
Useful? React with 👍 / 👎.
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
1 participant
Summary
codex_apps/get_tac_statusresults without blocking scans or changing machine-readable output.onTrustedAccessStatusSDK callback and route ChatGPT accounts versus environment-provided or stored API keys to the appropriate enrollment guidance.Rollout
Status messages appear only when the runtime emits the trusted TAC status event. Scans continue normally when that event is unavailable.
Verification
bun test --timeout 30000 --randomize --seed 12345 --dots ./tests-ts).pnpm run types,pnpm run format, andpnpm run build.