[AI Workspace] Refactor AI Workspace setup and configuration #2680
Conversation
- Remove hardcoded encryption and JWT secret keys from the GitHub Actions workflow. - Introduce a setup script to generate required keys, credentials, and TLS certificates for the AI Workspace and Platform API. - Update Docker Compose files to utilize generated keys and certificates, ensuring no self-signed fallback is used. - Revise documentation to reflect changes in setup and configuration processes, emphasizing the use of generated credentials and certificates. - Remove references to demo mode, enforcing strict production requirements for all configurations.
Dependency Validation ResultsDependency name: github.com/knadh/koanf/providers/confmap |
|
Warning Review limit reached
Next review available in: 28 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughChangesThe PR removes demo-mode configuration and self-signed TLS fallbacks. Platform API and AI Workspace now require explicit secrets, authentication settings, and mounted certificates. Quickstart scripts provision credentials and certificates, while Compose, CI, tests, and documentation adopt the new configuration flow. AI Workspace and Platform API runtime Provisioning and deployments Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
- Introduce a new configuration template for the Platform API, detailing all configurable keys and their defaults. - Update the Makefile to copy the new configuration template into the distribution directory. - Revise README to reflect the new location of the Platform API configuration reference.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
platform-api/internal/server/server.go (1)
485-493: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftMigrate local JWT auth off shared HMAC secrets.
platform-api/internal/server/server.goandplatform-api/internal/handler/auth_login.gostill usecfg.Auth.JWT.SecretKeywith HS256/HMAC, andplatform-api/config/config.go/platform-api/config/config_test.gostill enforce a 32-byte shared-secret contract. Switch this flow to an asymmetric allowlist and rejectHS*/none.🤖 Prompt for 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. In `@platform-api/internal/server/server.go` around lines 485 - 493, Replace shared-secret JWT validation in server.go ranges 485-493 and 548-560 and the auth_login.go login flow with asymmetric JWT verification using an explicit allowed public-key list; reject HS* and none algorithms. Update config.go range 561-573 to remove the 32-byte SecretKey contract and define the asymmetric key/algorithm configuration, and update config_test.go range 103-129 to validate the new configuration and rejection rules. Ensure LocalJWTAuthMiddleware and token issuance use the same asymmetric allowlist.Source: Coding guidelines
portals/developer-portal/docs/administer/manage-organizations.md (1)
145-151: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStale JWT secret wording
This still implies the secret is auto-generated and only needs pinning for persistence. The compose config now requires
APIP_CP_AUTH_JWT_SECRET_KEYat startup, so reword this section to say the key is mandatory instead of optional.🤖 Prompt for 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. In `@portals/developer-portal/docs/administer/manage-organizations.md` around lines 145 - 151, Update the “Session persistence and scripted access” section to state that APIP_CP_AUTH_JWT_SECRET_KEY is mandatory and must be configured at startup in both services, removing the implication that the key is auto-generated or only needed to preserve sessions. Keep the existing shared-value configuration example.
🧹 Nitpick comments (2)
portals/ai-workspace/Makefile (1)
62-62: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueConsider trusting the generated certificate explicitly instead of skipping verification.
Now that
setup.shgenerates aplatform-api.crtcertificate, you can explicitly trust it usingca_fileinstead of skipping TLS verification entirely. This aligns themake bff-runlocal development workflow with the production recommendations inconfig-template.toml.💡 Proposed change to use the generated CA file
- APIP_AIW_PLATFORM_API_TLS_SKIP_VERIFY=true \ + APIP_AIW_PLATFORM_API_CA_FILE=../certs/platform-api.crt \🤖 Prompt for 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. In `@portals/ai-workspace/Makefile` at line 62, Update the make bff-run environment configuration around APIP_AIW_PLATFORM_API_TLS_SKIP_VERIFY to trust the generated platform-api.crt through the supported ca_file setting instead of disabling TLS verification, matching the production configuration guidance.portals/developer-portal/docker-compose.platform-api.yaml (1)
37-69: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSame
platform-api-certgenblock copy-pasted in four compose files, each with the same permission gap.openssl reqdoesn't restrict output-file permissions (umask-based, typically world-readable), so the generatedkey.pemends up readable by any process with volume access, and the consumer-side mount is read-write when it only needs read access.
portals/developer-portal/docker-compose.platform-api.yaml#L37-L69: addchmod 600 /certs/key.pemto the certgen command, and mountplatform-api-certs:/app/data/certs:roon theplatform-apiservice.portals/developer-portal/docker-compose.yaml#L68-L99: samechmod 600addition and:romount onplatform-api.portals/developer-portal/it/docker-compose.test.postgres.yaml#L44-L71: samechmod 600addition and:romount onplatform-api.portals/developer-portal/it/docker-compose.test.yaml#L28-L55: samechmod 600addition and:romount onplatform-api.Given it's identical in all four files, consider extracting it into one shared compose fragment (e.g. via Compose
include:) so future changes to the cert-gen logic don't need to be applied four times.🤖 Prompt for 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. In `@portals/developer-portal/docker-compose.platform-api.yaml` around lines 37 - 69, Update the platform-api-certgen command to restrict the generated key.pem permissions to 600 and make the platform-api certificate-volume mount read-only in portals/developer-portal/docker-compose.platform-api.yaml (lines 37-69), portals/developer-portal/docker-compose.yaml (lines 68-99), portals/developer-portal/it/docker-compose.test.postgres.yaml (lines 44-71), and portals/developer-portal/it/docker-compose.test.yaml (lines 28-55). Consider extracting the identical cert-generation configuration into a shared Compose fragment so future changes are centralized.
🤖 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 `@distribution/all-in-one/docker-compose.yaml`:
- Around line 82-100: Update the platform-api service’s depends_on configuration
to include platform-api-certgen with condition service_completed_successfully,
while preserving the existing postgres service_healthy dependency. Ensure
platform-api waits for certificate generation before starting.
In `@platform-api/config/config.go`:
- Around line 563-571: The validateJWTConfig function currently requires
SecretKey only when JWT authentication is enabled; update its condition to
require and validate the shared key whenever either JWT or file-based
authentication is enabled. Preserve the early return only when both
authentication modes are disabled, and keep the existing key validation and
error behavior.
In `@platform-api/internal/server/server_tls_test.go`:
- Around line 20-25: Update the TLS test setup to reuse the checked-in
cert.pem/key.pem fixture, or extract that copying logic into a shared helper,
instead of generating a new P-256 ECDSA key pair. Remove the now-unneeded
certificate-generation imports and keep the test’s temporary-directory setup and
TLS behavior unchanged.
In `@platform-api/internal/server/server.go`:
- Around line 471-477: Update the operator-facing messages near the CORS
configuration and the related HTTP configuration block to use
APIP_CP_CORS_ALLOWED_ORIGINS and APIP_CP_HTTP_ENABLED instead of the unprefixed
environment-variable names. Preserve the existing warning behavior and message
context.
- Around line 642-648: Update Start to perform all listener configuration and
buildTLSConfig TLS certificate validation before launching the HTTP server,
background jobs, or listener goroutines. Move the buildTLSConfig call and
related preflight checks ahead of the existing server startup path, preserving
current error propagation and startup behavior after validation succeeds.
In `@platform-api/README.md`:
- Around line 232-242: The configuration documentation must state that
environment variables affect TOML values only through explicit `{{ env "NAME"
}}` interpolation, not by overriding literal values via the `APIP_CP_` prefix.
Update the README text around the `APIP_CP_` and
`APIP_CONFIG_FILE_SOURCE_ALLOWLIST` descriptions, preserving the interpolation
examples and aligning the documented behavior with the loader contract.
- Around line 261-273: Update the JWT validation documentation in the README so
the stated startup requirement and local-development command describe the same
supported behavior. Choose whether APIP_CP_AUTH_JWT_SKIP_VALIDATION=true is
permitted for development or rejected unconditionally, then revise the
requirement, variable description, and example consistently so the documented
command is executable.
- Around line 304-309: Update the claim-mapping environment variable names in
the README table to use the APIP_CP_AUTH_IDP_CLAIM_MAPPINGS_* prefix, matching
docs/ai-workspace/configuration.md and the AI Workspace config template.
Preserve the existing claim defaults and descriptions while correcting each
affected identifier.
In `@portals/ai-workspace/setup.sh`:
- Around line 95-104: Update bcrypt_hash so the plaintext password is supplied
through standard input rather than the command arguments for both the local
htpasswd and docker run paths. Use the appropriate interactive/input flags and
preserve the existing bcrypt options, output extraction, and fallback error
behavior.
- Around line 110-121: Update the ADMIN_PASSWORD_HASH value generated in
setup.sh before writing the ENV_FILE so every `$` in the bcrypt hash is replaced
with `$$`, preventing Docker Compose interpolation. Keep the bcrypt_hash result
unchanged for authentication while emitting the escaped value in the generated
environment file.
---
Outside diff comments:
In `@platform-api/internal/server/server.go`:
- Around line 485-493: Replace shared-secret JWT validation in server.go ranges
485-493 and 548-560 and the auth_login.go login flow with asymmetric JWT
verification using an explicit allowed public-key list; reject HS* and none
algorithms. Update config.go range 561-573 to remove the 32-byte SecretKey
contract and define the asymmetric key/algorithm configuration, and update
config_test.go range 103-129 to validate the new configuration and rejection
rules. Ensure LocalJWTAuthMiddleware and token issuance use the same asymmetric
allowlist.
In `@portals/developer-portal/docs/administer/manage-organizations.md`:
- Around line 145-151: Update the “Session persistence and scripted access”
section to state that APIP_CP_AUTH_JWT_SECRET_KEY is mandatory and must be
configured at startup in both services, removing the implication that the key is
auto-generated or only needed to preserve sessions. Keep the existing
shared-value configuration example.
---
Nitpick comments:
In `@portals/ai-workspace/Makefile`:
- Line 62: Update the make bff-run environment configuration around
APIP_AIW_PLATFORM_API_TLS_SKIP_VERIFY to trust the generated platform-api.crt
through the supported ca_file setting instead of disabling TLS verification,
matching the production configuration guidance.
In `@portals/developer-portal/docker-compose.platform-api.yaml`:
- Around line 37-69: Update the platform-api-certgen command to restrict the
generated key.pem permissions to 600 and make the platform-api
certificate-volume mount read-only in
portals/developer-portal/docker-compose.platform-api.yaml (lines 37-69),
portals/developer-portal/docker-compose.yaml (lines 68-99),
portals/developer-portal/it/docker-compose.test.postgres.yaml (lines 44-71), and
portals/developer-portal/it/docker-compose.test.yaml (lines 28-55). Consider
extracting the identical cert-generation configuration into a shared Compose
fragment so future changes are centralized.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c15a1747-02a7-4859-b824-0558a533b0ec
⛔ Files ignored due to path filters (1)
platform-api/go.sumis excluded by!**/*.sum
📒 Files selected for processing (47)
.github/workflows/ai-workspace-pr-check.ymldistribution/all-in-one/docker-compose.yamldocs/ai-workspace/configuration.mddocs/ai-workspace/features/secrets-management.mdgateway/examples/llm-provider.yamlplatform-api/README.mdplatform-api/config/config.goplatform-api/config/config.tomlplatform-api/config/config_test.goplatform-api/go.modplatform-api/internal/integration/harness_test.goplatform-api/internal/repository/api_deployments_test.goplatform-api/internal/repository/subscription_repository.goplatform-api/internal/server/server.goplatform-api/internal/server/server_tls_test.goplatform-api/plugins/eventgateway/service/websub_api_hmac_secret.goportals/ai-workspace/.gitignoreportals/ai-workspace/Makefileportals/ai-workspace/QUICKSTART.mdportals/ai-workspace/README.mdportals/ai-workspace/bff/internal/config/config.goportals/ai-workspace/bff/internal/config/shipped_config_test.goportals/ai-workspace/bff/internal/tlsutil/tls.goportals/ai-workspace/bff/main.goportals/ai-workspace/configs/config-platform-api-template.tomlportals/ai-workspace/configs/config-platform-api.tomlportals/ai-workspace/configs/config-template.tomlportals/ai-workspace/configs/config.tomlportals/ai-workspace/docker-compose.yamlportals/ai-workspace/package.jsonportals/ai-workspace/production/README.mdportals/ai-workspace/setup.shportals/developer-portal/README.mdportals/developer-portal/configs/config-platform-api.toml.exampleportals/developer-portal/distribution/docker-compose.yamlportals/developer-portal/docker-compose.platform-api.yamlportals/developer-portal/docker-compose.yamlportals/developer-portal/docs/administer/manage-organizations.mdportals/developer-portal/docs/introduction/quick-start.mdportals/developer-portal/it/configs/config-platform-api-it.tomlportals/developer-portal/it/docker-compose.test.postgres.yamlportals/developer-portal/it/docker-compose.test.yamltests/integration-e2e/README.mdtests/integration-e2e/docker-compose.sqlite.yamltests/integration-e2e/docker-compose.sqlserver.yamltests/integration-e2e/docker-compose.yamltests/integration-e2e/platform-api-config.toml
…space setup - Change references from `keys.env` to `api-platform.env` in various configuration files and documentation to reflect the new environment variable structure. - Update Docker Compose and setup scripts to utilize the new `api-platform.env` for managing secrets and credentials. - Revise paths for TLS certificates to ensure they are correctly referenced in the new directory structure. - Remove the obsolete `.env.example` file and update the `.gitignore` to include `api-platform.env` instead of `keys.env`.
Dependency Validation ResultsDependency name: github.com/knadh/koanf/providers/confmap |
Dependency Validation ResultsDependency name: github.com/knadh/koanf/providers/confmap |
- Changed the setup script reference from `./setup.sh` to `./scripts/setup.sh` in the AI Workspace quickstart guide. - Updated instructions for rotating the admin password to use the new script path.
Dependency Validation ResultsDependency name: github.com/knadh/koanf/providers/confmap |
Dependency Validation ResultsDependency name: github.com/knadh/koanf/providers/confmap |
1 similar comment
Dependency Validation ResultsDependency name: github.com/knadh/koanf/providers/confmap |
- Added a step in the GitHub Actions workflow to export quickstart credentials as environment variables for subsequent job steps. - Updated JWT configuration in multiple TOML files to include `skip_validation` option, ensuring consistency across platform API, integration tests, and developer portal configurations.
Dependency Validation ResultsDependency name: github.com/knadh/koanf/providers/confmap |
- Removed deprecated DevPortal integration settings from `config-template.toml` and `config.toml`. - Updated `default_config.go` to set DevPortal integration as disabled by default. - Simplified `docker-compose.yaml` by removing unnecessary comments and clarifying service configurations. - Enhanced `setup.sh` to streamline the provisioning of TLS certificates and secrets, with improved documentation in `README.md`. - Adjusted AI Workspace and Platform API configuration files to reflect changes in environment variable handling and TLS settings.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
portals/ai-workspace/README.md (1)
293-300: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSet the BFF to OIDC mode in the OIDC setup.
This section enables Platform API IDP authentication but explicitly says to leave
auth_mode = "basic". Following these instructions leaves the BFF on file-based login, so the documented OIDC flow cannot work. Setauth_mode = "oidc"(orAPIP_AIW_AUTH_MODE=oidc) and describe basic mode as the alternative quickstart configuration.🤖 Prompt for 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. In `@portals/ai-workspace/README.md` around lines 293 - 300, The OIDC setup instructions leave the BFF in basic mode, preventing the documented OIDC flow. Update the `[oidc]` configuration to set `auth_mode = "oidc"` (or the equivalent `APIP_AIW_AUTH_MODE=oidc`), and describe `auth_mode = "basic"` only as the alternative file-based quickstart configuration.platform-api/config/config-template.toml (1)
129-137: 🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy liftReplace symmetric HMAC keys with asymmetric keys for JWT validation.
The configuration relies on symmetric HMAC (
secret_key) for JWT signatures. As per coding guidelines, when verifying JWT signatures, allow only asymmetric signing methods such as RSA/RSA-PSS/EdDSA and explicitly reject symmetric algorithms (HS256/HS384/HS512).
platform-api/config/config-template.toml#L129-L137: Update the[auth.jwt]section to require an asymmetric public key (e.g.,public_key_path) instead of a symmetricsecret_key, and remove HMAC references.platform-api/config/config-template.toml#L26-L27: Change the quickstart instructions to generate an asymmetric keypair (e.g., usingopenssl genpkey) rather than a random hex string for JWT auth.platform-api/config/config.toml#L49-L53: Update the shipped configuration defaults to align with asymmetric JWT validation.🤖 Prompt for 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. In `@platform-api/config/config-template.toml` around lines 129 - 137, Replace symmetric JWT HMAC configuration with asymmetric public-key validation: in platform-api/config/config-template.toml lines 129-137, update the [auth.jwt] settings to use a public key path and remove secret_key/HMAC references; in lines 26-27, change quickstart key generation to create an asymmetric keypair with openssl genpkey; and in platform-api/config/config.toml lines 49-53, align the shipped JWT defaults with the public-key configuration and restrict validation to asymmetric algorithms.Source: Coding guidelines
🤖 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 `@platform-api/config/config-template.toml`:
- Around line 331-333: Add a TOML comment immediately above private_key_path
documenting the existing RSA dependency and marking it with “TODO(pqc):
migrate,” including the applicable tracking issue if one is available. Keep the
current configuration key and explanatory comments unchanged.
In `@portals/ai-workspace/bff/internal/server/composite_handlers_test.go`:
- Around line 298-302: The authentication failure response is using a
nonstandard code/message payload. Update the corresponding handler and, if
needed, writeErrorJSON to return exactly
{"error":"unauthorized","message":"Invalid or expired credentials."}, then
revise the assertions in the affected test to validate those fields and values.
In `@portals/ai-workspace/README.md`:
- Around line 267-285: The README setup instructions currently imply that
api-platform.env is loaded into both services, exposing Platform API secrets to
the BFF. Update the docker-compose configuration and documented environment
setup so the BFF receives only APIP_AIW_* values, while the Platform API retains
its APIP_CP_* settings and secrets; preserve the required shared IDP
configuration for the service that consumes it.
- Around line 445-460: Update the TLS setup documentation around setup.sh and
docker-compose.yaml to require separate certificate/key pairs for the
platform-api and ai-workspace services, rather than sharing
resources/certificates/cert.pem and key.pem. Document the dedicated CA trust
configuration needed for each service and adjust the replacement and restart
instructions to match the isolated certificate paths and mounts.
---
Outside diff comments:
In `@platform-api/config/config-template.toml`:
- Around line 129-137: Replace symmetric JWT HMAC configuration with asymmetric
public-key validation: in platform-api/config/config-template.toml lines
129-137, update the [auth.jwt] settings to use a public key path and remove
secret_key/HMAC references; in lines 26-27, change quickstart key generation to
create an asymmetric keypair with openssl genpkey; and in
platform-api/config/config.toml lines 49-53, align the shipped JWT defaults with
the public-key configuration and restrict validation to asymmetric algorithms.
In `@portals/ai-workspace/README.md`:
- Around line 293-300: The OIDC setup instructions leave the BFF in basic mode,
preventing the documented OIDC flow. Update the `[oidc]` configuration to set
`auth_mode = "oidc"` (or the equivalent `APIP_AIW_AUTH_MODE=oidc`), and describe
`auth_mode = "basic"` only as the alternative file-based quickstart
configuration.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 14570c2f-b671-4bbc-a404-a0d32f8996dc
📒 Files selected for processing (26)
.github/workflows/ai-workspace-pr-check.ymldocs/ai-workspace/authentication/asgardeo-setup.mddocs/ai-workspace/authentication/oidc-auth.mddocs/ai-workspace/configuration.mddocs/ai-workspace/features/secrets-management.mdplatform-api/config/config-template.tomlplatform-api/config/config.tomlplatform-api/config/default_config.goportals/ai-workspace/.env.exampleportals/ai-workspace/.gitignoreportals/ai-workspace/Makefileportals/ai-workspace/QUICKSTART.mdportals/ai-workspace/README.mdportals/ai-workspace/bff/internal/config/config.goportals/ai-workspace/bff/internal/config/shipped_config_test.goportals/ai-workspace/bff/internal/server/composite_handlers_test.goportals/ai-workspace/configs/config-platform-api.tomlportals/ai-workspace/configs/config-template.tomlportals/ai-workspace/configs/config.tomlportals/ai-workspace/distribution/README.mdportals/ai-workspace/docker-compose.yamlportals/ai-workspace/production/README.mdportals/ai-workspace/setup.shportals/developer-portal/distribution/docker-compose.yamlportals/developer-portal/docker-compose.yamlportals/developer-portal/it/configs/config-platform-api-it.toml
💤 Files with no reviewable changes (1)
- portals/ai-workspace/.env.example
🚧 Files skipped from review as they are similar to previous changes (7)
- portals/ai-workspace/production/README.md
- portals/ai-workspace/bff/internal/config/shipped_config_test.go
- portals/ai-workspace/configs/config-platform-api.toml
- docs/ai-workspace/configuration.md
- portals/ai-workspace/configs/config.toml
- portals/ai-workspace/configs/config-template.toml
- portals/ai-workspace/bff/internal/config/config.go
- Removed deprecated file-based authentication settings from `config.toml`. - Enhanced `README.md` to clarify production setup for OIDC client secrets and TLS certificate usage. - Updated error messages in `composite_handlers.go` and `composite_handlers_test.go` to provide clearer feedback for unauthorized access. - Modified environment variable handling in `config-platform-api.toml` and `config-template.toml` for improved flexibility. - Ensured TLS certificate permissions are set correctly in `docker-compose` files for integration tests.
- Introduced new intercepts for GET requests to fetch server details in the Cypress tests for MCP secret management. - Ensured that the tests wait for the server details to be fetched before proceeding with tab navigation, preventing race conditions. - Updated test logic to improve reliability and accuracy in the server management workflow.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/integration-e2e/docker-compose.yaml (1)
52-59: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMake all certificate-generation init containers fail closed.
A failed
openssl reqcan be masked by a successful finalchmod, causing Compose to consider certgen successful whileplatform-apihas unusable TLS files.
tests/integration-e2e/docker-compose.yaml#L52-L59: addset -e/set -euor chain generation and permission updates with&&.tests/integration-e2e/docker-compose.sqlserver.yaml#L51-L58: apply the same fail-closed shell handling.tests/integration-e2e/docker-compose.sqlite.yaml#L12-L19: apply the same fail-closed shell handling.🤖 Prompt for 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. In `@tests/integration-e2e/docker-compose.yaml` around lines 52 - 59, Make every certificate-generation init container fail closed by enabling strict shell error handling or chaining the OpenSSL generation and chmod commands. Update the certificate-generation blocks in tests/integration-e2e/docker-compose.yaml lines 52-59, tests/integration-e2e/docker-compose.sqlserver.yaml lines 51-58, and tests/integration-e2e/docker-compose.sqlite.yaml lines 12-19 so a failed openssl req propagates a nonzero exit status and cannot be masked by chmod.
🤖 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 `@portals/ai-workspace/bff/internal/server/composite_handlers.go`:
- Line 138: Replace the authentication failure response in writeErrorJSON’s
handler path with the exact payload {"error":"unauthorized","message":"Invalid
or expired credentials."} rather than the code-based helper output. In
portals/ai-workspace/bff/internal/server/composite_handlers.go lines 138-138,
write the standardized JSON directly; in
portals/ai-workspace/bff/internal/server/composite_handlers_test.go lines
298-302, update assertions from body["code"] to body["error"] equal to
"unauthorized".
In `@portals/ai-workspace/README.md`:
- Line 463: Update the certificate replacement instructions in the README to
explicitly restart the affected services, replacing `docker compose up -d` with
`docker compose restart` or `docker compose up -d --force-recreate` so
containers reload the new certificates.
In `@tests/integration-e2e/docker-compose.yaml`:
- Around line 57-59: Restrict generated TLS private keys by changing ownership
to UID 10001 and permissions to 0600. Apply this in
tests/integration-e2e/docker-compose.yaml lines 57-59,
tests/integration-e2e/docker-compose.sqlserver.yaml lines 56-58, and
tests/integration-e2e/docker-compose.sqlite.yaml lines 17-19.
---
Outside diff comments:
In `@tests/integration-e2e/docker-compose.yaml`:
- Around line 52-59: Make every certificate-generation init container fail
closed by enabling strict shell error handling or chaining the OpenSSL
generation and chmod commands. Update the certificate-generation blocks in
tests/integration-e2e/docker-compose.yaml lines 52-59,
tests/integration-e2e/docker-compose.sqlserver.yaml lines 51-58, and
tests/integration-e2e/docker-compose.sqlite.yaml lines 12-19 so a failed openssl
req propagates a nonzero exit status and cannot be masked by chmod.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b88f8f34-471b-4c7b-9241-f50ecd01fb45
📒 Files selected for processing (13)
platform-api/README.mdplatform-api/config/config.goplatform-api/config/config.tomlplatform-api/internal/server/server.goportals/ai-workspace/README.mdportals/ai-workspace/bff/internal/server/composite_handlers.goportals/ai-workspace/bff/internal/server/composite_handlers_test.goportals/ai-workspace/configs/config-platform-api.tomlportals/ai-workspace/configs/config-template.tomlportals/ai-workspace/cypress/e2e/002-mcp-proxies/004-mcp-secret-management-update.cy.jstests/integration-e2e/docker-compose.sqlite.yamltests/integration-e2e/docker-compose.sqlserver.yamltests/integration-e2e/docker-compose.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- platform-api/internal/server/server.go
- Enhanced README.md to clarify the use of `--force-recreate` when restarting the Docker stack to ensure new certificates are loaded. - Modified `docker-compose` files for SQLite, SQL Server, and general integration tests to improve TLS certificate permissions by changing ownership and restricting access to the private key. - Added error handling with `set -e` to ensure the script exits on failure during certificate generation.
|
@CodeRabbit approve |
✅ Action performedComments resolved. Approval is disabled; enable |
|
@CodeRabbit review |
✅ Action performedReview finished.
|
|
@CodeRabbit approve |
✅ Action performedComments resolved and changes approved. |
2 participants
AI Workspace Setup Refactor —
setup.shand the New Quick StartWhat changed and why
Previously the stack could start with no preparation: the Platform API
auto-generated a self-signed TLS certificate at runtime when none was mounted,
APIP_DEMO_MODE(default true) relaxed the auth checks, the compose filesshipped with a hardcoded
admin / adminuser, and CI carried hardcodedencryption/JWT keys in the workflow file.
This commit removes all of those defaults. Nothing is auto-generated at
runtime anymore — every secret, credential, and certificate must exist before
first start, and a new one-shot
setup.shscript generates all of them.The
setup.shscript (portals/ai-workspace/setup.sh)Run once before the first
docker compose up. In the repo it sits next todocker-compose.yaml; in the distribution zip it is shipped asscripts/setup.sh, one level below the compose root (the script detects thisand
cds up, so it always writes its outputs next todocker-compose.yaml).It generates:
api-platform.envAPIP_CP_ENCRYPTION_KEY(at-rest encryption key),APIP_CP_AUTH_JWT_SECRET_KEY(JWT signing key),APIP_CP_ADMIN_USERNAME,APIP_CP_ADMIN_PASSWORD_HASH(bcrypt)resources/certificates/cert.pem/key.pemlocalhost,platform-api,ai-workspace,host.docker.internal,127.0.0.1); being self-signed, the cert also serves as the CA bundle the BFF trusts for the upstream platform-api hopBehavior details:
--forceregenerates everything(rotates keys and credentials).
--certs-onlygenerates just the TLS pairs; used bymake bff-runforlocal development.
hash lands in
api-platform.env(written withumask 177, so mode 600).admin) and password (default: a generated random 20-character string);ADMIN_USERNAME/ADMIN_PASSWORDenv vars skip the prompts and pin thecredentials (CI pins
admin/adminfor the Cypress suite).htpasswdwhen installed, otherwise falls back to thehttpd:2.4-alpineDocker image; requiresopenssl. The password is fed viastdin so it never appears in the process list.
api-platform.envandresources/certificates/are git-ignored in the repo,and
make distwrites a.gitignoreinto the distribution covering*.envand
resources/certificates/*for the same reason.The new quick start flow (
QUICKSTART.md)The only change for users versus the old flow is the extra
setup.shstep(compose loads the generated
api-platform.envitself viaenv_file); loginnow uses the generated credentials instead of
admin/admin. Certificates are self-signed, so thebrowser trust warning remains — drop a CA-issued cert into
resources/certificates/(same file names, SANs covering both hostnames) toremove it.
Supporting code changes
Platform API (
platform-api/)demoMode()helper and everyAPIP_DEMO_MODEbranch are gone. All auth checks are now unconditional:auth.jwt.skip_validation=trueis always rejected — JWT signatures arealways verified (
SkipValidationis hard-coded tofalse).cors.allowed_originsmay never contain"*"(empty means cross-origindisabled, fail-closed);
AllowCredentialsis now alwaystrue.always logs a not-for-production warning.
generateSelfSignedCertand~100 lines removed from
server.go).buildTLSConfignow just loadscert.pem/key.pemfrom the cert dir and errors with remediation guidanceif they're missing.
non-empty
usernameandpassword_hash(supplied via{{ env }}/{{ file }}tokens).AI Workspace compose (
portals/ai-workspace/docker-compose.yaml)setup.sh-generated certificates(previously commented-out optional mounts); there is no runtime fallback.
api-platform.envis loaded viaenv_filewithformat: raw— required becausethe bcrypt hash contains
$, which compose would otherwise treat asinterpolation.
platform-api cert is mounted as a CA bundle at the
[platform_api] ca_filedefault in
configs/config.toml, instead ofAPIP_AIW_PLATFORM_API_TLS_SKIP_VERIFY=true.APIP_DEMO_MODEand hardcoded-secret environment entries removed. Theservices carry no
environment:blocks at all — every value comes from theconfig TOMLs (whose defaults are the compose deployment's values) plus the
single
env_fileentry,api-platform.env. Optional overrides go there too— e.g. the
APIP_CP_AUTH_IDP_*/APIP_AIW_OIDC_*keys (includingAPIP_AIW_OIDC_CLIENT_SECRET) that switch the stack to an OIDC provider. Theold
.env.examplefile was deleted.BFF (
portals/ai-workspace/bff/)tlsutilsimplified in line with the no-fallback model — TLScert/key files are required, no self-signed generation.
make bff-runcalls./setup.sh --certs-onlyand pointsAPIP_AIW_TLS_CERT_FILE/APIP_AIW_TLS_KEY_FILEat the generated pair.CI (
.github/workflows/ai-workspace-pr-check.yml)APIP_CP_ENCRYPTION_KEY/APIP_CP_AUTH_JWT_SECRET_KEYvalues were removed from the workflow. CI now runs
ADMIN_USERNAME=admin ADMIN_PASSWORD=admin ./setup.shand starts the stackwith
docker compose up -d --wait.Other compose files
Developer Portal, distribution, all-in-one, and integration/e2e compose files
were updated consistently with the same generated-keys pattern (no hardcoded
secrets, no demo mode).
Documentation updates
QUICKSTART.md— rewritten around thesetup.sh→docker compose upflow(see above).
README.md,production/README.md,docs/ai-workspace/configuration.md,platform-api/README.md, developer-portal quick start — demo-modereferences removed; generated credentials and required certificates
documented.
configs/config-template.toml, the Platform APIreference template, and the shipped TOMLs) updated to resolve the admin
user and keys from the
api-platform.envvalues via{{ env }}tokens.platform-api/config/config-template.tomland copied into thedistribution zip by
make distasconfigs/config-platform-api-template.toml, so it cannot drift from thePlatform API's actual config surface.
Tryout