Skip to content

fix(connectivity): 改接 kimi(LLM_API_KEY1)+ allowlist 折叠假绿修复(ADR-0048,#258 关联) - #93

Merged
randypanding merged 1 commit into
mainfrom
fix/llm-connectivity-kimi
Aug 24, 2026
Merged

fix(connectivity): 改接 kimi(LLM_API_KEY1)+ allowlist 折叠假绿修复(ADR-0048,#258 关联)#93
randypanding merged 1 commit into
mainfrom
fix/llm-connectivity-kimi

Conversation

@randypanding

Copy link
Copy Markdown
Contributor

动机

org secret LLM_API_KEY 已随供应商切换被删除(只剩 LLM_API_KEY1=kimi),本 workflow 连续红;且 allowlist 缺块标量被折叠为单行,harden-runner 策略从未实际生效(假绿——adversary.yml 2026-08-24 八轮实测证实 block 模式拦 git fetch 后的连带发现)。

变更

  • env:LLM_API_KEY→secrets.LLM_API_KEY1;显式 LLM_BASE_URL=vars.LLM_ENDPOINT1;默认模型→vars.MODEL1(kimi-for-coding)
  • allowlist 补 | 块标量 + egress 降 audit(block 模式在 org 内不可用,偏离记录见 adversary.yml)

测试

合并后 workflow_dispatch 实跑一次(连通性 probe 经计量 wrapper)。

Card: Cloudbird-Software/.github#279
Ref: ADR-0048(LLM 接入)

Copilot AI lite review requested due to automatic review settings August 24, 2026 01:39

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 4 minutes.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cb562c38-9f3c-4bd1-9940-fe1d67453977

📥 Commits

Reviewing files that changed from the base of the PR and between 0ee13b6 and e83a71d.

📒 Files selected for processing (1)
  • .github/workflows/llm-connectivity.yml

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix LLM connectivity workflow for Kimi secrets/vars and harden-runner allowlist parsing

🐞 Bug fix ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Switch connectivity probe to Kimi credentials and org vars (KEY1/ENDPOINT1/MODEL1).
• Fix harden-runner allowlist YAML block-scalar to prevent “policy never applied” false-green.
• Use audit egress policy to avoid blocking checkout/git fetch in this org.
Diagram

graph TD
  A["llm-connectivity job"] --> B["harden-runner"] --> C["checkout"] --> D["metering-wrapper"] --> E{{"Kimi API"}}
  F[("Org secrets/vars")] --> D
  subgraph Legend
    direction LR
    _job["Workflow step"] ~~~ _cfg[("Config store")] ~~~ _ext{{"External API"}}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Keep egress-policy=block but move harden-runner after checkout
  • ➕ Preserves strict outbound enforcement for the actual LLM probe step
  • ➕ Avoids git/checkout breakage without loosening policy for later steps
  • ➖ Does not protect the checkout step’s network behavior
  • ➖ Requires reordering steps and re-validating intent of INV-06 scope
2. Keep egress-policy=block and expand allowlist for checkout endpoints
  • ➕ Maintains strict outbound enforcement for the entire job
  • ➕ Keeps current step order
  • ➖ Needs careful enumeration (e.g., codeload.* and any other GitHub fetch endpoints)
  • ➖ Higher maintenance burden and risk of future breakage when GitHub endpoints change
3. Run probe on a self-hosted runner with enforceable egress controls
  • ➕ Can enforce true network blocking at the infrastructure level
  • ➕ Avoids reliance on action-level egress policy semantics
  • ➖ Operational overhead and runner security hardening requirements
  • ➖ Longer lead time than a workflow-only fix

Recommendation: Given the stated constraint that block mode is not usable in this org (breaks git fetch), the PR’s audit-mode choice is pragmatic for restoring signal. If the long-term goal is real egress enforcement (not just logging), consider either moving harden-runner after checkout (best low-effort strictness) or expanding the allowlist to include all checkout endpoints so block can remain enabled.

Files changed (1) +11 / -5

Other (1) +11 / -5
llm-connectivity.ymlSwitch probe to Kimi KEY1/vars and fix harden-runner allowlist/audit mode +11/-5

Switch probe to Kimi KEY1/vars and fix harden-runner allowlist/audit mode

• Updates the probe environment to use secrets.LLM_API_KEY1 plus vars.LLM_ENDPOINT1 and vars.MODEL1 defaults for the Kimi provider. Fixes the harden-runner allowed-endpoints YAML by using a proper block scalar and switches egress-policy to audit to avoid org-specific block-mode breakage during checkout.

.github/workflows/llm-connectivity.yml

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Allowlist not enforced 🐞 Bug ⛨ Security
Description
The workflow sets egress-policy: audit, which does not block outbound traffic; allowed-endpoints
is only applied when the policy is block, so the job still has unrestricted egress despite the
allowlist block-scalar fix. This can reintroduce a “looks configured but not actually enforcing”
outcome for INV-06, just in a different form (audit-only).
Code

.github/workflows/llm-connectivity.yml[R42-43]

+          egress-policy: audit
+          allowed-endpoints: |
Relevance

●● Moderate

Close allowlist precedents favor block enforcement, but this repository recently documents audit
mode as intentional due to org constraints.

PR-#31
PR-#56

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR changes the workflow to use audit mode. The harden-runner action documents that
allowed-endpoints only applies when egress-policy is block, so in audit mode the allowlist
will not be enforced.

.github/workflows/llm-connectivity.yml[36-47]
🌐 The allowed-endpoints input is described as: “Only these endpoints will be allowed if egress-policy is set to block”, while egress-policy can be audit or block.

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`step-security/harden-runner` is configured with `egress-policy: audit`, which does not enforce the endpoint allowlist. The action’s `allowed-endpoints` input only takes effect when `egress-policy` is `block`, so the workflow currently does not actually restrict outbound egress.

### Issue Context
This PR explicitly aims to fix an allowlist “假绿” (policy not effective). However, changing to audit mode means the allowlist still won’t be enforced; it will only be observed/logged.

### Fix Focus Areas
- .github/workflows/llm-connectivity.yml[36-48]

### Suggested fix
Pick one approach:
1) **Restore enforcement:** set `egress-policy: block` and expand `allowed-endpoints` to include all domains needed by `actions/checkout` and subsequent steps (or use a baseline policy), OR
2) **Preserve enforcement without breaking checkout:** move the `harden-runner` step *after* `actions/checkout` so the initial git fetch isn’t subject to the block policy, then run the rest of the job under `block` with a complete allowlist.

If audit mode is truly required by org constraints, add an explicit guardrail (e.g., parse harden-runner’s audit output and fail if any endpoint outside the allowlist is contacted), so the job can still detect drift rather than silently allowing it.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Missing vars fallback/validation 🐞 Bug ☼ Reliability
Description
LLM_BASE_URL and the model default now come solely from org vars (vars.LLM_ENDPOINT1,
vars.MODEL1), so if either is unset/empty (common on forks or misconfigured repos), scheduled runs
can fail early (empty model) or probe the wrong provider because the wrapper silently falls back to
BigModel’s default base URL. This produces misleading “connectivity red” signals that are actually
configuration errors, not provider reachability/key validity.
Code

.github/workflows/llm-connectivity.yml[R55-57]

+          LLM_API_KEY: ${{ secrets.LLM_API_KEY1 }}
+          LLM_BASE_URL: ${{ vars.LLM_ENDPOINT1 }}
+          LLM_PROBE_MODEL: ${{ inputs.model || vars.MODEL1 }}
Relevance

●● Moderate

Missing configuration fallback is plausible, but no close historical precedent establishes this
workflow’s vars contract or expected fallback behavior.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR changes the workflow to source base URL and model from vars. The wrapper requires --model
to be non-empty and will default the base URL to BigModel when LLM_BASE_URL is unset, which can
cause failures or probing the wrong endpoint when vars are missing.

.github/workflows/llm-connectivity.yml[12-18]
.github/workflows/llm-connectivity.yml[51-62]
pipeline/metering/metering-wrapper.sh[59-65]
pipeline/metering/metering-wrapper.sh[90-94]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The workflow now depends on `vars.LLM_ENDPOINT1` and `vars.MODEL1` without any fallback or explicit validation. If these vars are missing/empty:
- `LLM_PROBE_MODEL` may become empty (especially on `schedule` runs where `inputs.model` is absent), causing `metering-wrapper.sh` to exit with a parameter error.
- `LLM_BASE_URL` may be empty, and `metering-wrapper.sh` will silently default to `https://open.bigmodel.cn/api/paas/v4`, which conflicts with the intended Kimi setup and can create misleading failures.

### Issue Context
This workflow is a monitoring signal; it should fail for true connectivity/key issues, but should fail *explicitly* and *diagnostically* for missing configuration.

### Fix Focus Areas
- .github/workflows/llm-connectivity.yml[12-18]
- .github/workflows/llm-connectivity.yml[51-62]
- pipeline/metering/metering-wrapper.sh[59-96]

### Suggested fix
Add a short guard step before calling the wrapper to validate required inputs and produce clear errors:
- Fail if `vars.LLM_ENDPOINT1` is empty (so you don’t accidentally hit the wrapper’s BigModel default).
- Provide a safe default for the model, e.g. `LLM_PROBE_MODEL: ${{ inputs.model || vars.MODEL1 || 'kimi-for-coding' }}` (or whatever constant is correct for your org).

Optionally also validate that `LLM_BASE_URL`’s hostname matches the expected provider domain (and/or matches the allowlist) to prevent configuration drift.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Web pages:
  +7 more
Review mode: ⚖️ Balanced: This changes a GitHub Actions workflow’s secrets, endpoint/model selection, and harden-runner egress enforcement; despite one localized hunk, it has meaningful security and connectivity behavior risk warranting a careful single-pass review.

Grey Divider

Tip of the day
💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment on lines +42 to +43
egress-policy: audit
allowed-endpoints: |

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Allowlist not enforced 🐞 Bug ⛨ Security

The workflow sets egress-policy: audit, which does not block outbound traffic; allowed-endpoints
is only applied when the policy is block, so the job still has unrestricted egress despite the
allowlist block-scalar fix. This can reintroduce a “looks configured but not actually enforcing”
outcome for INV-06, just in a different form (audit-only).
Agent Prompt
### Issue description
`step-security/harden-runner` is configured with `egress-policy: audit`, which does not enforce the endpoint allowlist. The action’s `allowed-endpoints` input only takes effect when `egress-policy` is `block`, so the workflow currently does not actually restrict outbound egress.

### Issue Context
This PR explicitly aims to fix an allowlist “假绿” (policy not effective). However, changing to audit mode means the allowlist still won’t be enforced; it will only be observed/logged.

### Fix Focus Areas
- .github/workflows/llm-connectivity.yml[36-48]

### Suggested fix
Pick one approach:
1) **Restore enforcement:** set `egress-policy: block` and expand `allowed-endpoints` to include all domains needed by `actions/checkout` and subsequent steps (or use a baseline policy), OR
2) **Preserve enforcement without breaking checkout:** move the `harden-runner` step *after* `actions/checkout` so the initial git fetch isn’t subject to the block policy, then run the rest of the job under `block` with a complete allowlist.

If audit mode is truly required by org constraints, add an explicit guardrail (e.g., parse harden-runner’s audit output and fail if any endpoint outside the allowlist is contacted), so the job can still detect drift rather than silently allowing it.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +55 to +57
LLM_API_KEY: ${{ secrets.LLM_API_KEY1 }}
LLM_BASE_URL: ${{ vars.LLM_ENDPOINT1 }}
LLM_PROBE_MODEL: ${{ inputs.model || vars.MODEL1 }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

2. Missing vars fallback/validation 🐞 Bug ☼ Reliability

LLM_BASE_URL and the model default now come solely from org vars (vars.LLM_ENDPOINT1,
vars.MODEL1), so if either is unset/empty (common on forks or misconfigured repos), scheduled runs
can fail early (empty model) or probe the wrong provider because the wrapper silently falls back to
BigModel’s default base URL. This produces misleading “connectivity red” signals that are actually
configuration errors, not provider reachability/key validity.
Agent Prompt
### Issue description
The workflow now depends on `vars.LLM_ENDPOINT1` and `vars.MODEL1` without any fallback or explicit validation. If these vars are missing/empty:
- `LLM_PROBE_MODEL` may become empty (especially on `schedule` runs where `inputs.model` is absent), causing `metering-wrapper.sh` to exit with a parameter error.
- `LLM_BASE_URL` may be empty, and `metering-wrapper.sh` will silently default to `https://open.bigmodel.cn/api/paas/v4`, which conflicts with the intended Kimi setup and can create misleading failures.

### Issue Context
This workflow is a monitoring signal; it should fail for true connectivity/key issues, but should fail *explicitly* and *diagnostically* for missing configuration.

### Fix Focus Areas
- .github/workflows/llm-connectivity.yml[12-18]
- .github/workflows/llm-connectivity.yml[51-62]
- pipeline/metering/metering-wrapper.sh[59-96]

### Suggested fix
Add a short guard step before calling the wrapper to validate required inputs and produce clear errors:
- Fail if `vars.LLM_ENDPOINT1` is empty (so you don’t accidentally hit the wrapper’s BigModel default).
- Provide a safe default for the model, e.g. `LLM_PROBE_MODEL: ${{ inputs.model || vars.MODEL1 || 'kimi-for-coding' }}` (or whatever constant is correct for your org).

Optionally also validate that `LLM_BASE_URL`’s hostname matches the expected provider domain (and/or matches the allowlist) to prevent configuration drift.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@randypanding
randypanding merged commit 63625cb into main Aug 24, 2026
24 checks passed
@randypanding
randypanding deleted the fix/llm-connectivity-kimi branch August 24, 2026 02:48
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.

2 participants