fix(openshell): generate JWT signing certs for auto-started gateway#2370
fix(openshell): generate JWT signing certs for auto-started gateway#2370jeffmaury wants to merge 2 commits into
Conversation
jeffmaury
requested review from
MarsKubeX and
benoitf
and removed request for
a team
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📜 Recent review details⏰ Context from checks skipped due to timeout. (10)
|
| Layer / File(s) | Summary |
|---|---|
Cert generation and config writing implementation packages/main/src/plugin/openshell-cli/openshell-gateway.ts, packages/main/src/plugin/openshell-cli/openshell-gateway.toml.template |
Injects Directories and Exec, generates certs, writes gateway.toml with JWT/auth settings, and conditionally includes --config in gateway startup args. |
Test suite updates for config-driven startup packages/main/src/plugin/openshell-cli/openshell-gateway.spec.ts |
Updates DI setup and assertions for --config, and adds coverage for cert generation, config writing, and startup fallback when generation fails. |
Estimated code review effort: 3 (Moderate) | ~25 minutes
Possibly related PRs
- openkaiden/kaiden#2335: Also changes
OpenshellGatewaystartup-related logic and its accompanying spec coverage. - openkaiden/kaiden#2268: Related gateway startup work around TLS/cert generation and config usage.
Suggested reviewers: benoitf, MarsKubeX
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Out of Scope Changes check | The gateway template adds an unauthenticated auth section, which is unrelated to the cert-generation fix in #2369. |
Remove the auth-section change or split it into a separate PR unless it is required for the gateway cert-generation fix. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title is concise and accurately describes the main change to generate JWT signing certs for the auto-started gateway. |
| Description check | ✅ Passed | The description matches the changes and clearly summarizes cert generation, config writing, --config startup, and fallback behavior. |
| Linked Issues check | ✅ Passed | The implementation satisfies #2369 by generating certs with the required SANs, writing gateway.toml, and starting with --config when available. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/main/src/plugin/openshell-cli/openshell-gateway.ts`:
- Around line 248-268: In writeGatewayConfig, the JWT path entries are currently
emitted as TOML basic strings, which can break on Windows because backslashes
are treated as escapes. Update the openshell.gateway.gateway_jwt block in
openshell-gateway.ts to use TOML literal strings for signing_key_path,
public_key_path, and kid_path, or otherwise ensure backslashes are properly
escaped.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 16337e18-2a83-49cb-bdc8-19d10aa0a8ea
📒 Files selected for processing (2)
packages/main/src/plugin/openshell-cli/openshell-gateway.spec.tspackages/main/src/plugin/openshell-cli/openshell-gateway.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (10)
- GitHub Check: Windows
- GitHub Check: smoke-e2e-tests (dev) / ubuntu-24.04 (ollama)
- GitHub Check: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
- GitHub Check: unit tests / macos-15
- GitHub Check: Linux
- GitHub Check: typecheck
- GitHub Check: unit tests / windows-2022
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: macOS
- GitHub Check: linter, formatters
⚠️ CI failures not shown inline (4)
GitHub Actions: fullsend / dispatch _ Route: fix(openshell): generate JWT signing certs for auto-started gateway
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1m�[0m
�[36;1mif [[ ! "$STAGE" =~ ^[a-z][a-z0-9_-]*$ ]]; then�[0m
�[36;1m echo "::error::Invalid stage name: must start with lowercase letter and contain only [a-z0-9_-]"�[0m
GitHub Actions: fullsend / 6_dispatch _ Route.txt: fix(openshell): generate JWT signing certs for auto-started gateway
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1m�[0m
�[36;1mif [[ ! "$STAGE" =~ ^[a-z][a-z0-9_-]*$ ]]; then�[0m
�[36;1m echo "::error::Invalid stage name: must start with lowercase letter and contain only [a-z0-9_-]"�[0m
GitHub Actions: fullsend / dispatch _ Route: fix(openshell): generate JWT signing certs for auto-started gateway
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mif [[ -f .fullsend/config.yaml ]]; then�[0m
�[36;1m KILL_SWITCH=$(yq '.kill_switch // false' .fullsend/config.yaml)�[0m
�[36;1m if [[ "$KILL_SWITCH" == "true" ]]; then�[0m
�[36;1m echo "::error::Kill switch is active — all agent dispatch halted"�[0m
GitHub Actions: fullsend / dispatch _ Route: fix(openshell): generate JWT signing certs for auto-started gateway
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mEVENT_PAYLOAD=$(jq -c '{�[0m
�[36;1m issue: (.issue // null | if . then {number, html_url} else null end),�[0m
�[36;1m pull_request: (.pull_request // null | if . then {number, html_url,�[0m
�[36;1m head: {ref: .head.ref, sha: .head.sha, repo: {full_name: .head.repo.full_name}},�[0m
�[36;1m base: {ref: .base.ref, repo: {full_name: .base.repo.full_name}}} else null end),�[0m
�[36;1m comment: (.comment // null | if . then {body: .body[:4096]} else null end)�[0m
�[36;1m}' "$GITHUB_EVENT_PATH") || {�[0m
�[36;1m echo "::error::Failed to extract event payload from GITHUB_EVENT_PATH"�[0m
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
/@/path aliases instead of relative paths for imports outside the current directory's module group; use relative imports only for sibling modules within the same directory
Files:
packages/main/src/plugin/openshell-cli/openshell-gateway.spec.tspackages/main/src/plugin/openshell-cli/openshell-gateway.ts
**/*.spec.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.spec.{ts,tsx,js,jsx}: Usetest()instead ofit()for test cases in Vitest unit tests
Usevi.mock(import('...'))for auto-mocking modules in unit tests; avoid manual mock factories when possible
Usevi.resetAllMocks()inbeforeEachhooks instead ofvi.clearAllMocks()for resetting mocks between tests
When an auto-mocked function or class method needs a real implementation, usevi.mocked(...)with the prototype pattern for class methods:vi.mocked(MyClass.prototype.myMethod).mockImplementation(...)
Files:
packages/main/src/plugin/openshell-cli/openshell-gateway.spec.ts
packages/main/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/main/src/**/*.{ts,tsx}: UseipcHandle()to expose handlers in the main process with naming convention<registry-name>:<action>(e.g.,container-provider-registry:listContainers)
UseapiSender.send()to send events from main process to renderer for real-time updates
Long-running operations should useTaskManager.createTask()with title and action configuration
Files:
packages/main/src/plugin/openshell-cli/openshell-gateway.spec.tspackages/main/src/plugin/openshell-cli/openshell-gateway.ts
packages/{main,renderer,preload}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Container operations must include
engineIdparameter to identify the container engine
Files:
packages/main/src/plugin/openshell-cli/openshell-gateway.spec.tspackages/main/src/plugin/openshell-cli/openshell-gateway.ts
🧠 Learnings (3)
📚 Learning: 2026-03-09T08:47:09.657Z
Learnt from: benoitf
Repo: kortex-hub/kortex PR: 1077
File: packages/main/src/plugin/skill/skill-manager.ts:80-109
Timestamp: 2026-03-09T08:47:09.657Z
Learning: In the kortex-hub/kortex repository, IPC handlers (via ipcHandle()) may be registered directly inside feature manager/service classes (e.g., SkillManager in packages/main/src/plugin/skill/skill-manager.ts) rather than exclusively in packages/main/src/plugin/index.ts. Treat this as an accepted design pattern for files under the plugin directory. Reviewers should not require centralization in index.ts; allow IPC registration proximity to the feature that owns the handler. When reviewing code, accept direct ipcHandle() registrations inside feature managers and ensure the pattern is consistently applied across similar feature-manager modules.
Applied to files:
packages/main/src/plugin/openshell-cli/openshell-gateway.spec.tspackages/main/src/plugin/openshell-cli/openshell-gateway.ts
📚 Learning: 2026-05-12T17:14:02.153Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1850
File: packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.svelte:66-70
Timestamp: 2026-05-12T17:14:02.153Z
Learning: When reviewing code that uses `AgentWorkspaceSummaryUI.runtime`, treat it as a required, non-null `string` per the `openkaiden/kdn-api` 0.12.0 schema. Therefore, code like `a.runtime.localeCompare(b.runtime)` is safe and should not trigger warnings about possible `undefined`/`null` values or suggestions to use nullish coalescing/optional chaining for `runtime` (unless the current local types still mark `runtime` as optional, indicating a schema/version mismatch).
Applied to files:
packages/main/src/plugin/openshell-cli/openshell-gateway.spec.tspackages/main/src/plugin/openshell-cli/openshell-gateway.ts
📚 Learning: 2026-06-29T13:16:53.102Z
Learnt from: benoitf
Repo: openkaiden/kaiden PR: 2296
File: extensions/container/packages/extension/src/helper/socket-finder/_socket-finder-module.ts:28-29
Timestamp: 2026-06-29T13:16:53.102Z
Learning: When reviewing imports in openkaiden/kaiden TypeScript/JavaScript files, prefer the configured `/@/` path alias instead of relative imports that would require traversing out of the current directory/module group (i.e., paths containing `..` that cross boundaries).
Do not require alias conversion for descendant-path relative imports within the socket-finder module directory—for example, in `extensions/container/packages/extension/src/helper/socket-finder/**`, imports like `./podman/podman-version-detector` and `./podman/podman-windows-finder` are acceptable and should not be flagged.
Applied to files:
packages/main/src/plugin/openshell-cli/openshell-gateway.spec.tspackages/main/src/plugin/openshell-cli/openshell-gateway.ts
🪛 OpenGrep (1.23.0)
packages/main/src/plugin/openshell-cli/openshell-gateway.ts
[ERROR] 232-242: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🔇 Additional comments (8)
packages/main/src/plugin/openshell-cli/openshell-gateway.ts (4)
58-61: LGTM!
141-150: LGTM!
227-246: The OpenGrepcommand-injection.exec-jshint on Lines 232-242 is a false positive:Exec.exec()invokesspawn(command, args, ...)with an argument array (no shell), so the SAN values and paths are not interpretable as shell. No action needed.
270-281: LGTM!packages/main/src/plugin/openshell-cli/openshell-gateway.spec.ts (4)
21-38: LGTM!
69-84: LGTM!
252-292: LGTM!
353-404: LGTM!
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The auto-started openshell-gateway binary was missing JWT signing configuration. Before spawning the server, generate certificates via `openshell-gateway generate-certs` using the internal Exec utility, write a gateway.toml config with JWT key paths, and pass `--config` to the gateway process. Falls back gracefully if cert generation fails. Fixes openkaiden#2369 Signed-off-by: Jeff MAURY <jmaury@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jeff MAURY <jmaury@redhat.com>
jeffmaury
force-pushed
the
jeffmaury/GH-2369
branch
from
5cde631 to
e4a06d2
Compare
2 participants
Summary
openshell-gatewaybinary, generate JWT signing certificates viaopenshell-gateway generate-certsusing the internalExecutilitygateway.tomlconfig file referencing the generated JWT key paths--config <path>to the gateway process so it starts with proper signing configurationFixes #2369
Test plan
--configargumentgenerate-certsgateway.tomlconfig with JWT paths--configwhen cert generation fails (graceful degradation)pnpm run typecheck:mainpassespnpm run lint:checkpasses🤖 Generated with Claude Code