release: v0.8.0-alpha.1 — module migration, SDK-codegen surface, security hardening#3
Conversation
…rity hardening
Consolidates the v0.8.0-alpha.1 release work for core/api. Module path
migration, new SDK-codegen interfaces, and significant security
hardening across the request boundary.
* Module path migration: forge.lthn.ai/core/api -> dappco.re/go/api.
All sibling dappco.re/go/* deps pinned to v0.8.0-alpha.1; PHP-side
dedoc/scramble bumped ^0.12 -> ^0.13.
* SDK codegen surface (Go side):
- Describable interface — providers expose human-readable metadata
- Renderable interface — renderers expose templates and hints
- StreamGroup interface — SSE endpoints declare event shapes
- Cache-Control hints integrate with RouteGroup
- IsValidMCPServerID validates server identifiers at the boundary
* Security hardening:
- SSRF guard at the doHTTPClientRequest choke point with allow-list
and unit + integration coverage
- PackageName hardened against flag-injection (G204 false-positive
annotated)
- schemaInt bound-checks uint/uint64 -> int coercion (G115)
- newChatCompletionID G404 false-positive annotated
- outbound request cookie path G124 false-positive annotated
- API key auth lookup failure path hardened
- .gitleaksignore added for one documented false-positive
* PHP test infrastructure:
- Pest testbench bootstrap signature corrected
- Stable core module release pinned
* AX-10 scaffold: tests/cli/api/ Taskfile drivers cover the CLI surface.
* Repo hygiene:
- removed previously-tracked .DS_Store under src/php/src/Website
- dropped empty .core/TODO.md
Refs: RFC-CORE-008-AGENT-EXPERIENCE.md (AX-1, AX-6, AX-10)
Co-authored-by: Athena <athena@lthn.ai>
Co-authored-by: Cerberus <cerberus@lthn.ai>
Co-authored-by: Hephaestus <hephaestus@lthn.ai>
Co-authored-by: Cladius Maximus <cladius@lthn.ai>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request introduces a new stream endpoint package for declarative SSE and WebSocket registration, adds comprehensive test suites for OpenAPI specification generation from handler metadata, implements engine configuration inspection APIs with test coverage, and refactors SDK generator tests to use standard library helpers instead of internal utilities. ChangesStream Endpoint Group Declaration and Registration
API OpenAPI Handler Metadata Flow
Engine Configuration and Utility Testing
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| "metadata.google.internal": {}, | ||
| "metadata.googleapis.com": {}, | ||
| "metadata.azure.com": {}, | ||
| "169.254.169.254": {}, // AWS / GCP / OpenStack / Azure (legacy) |
| "metadata.googleapis.com": {}, | ||
| "metadata.azure.com": {}, | ||
| "169.254.169.254": {}, // AWS / GCP / OpenStack / Azure (legacy) | ||
| "fd00:ec2::254": {}, // AWS IPv6 |
| "metadata.azure.com": {}, | ||
| "169.254.169.254": {}, // AWS / GCP / OpenStack / Azure (legacy) | ||
| "fd00:ec2::254": {}, // AWS IPv6 | ||
| "100.100.100.200": {}, // Alibaba Cloud |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
bridge.go (1)
659-695:⚠️ Potential issue | 🟠 MajorAdd bounds checks for signed 64-bit integer coercions.
The unsigned types (
uint,uint64) are guarded againstmath.MaxInt, but the signed paths (int64andjson.Number.Int64()) cast directly tointwithout validation. On 32-bit systems whereintis narrower thanint64, values outside the valid range will wrap or cause undefined behaviour. Apply the same bounds check to both signed cases before casting.Proposed fix
func schemaInt(value any) (int, bool) { switch v := value.(type) { case int: return v, true case int8: return int(v), true case int16: return int(v), true case int32: return int(v), true case int64: + if v < int64(math.MinInt) || v > int64(math.MaxInt) { + return 0, false + } return int(v), true case uint: if v > math.MaxInt { return 0, false } return int(v), true @@ case json.Number: if n, err := v.Int64(); err == nil { + if n < int64(math.MinInt) || n > int64(math.MaxInt) { + return 0, false + } return int(n), true } } return 0, false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bridge.go` around lines 659 - 695, The schemaInt function currently casts int64 and json.Number.Int64() results straight to int without validating platform bounds; update the int64 case and the json.Number branch to check that the value lies between math.MinInt and math.MaxInt before casting, returning (0,false) if out of range, so both signed 64-bit coercions are guarded the same way as the uint/uint64 branches.pkg/provider/registry_test.go (1)
1-1:⚠️ Potential issue | 🟠 MajorFix SPDX header spelling for licence scanners.
Line [1] uses
SPDX-Licence-Identifier; the expected token isSPDX-License-Identifier. This can prevent automated licence tooling from detecting the header.Suggested change
-// SPDX-Licence-Identifier: EUPL-1.2 +// SPDX-License-Identifier: EUPL-1.2As per coding guidelines,
**/*.go: Include SPDX-License-Identifier header comment// SPDX-License-Identifier: EUPL-1.2at the top of all Go files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/provider/registry_test.go` at line 1, Replace the misspelled SPDX header token "SPDX-Licence-Identifier" with the correct "SPDX-License-Identifier" at the top of the file (the existing comment line starting with "// SPDX-..."); ensure the header reads exactly "// SPDX-License-Identifier: EUPL-1.2" so Go files comply with automated license scanners and project coding guidelines.src/php/src/Api/Documentation/config.php (1)
1-3: 🛠️ Refactor suggestion | 🟠 MajorAdd strict types declaration to this PHP file.
This touched PHP file is missing
declare(strict_types=1);immediately after the opening tag.As per coding guidelines: `**/*.php`: Include `declare(strict_types=1);` as the first statement (after opening PHP tag) in every PHP file.🔧 Minimal fix
<?php + +declare(strict_types=1); /** * API Documentation Configuration🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/Documentation/config.php` around lines 1 - 3, Add the strict types declaration by inserting declare(strict_types=1); immediately after the opening <?php tag in src/php/src/Api/Documentation/config.php so it is the first statement in the file (no blank lines or comments before it); ensure any subsequent namespace or use statements remain after this declaration.src/php/src/Api/Controllers/McpApiController.php (1)
1187-1212:⚠️ Potential issue | 🟠 MajorHeader redaction is incomplete while request and result bodies stay raw.
McpApiRequest::log()still receives the full validated tool arguments and full response payload. MCP calls commonly carry credentials, prompts, and customer data, so this change still leaves a high-risk persistence path even though the headers are now sanitised.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/Controllers/McpApiController.php` around lines 1187 - 1212, logApiRequest currently passes raw validated request body and raw response to McpApiRequest::log even though headers are sanitized; update logApiRequest to sanitize/redact sensitive fields from $validated and $response before calling McpApiRequest::log. Implement or call a sanitizer (e.g., add a private sanitizePayload(array $payload): array) and apply it to requestBody and responseBody, removing or masking keys like prompt, password, token, api_key, secret, key, access_token, authorization, credentials, and any nested occurrences; keep workspace/server/tool identifiers intact and still pass sanitized headers via sanitizeHeaders($request->headers->all()) when calling McpApiRequest::log.
🟠 Major comments (23)
src/php/src/Api/Services/WebhookTemplateService.php-496-502 (1)
496-502:⚠️ Potential issue | 🟠 MajorValidation now allows
@block tags that rendering does not process.Line [501] accepts tags like
{{#if@FIRST}}, butrenderMustache()andrenderSimple()still use patterns that exclude@, so templates can pass validation and then render incorrectly.Suggested fix
- return preg_replace_callback( - '/\{\{\s*([a-zA-Z0-9_\.]+)(?:\s*\|\s*([a-zA-Z0-9_]+)(?::([^\}]+))?)?\s*\}\}/', + return preg_replace_callback( + '/\{\{\s*(@?[a-zA-Z0-9_\.]+)(?:\s*\|\s*([a-zA-Z0-9_]+)(?::([^\}]+))?)?\s*\}\}/', function ($matches) use ($context) { @@ - '/\{\{`#if`\s+([a-zA-Z0-9_\.]+)\s*\}\}(.*?)\{\{\/if\}\}/s', + '/\{\{`#if`\s+(@?[a-zA-Z0-9_\.]+)\s*\}\}(.*?)\{\{\/if\}\}/s', @@ - '/\{\{`#unless`\s+([a-zA-Z0-9_\.]+)\s*\}\}(.*?)\{\{\/unless\}\}/s', + '/\{\{`#unless`\s+(@?[a-zA-Z0-9_\.]+)\s*\}\}(.*?)\{\{\/unless\}\}/s', @@ - '/\{\{`#each`\s+([a-zA-Z0-9_\.]+)\s*\}\}(.*?)\{\{\/each\}\}/s', + '/\{\{`#each`\s+(@?[a-zA-Z0-9_\.]+)\s*\}\}(.*?)\{\{\/each\}\}/s',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/Services/WebhookTemplateService.php` around lines 496 - 502, Validation in isSupportedMustacheBlockTag allows tags with an @ (e.g. {{`#if` `@first`}}) but renderMustache() and renderSimple() still use patterns that exclude @ causing validated templates to render incorrectly; update the rendering regexes in renderMustache() and renderSimple() to accept an optional leading @ in variable/identifier patterns (mirror the allowed @ in isSupportedMustacheBlockTag) so block conditions and simple tags containing @ are matched and processed the same way as validation.src/php/src/Api/Documentation/Middleware/ProtectDocumentation.php-32-35 (1)
32-35:⚠️ Potential issue | 🟠 MajorDo not default production to public docs access
Line 32 currently makes production publicly accessible when
api-docs.access.public_environmentsis unset. That creates an unsafe default and can expose internal documentation unintentionally.🔐 Suggested change
- $publicEnvironments = $config['public_environments'] ?? ['local', 'testing', 'staging', 'production']; + $publicEnvironments = $config['public_environments'] ?? ['local', 'testing', 'staging'];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/Documentation/Middleware/ProtectDocumentation.php` around lines 32 - 35, The middleware ProtectDocumentation currently defaults $publicEnvironments to include 'production', making docs public if config is missing; change the default in the $publicEnvironments assignment (used in the middleware's handle method) to a safe set that excludes 'production' (e.g., ['local', 'testing', 'staging'] or an empty array) so production is not public unless explicitly configured via api-docs.access.public_environments; keep the rest of the in_array check ($publicEnvironments, app()->environment(), true) unchanged so behavior only permits public access when the config explicitly lists production.brotli.go-51-51 (1)
51-51:⚠️ Potential issue | 🟠 MajorHonour
Accept-Encodingquality values before enabling Brotli.
core.Contains(..., "br")can still matchbr;q=0, which explicitly means Brotli is not acceptable. That can cause incorrect content negotiation and client-side decode failures.Proposed fix
- if !core.Contains(c.Request.Header.Get("Accept-Encoding"), "br") { + if !acceptsBrotli(c.Request.Header.Get("Accept-Encoding")) { c.Next() return }func acceptsBrotli(acceptEncoding string) bool { for _, part := range strings.Split(strings.ToLower(acceptEncoding), ",") { token := strings.TrimSpace(part) if token == "br" { return true } if strings.HasPrefix(token, "br;") { // Treat q=0 as not acceptable. if strings.Contains(token, "q=0") || strings.Contains(token, "q=0.0") { return false } return true } } return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@brotli.go` at line 51, The current check using core.Contains(c.Request.Header.Get("Accept-Encoding"), "br") can incorrectly accept encodings like "br;q=0"; replace it with a small parser function (e.g., acceptsBrotli) that splits the header by commas, lowercases and trims tokens, returns true for a plain "br" or "br" with a q>0, and returns false if a token is "br" with q=0 (or q=0.0); update the Brotli enablement branch to call acceptsBrotli(c.Request.Header.Get("Accept-Encoding")) instead of core.Contains so content negotiation respects quality values.src/php/src/Api/Concerns/ResolvesWorkspace.php-48-49 (1)
48-49:⚠️ Potential issue | 🟠 MajorValidate workspace route parameters before
(int)coercion.Casting arbitrary scalar route values to
intallows non-numeric inputs (e.g."abc") to become0, which silently changes workspace resolution behaviour. Usingis_numeric()validation before casting will ensure only valid numeric strings are accepted.Suggested fix
- if (is_scalar($workspaceParam) && $workspaceParam !== '') { - return $this->findWorkspaceForUser($request, (int) $workspaceParam); + if (is_scalar($workspaceParam) && is_numeric($workspaceParam)) { + $workspaceId = (int) $workspaceParam; + if ($workspaceId > 0) { + return $this->findWorkspaceForUser($request, $workspaceId); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/Concerns/ResolvesWorkspace.php` around lines 48 - 49, The current check casts any non-empty scalar $workspaceParam to int which turns non-numeric strings into 0; update the condition to require a numeric value before casting by adding is_numeric($workspaceParam) (e.g., if (is_scalar($workspaceParam) && $workspaceParam !== '' && is_numeric($workspaceParam)) { return $this->findWorkspaceForUser($request, (int) $workspaceParam); }), so only valid numeric route params are passed to findWorkspaceForUser and non-numeric inputs fall through to the existing resolution/validation path.go.mod-131-137 (1)
131-137:⚠️ Potential issue | 🟠 MajorUpdate
replacedirectives to match the required module paths.The
requireblock referencesdappco.re/go/{i18n,inference,io,log}, but thereplaceblock still targets the legacydappco.re/go/core/*paths. This mismatch prevents local development overrides from applying to the actual required modules.Suggested change
replace ( dappco.re/go/core => ../go - dappco.re/go/core/i18n => ../go-i18n - dappco.re/go/core/inference => ../go-inference - dappco.re/go/core/io => ../go-io - dappco.re/go/core/log => ../go-log + dappco.re/go/i18n => ../go-i18n + dappco.re/go/inference => ../go-inference + dappco.re/go/io => ../go-io + dappco.re/go/log => ../go-log )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` around lines 131 - 137, The replace directives currently point to legacy module paths (dappco.re/go/core/*) and should be updated to match the modules referenced in the require block (dappco.re/go/i18n, dappco.re/go/inference, dappco.re/go/io, dappco.re/go/log); modify the replace entries so each key is dappco.re/go/<i18n|inference|io|log> mapping to the corresponding local folder (e.g., ../go-i18n, ../go-inference, ../go-io, ../go-log) so local overrides apply correctly to the modules used by the build.middleware.go-60-62 (1)
60-62:⚠️ Potential issue | 🟠 MajorUse constant-time comparison for bearer token checks.
Line 61 compares secrets with
!=. At the authentication boundary, use constant-time comparison to reduce timing leakage.🔐 Suggested patch
import ( "crypto/rand" + "crypto/subtle" "encoding/hex" "net/http" "runtime/debug" "strings" "time" @@ - parts := core.SplitN(header, " ", 2) - if len(parts) != 2 || core.Lower(parts[0]) != "bearer" || parts[1] != token { + parts := core.SplitN(header, " ", 2) + if len(parts) != 2 || core.Lower(parts[0]) != "bearer" { + c.AbortWithStatusJSON(http.StatusUnauthorized, Fail("unauthorised", "invalid bearer token")) + return + } + if len(parts[1]) != len(token) || subtle.ConstantTimeCompare([]byte(parts[1]), []byte(token)) != 1 { c.AbortWithStatusJSON(http.StatusUnauthorized, Fail("unauthorised", "invalid bearer token")) return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@middleware.go` around lines 60 - 62, The bearer token check uses plain != which leaks timing; change the validation in the block that handles parts := core.SplitN(header, " ", 2) so the secret comparison uses a constant-time comparison (e.g., crypto/subtle.ConstantTimeCompare) against token instead of parts[1] != token while keeping the existing header format checks (core.Lower(parts[0]) != "bearer", len(parts) != 2) and the same abort path (c.AbortWithStatusJSON/Fail) when the constant-time check indicates a mismatch.src/php/src/Api/RateLimit/RateLimitService.php-303-331 (1)
303-331:⚠️ Potential issue | 🟠 MajorFix native lock detection to account for Laravel's store forwarding pattern.
$this->cacheis aRepositorycontract, so checkingmethod_exists($this->cache, 'lock')is unreliable. Lock support is often exposed via the underlying store, so this branch stays false even on lock-capable backends. Additionally, line 315 should useself::LOCK_TTL_SECONDSinstead of hardcoded10.Suggested direction
+use Illuminate\Contracts\Cache\LockProvider; protected function supportsAtomicLocks(): bool { - return method_exists($this->cache, 'lock'); + return method_exists($this->cache, 'getStore') + && $this->cache->getStore() instanceof LockProvider; } protected function acquireBucketLock(string $cacheKey): mixed { if ($this->supportsAtomicLocks()) { try { - $lock = $this->cache->lock($cacheKey.':lock', 10); + $lock = $this->cache->lock($cacheKey.':lock', self::LOCK_TTL_SECONDS);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/RateLimit/RateLimitService.php` around lines 303 - 331, supportsAtomicLocks() and acquireBucketLock() should detect native lock support on the underlying cache store (not the Repository), and use the class constant for TTL; change the detection to check method_exists($this->cache->getStore(), 'lock') (or call getStore() and inspect that object) and in acquireBucketLock() pass self::LOCK_TTL_SECONDS instead of the hardcoded 10 when creating the lock; ensure the rest of the logic (checking $lock->get() and falling back to acquireAdvisoryLock) remains the same.src/php/src/Api/Controllers/Api/WorkspaceMemberController.php-61-64 (1)
61-64:⚠️ Potential issue | 🟠 MajorDon’t allow
ownerto be granted through the invite endpoint.Line 63 accepts
role=owner, but Line 108 explicitly blocks owner removal here. This endpoint can therefore create privileged, effectively sticky memberships. Ownership changes should go through a separate audited flow.Suggested fix
- 'role' => ['sometimes', 'string', 'in:owner,admin,member'], + 'role' => ['sometimes', 'string', 'in:admin,member'],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/Controllers/Api/WorkspaceMemberController.php` around lines 61 - 64, The invite endpoint currently allows role=owner in the validation array inside WorkspaceMemberController (the $request->validate call), which lets invites grant ownership; update the validation to remove 'owner' from the allowed values (use 'in:admin,member' or similar) and ensure the controller’s invite/create method (the method handling $data after validation) never assigns or elevates a user to owner—default to 'member' when role is absent and reject or normalize any incoming 'owner' value before creating the membership so ownership changes remain handled only by the audited owner transfer flow.src/php/src/Api/Middleware/RateLimitApi.php-232-235 (1)
232-235:⚠️ Potential issue | 🟠 MajorDon’t bypass the free-tier fallback for malformed workspace attributes.
Line 233 returns before
getWorkspaceTier()can downgrade a present-but-invalidworkspaceattribute to'free'. That means malformed requests now fall through to the broader authenticated/default bucket instead of the stricter tier bucket.Suggested fix
protected function getTierRateLimit(Request $request): ?array { - $workspace = $request->attributes->get('workspace'); - if (! is_object($workspace)) { + if (! $request->attributes->has('workspace')) { return null; } + + $workspace = $request->attributes->get('workspace'); $tier = $this->getWorkspaceTier($workspace); $tierConfig = config("api.rate_limits.tiers.{$tier}");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/Middleware/RateLimitApi.php` around lines 232 - 235, The early return when $workspace is not an object bypasses the later call to getWorkspaceTier(), so malformed workspace attributes never get downgraded to the 'free' tier; instead of returning null in the RateLimitApi middleware, ensure you do not short-circuit: replace the return with normalizing $workspace to null (or otherwise ensure getWorkspaceTier() is invoked with the non-object value) so that getWorkspaceTier($workspace) runs and yields the 'free' tier for malformed attributes before proceeding.src/php/src/Api/Controllers/Api/PaymentMethodController.php-99-106 (1)
99-106:⚠️ Potential issue | 🟠 MajorAvoid returning raw service exception messages to clients.
Line 104 forwards the original
RuntimeExceptionmessage across the API boundary. That can leak provider/internal details and makes the 422 payload unstable. Return a fixed client-safe message here and report the original exception server-side.Suggested fix
try { $this->service->removePaymentMethod($workspace, $paymentMethod); } catch (\RuntimeException $e) { + report($e); + return $this->errorResponse( errorCode: 'invalid_status', - message: $e->getMessage(), + message: 'The payment method cannot be removed in its current state.', status: 422, ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/Controllers/Api/PaymentMethodController.php` around lines 99 - 106, The catch block in PaymentMethodController that wraps $this->service->removePaymentMethod($workspace, $paymentMethod) should not return the raw RuntimeException message; instead log or report $e (e.g., to the app logger or error monitoring) and call errorResponse with a stable, client-safe message (e.g., "Unable to remove payment method" or "Invalid payment method status") and the same 422 status; update the catch for \RuntimeException to emit the original exception server-side and replace message: $e->getMessage() with the fixed message while keeping errorCode: 'invalid_status' and status: 422.src/php/src/Api/Documentation/Routes/docs.php-19-19 (1)
19-19:⚠️ Potential issue | 🟠 MajorRoot docs route now bypasses configured default UI.
Line 19 hard-codes
/toswagger, soapi-docs.ui.defaultandDocumentationController::index()no longer control landing behaviour.💡 Proposed fix
-Route::get('/', [DocumentationController::class, 'swagger'])->name('api.docs'); +Route::get('/', [DocumentationController::class, 'index'])->name('api.docs');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/Documentation/Routes/docs.php` at line 19, The root docs route currently hard-codes '/' to DocumentationController::swagger which bypasses the configured api-docs.ui.default and the controller's index() logic; update the route so the '/' endpoint uses DocumentationController::index (or resolves to the UI specified by config('api-docs.ui.default')) instead of directly calling swagger, ensuring DocumentationController::index() can decide whether to forward to swagger() or another UI based on the api-docs.ui.default setting.src/php/src/Api/Resources/ApiKeyResource.php-60-60 (1)
60-60:⚠️ Potential issue | 🟠 MajorPrevent possible null dereference on
updated_atserialisation.Line 60 calls
toIso8601String()directly; ifupdated_atis null this will 500 at runtime.🔧 Suggested fix
- 'updated_at' => $this->updated_at->toIso8601String(), + 'updated_at' => $this->updated_at?->toIso8601String(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/Resources/ApiKeyResource.php` at line 60, The serialization of updated_at in ApiKeyResource may call toIso8601String() on null; update the ApiKeyResource serialization (where 'updated_at' => ...) to guard against null by using a null-safe check or conditional and return null (or an empty string) when $this->updated_at is null—for example replace the direct call with a null-safe expression ($this->updated_at?->toIso8601String()) or a ternary ($this->updated_at ? $this->updated_at->toIso8601String() : null) so no null dereference occurs.src/php/src/Api/Observers/WorkspaceWebhookObserver.php-42-52 (1)
42-52:⚠️ Potential issue | 🟠 MajorMake the observer defer until after transaction commit.
The observer methods execute synchronously during the parent transaction. Whilst
WebhookService->dispatch()does queue jobs with.afterCommit(), the observer itself runs before the parent transaction commits. Addimplements ShouldHandleEventsAfterCommitto the observer class to defer the entire webhook dispatch until after commit, ensuring neither the observer nor delivery record creation occur during a potentially rollbackable transaction.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/Observers/WorkspaceWebhookObserver.php` around lines 42 - 52, Add the Laravel post-commit event interface to the observer class so the webhook dispatch is run after the DB transaction commits: update the WorkspaceWebhookObserver class declaration to implement ShouldHandleEventsAfterCommit (and add the corresponding use/import for that interface) so the existing $this->webhookService->dispatch(...) call is executed after commit rather than during the parent transaction.src/php/src/Api/Models/Link.php-16-27 (1)
16-27:⚠️ Potential issue | 🟠 MajorRemove analytics fields from
$fillable.
click_countandlast_clicked_atare server-derived metrics and should not be mass-assignable. Although the controller currently validates request data and excludes these fields, including them in$fillablerepresents unnecessary risk. These fields should be updated only by click-tracking code, never through request-backedfill()orcreate()calls.Suggested change
protected $fillable = [ 'workspace_id', 'user_id', 'name', 'slug', 'destination_url', 'short_code', 'is_active', - 'click_count', - 'last_clicked_at', 'metadata', ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/Models/Link.php` around lines 16 - 27, Remove the analytics fields from the model's mass-assignable list: in the Link model adjust the protected $fillable array to no longer include 'click_count' and 'last_clicked_at' so these server-derived metrics cannot be set via create()/fill() calls; ensure only click-tracking code updates these attributes (e.g., methods that increment click_count or set last_clicked_at) and leave the rest of $fillable (workspace_id, user_id, name, slug, destination_url, short_code, is_active, metadata) intact.openapi.go-1408-1471 (1)
1408-1471:⚠️ Potential issue | 🟠 MajorDo not document chat completions as public when the runtime still authenticates it.
This path item hard-codes
security: []any{}and drops401/403, butWithBearerAuthandWithAuthentikonly exempt/health, Swagger, and the standalone OpenAPI JSON route. As written, generated clients will be told/v1/chat/completionsis anonymous even though the engine still returns auth failures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi.go` around lines 1408 - 1471, The OpenAPI entry for the "post" chat completions path currently hard-codes "security: []any{}" and omits 401/403 responses which incorrectly documents the endpoint as public; remove the hard-coded security override (or set it to inherit the global security) in the same map where operationId is set (the block using operationID("post", path, operationIDs)) and add 401 and 403 response entries mirroring the 400/404/500 blocks (use chatCompletionsErrorSchema() and errorHeaders) so generated clients see authentication errors; search for the map containing "requestBody": map[string]any{...} and update the "security" key and "responses" map accordingly.src/php/src/Api/Services/SeoReportService.php-591-621 (1)
591-621:⚠️ Potential issue | 🟠 MajorRestore the explicit special-use IPv4 blocked-range table here.
This
filter_var(...FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE)approach is weaker than the explicit range table used inWebhookEndpoint::blockedIpv4Ranges()and reopens the SEO fetch path to non-public address space. Specifically:
100.64.0.0/10(IPv4 Shared Address Space) is not rejected by either flag198.51.100.0/24and203.0.113.0/24(documentation and test networks) are not documented as rejected byFILTER_FLAG_NO_RES_RANGEThese gaps represent a material SSRF control weakness. Adopt the same table-based approach as
WebhookEndpointto ensure consistent, explicit protection against reserved and special-use ranges.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/Services/SeoReportService.php` around lines 591 - 621, The isPrivateIp(string $ip) function currently relies on FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE which misses special-use IPv4 blocks (e.g., 100.64.0.0/10, 198.51.100.0/24, 203.0.113.0/24); replace that logic with the same explicit IPv4 blocked-range table used by WebhookEndpoint::blockedIpv4Ranges() and check membership against that table (including handling IPv4-mapped IPv6 by extracting the embedded IPv4 and checking it against the table); return true when the address (or embedded IPv4) falls into any blocked range, otherwise return false.src/php/src/Api/Controllers/Api/ApiKeyController.php-84-109 (1)
84-109:⚠️ Potential issue | 🟠 MajorCatch a dedicated exception for quota exhaustion, not every
RuntimeException.The
ApiKeyService::create()explicitly throws\RuntimeExceptiononly for the quota limit (line 51–53), but the controller catches allRuntimeExceptionand maps them toentitlement_exceeded. This masks genuine failures from database operations,ApiKey::generate(), scope normalisation, or transaction rollback—all of which could throwRuntimeException—leaving the client with a misleading 422 quota error instead of the real fault.Replace the broad catch with a dedicated exception class (e.g.
QuotaExceededException) in the service, then catch only that in the controller.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/Controllers/Api/ApiKeyController.php` around lines 84 - 109, The controller currently catches all \RuntimeException in ApiKeyController::create which masks real errors; change ApiKeyService::create to throw a specific QuotaExceededException (new class) when the API key quota is hit, update ApiKeyController to catch QuotaExceededException instead of \RuntimeException, remove or narrow any other broad RuntimeException catch branches so real runtime errors bubble up, and add the appropriate use/import for QuotaExceededException in the controller.src/php/src/Api/Controllers/Api/WebhookController.php-180-188 (1)
180-188:⚠️ Potential issue | 🟠 MajorPaginate webhook delivery history before this ships.
This endpoint currently loads every delivery row and every stored
response_bodyfor the webhook into one response. A busy endpoint will eventually make this action slow and memory-heavy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/Controllers/Api/WebhookController.php` around lines 180 - 188, The current WebhookController code fetches all delivery rows into $deliveries via $webhook->deliveries()->latest()->get(), which will load every response_body into memory; change this to use Laravel pagination (e.g. ->paginate($perPage)) on the $webhook->deliveries() query, then map/serialize only the paginated items with serializeDelivery($delivery) and return pagination metadata (total, per_page, current_page, etc.) instead of count(); update the response shape to include the paginated deliveries and metadata so the endpoint no longer returns the entire delivery history in one payload.chat_completions.go-44-54 (1)
44-54:⚠️ Potential issue | 🟠 Major
stopis not OpenAI-compatible yet.OpenAI clients can send
"stop": "\n"as a single string. WithStop []string, those requests fail during JSON decoding as an invalid body, so many off-the-shelf clients will not work against this endpoint.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@chat_completions.go` around lines 44 - 54, The Stop field on ChatCompletionRequest is typed as []string which rejects JSON where "stop" is a single string; change Stop to a custom type (e.g., StopSequences) and implement custom JSON unmarshal/marshal logic so it accepts either a string or an array of strings during decode/encode; update the ChatCompletionRequest struct to use StopSequences (field name Stop) and ensure the UnmarshalJSON for StopSequences normalizes a single string into a one-element slice and MarshalJSON emits an array for consistency.src/php/src/Api/Controllers/Api/QrCodeController.php-51-69 (1)
51-69:⚠️ Potential issue | 🟠 Major
format=pngis a broken contract at the moment.The create endpoint accepts and stores
png, butdownload()always emits SVG bytes withimage/svg+xmland a.svgfilename. Any client that requests PNG will get the wrong media type and payload.Smallest safe fix until PNG rendering exists
- 'format' => ['sometimes', 'string', 'in:png,svg'], + 'format' => ['sometimes', 'string', 'in:svg'], ... - 'format' => $data['format'] ?? 'svg', + 'format' => 'svg',Also applies to: 93-98
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/Controllers/Api/QrCodeController.php` around lines 51 - 69, The controller currently accepts and persists 'format' values including "png" but the QrCode download() always returns SVG bytes/headers, causing a mismatch; fix by preventing "png" from being accepted or by normalizing any requested format to "svg" before create. Update the request->validate rules (remove "png" from the 'in:png,svg' rule) or coerce $data['format'] to 'svg', and ensure QrCode::query()->create(...) stores only 'svg' for the 'format' field; also audit any uses of download() to confirm they expect 'svg'.chat_completions.go-960-982 (1)
960-982:⚠️ Potential issue | 🟠 Major
RemoteAddris not a trustworthy loopback-only security boundary.If this handler sits behind nginx, Caddy, or Apache on the same host, external traffic will often arrive with a loopback
RemoteAddr, so this check can be bypassed while the endpoint is still publicly exposed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@chat_completions.go` around lines 960 - 982, The current isLoopbackRequest function incorrectly treats r.RemoteAddr as a secure loopback boundary; update isLoopbackRequest to stop trusting RemoteAddr alone and instead determine the true client IP by (1) accepting a configurable trusted proxy IP/networks list and, if X-Forwarded-For (or Forwarded) headers are present, parse the header and validate that all upstream proxy hops (except the left-most client IP) are within the trusted proxies before using the left-most IP as the client, and (2) if no proxy headers exist or proxies are not trusted, fall back to using the connection peer (r.RemoteAddr or the connection from the request context) to verify loopback; ensure you reference and update the isLoopbackRequest function and any callers to pass the trusted proxy config and implement strict parsing/validation of X-Forwarded-For/Forwarded to avoid treating proxied external requests as loopback.src/php/src/Api/Routes/api.php-118-120 (1)
118-120:⚠️ Potential issue | 🟠 MajorRe-add scope enforcement on the authenticated auth routes.
GET /meandDELETE /tokenare the only authenticatedv1auth endpoints here that skipapi.scope.enforce. That means any valid token can reach them regardless of its granted scopes, which weakens the boundary hardening added elsewhere in this PR.Suggested fix
- Route::middleware(['auth.api', 'api.rate', 'api.cache:ephemeral'])->group(function () { + Route::middleware(['auth.api', 'api.scope.enforce', 'api.rate', 'api.cache:ephemeral'])->group(function () { Route::delete('/token', [AuthController::class, 'destroy'])->name('token.destroy'); Route::get('/me', [AuthController::class, 'show'])->name('me'); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/Routes/api.php` around lines 118 - 120, The auth routes for GET /me (AuthController::show) and DELETE /token (AuthController::destroy) are missing scope enforcement; update the Route::middleware call that currently uses ['auth.api','api.rate','api.cache:ephemeral'] to include the 'api.scope.enforce' middleware so these endpoints require proper token scopes (i.e., add 'api.scope.enforce' to the middleware array or insert it into the group definition protecting these routes).src/php/src/Api/Controllers/Api/TicketController.php-55-77 (1)
55-77:⚠️ Potential issue | 🟠 MajorReject ticket creation when neither workspace nor user is resolved.
store()can currently persist a ticket with bothworkspace_idanduser_idset tonull. That creates orphaned records thatindex()already refuses to list and that the rest of the controller cannot safely scope later.Suggested fix
public function store(Request $request): JsonResponse { $workspace = $this->resolveWorkspace($request); + if ($workspace === null && $request->user() === null) { + return $this->forbiddenResponse('Authentication or workspace context required.'); + } $data = $request->validate([🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/Controllers/Api/TicketController.php` around lines 55 - 77, In store(), before creating the SupportTicket, add a guard that rejects the request when both resolveWorkspace($request) returns null and $request->user() is null: check $workspace and $request->user() immediately after calling resolveWorkspace and if both are missing return a 400/422 JSON error (or use $this->errorResponse / abort with a clear message) so no ticket is created with workspace_id and user_id both null; keep the rest of the creation logic unchanged and reference store(), resolveWorkspace(), $request->user(), and SupportTicket::query()->create when making this change.
🟡 Minor comments (7)
docs/index.md-150-150 (1)
150-150:⚠️ Potential issue | 🟡 MinorModule-path migration is only partially applied in this doc
Line 150 uses the new ecosystem path, but this page still references the old module path at Line 10 and Line 35. Please update those to avoid conflicting guidance.
📝 Suggested follow-up edits in the same file
-**Module path:** `forge.lthn.ai/core/go-api` +**Module path:** `dappco.re/go/api` ... - api "forge.lthn.ai/core/go-api" + api "dappco.re/go/api"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/index.md` at line 150, Update the remaining references to the old module path in this document so they match the new ecosystem path `dappco.re/go/core/cli`; locate the earlier module import mentions near the top of the page (the two places that still use the old path) and replace them with `dappco.re/go/core/cli` so the doc consistently uses the new module path across the file.src/php/src/Api/Concerns/HasApiResponses.php-25-36 (1)
25-36:⚠️ Potential issue | 🟡 MinorData duplication in error response when
$metais non-empty.When
$metais provided, the same data appears twice in the response:
- Under the
detailskey- Merged at the top level via
array_merge($response, $meta)This could lead to bloated responses and potential confusion for API consumers. Consider whether both inclusions are intentional or if one should be removed.
🔧 Suggested fix if duplication is unintended
If only top-level merge is desired:
- if ($meta !== []) { - $response['details'] = $meta; - } - - return response()->json(array_merge($response, $meta), $status); + return response()->json(array_merge($response, $meta), $status);Or if only
detailsnesting is desired:if ($meta !== []) { $response['details'] = $meta; } - return response()->json(array_merge($response, $meta), $status); + return response()->json($response, $status);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/Concerns/HasApiResponses.php` around lines 25 - 36, The response currently duplicates $meta by both adding it as $response['details'] and merging it into the top-level with array_merge($response, $meta) before returning; to fix, pick one approach — I recommend keeping the nested details key for clarity: remove the array_merge and return response()->json($response, $status) so that HasApiResponses returns $response with 'details' containing $meta (remove the top-level merge that causes duplication).src/php/src/Api/RateLimit/RateLimitService.php-264-297 (1)
264-297:⚠️ Potential issue | 🟡 MinorRecompute
resetsAtafter appending the accepted hit.
calculateResetTime()currently runs on the pre-write slice. When this request consumes the last remaining slot, the returned reset time is based on the old hit set, so callers can receive a later reset timestamp than the bucket actually has.Proposed fix
protected function hitWithoutLock(string $cacheKey, int $limit, int $window, float $burst): RateLimitResult { $effectiveLimit = (int) floor($limit * $burst); $now = Carbon::now(); $windowStart = $now->timestamp - $window; @@ - // Calculate reset time - $resetsAt = $this->calculateResetTime($hits, $window, $effectiveLimit); - if ($currentCount >= $effectiveLimit) { // Find oldest hit to determine retry after $oldestHit = $hits !== [] ? min($hits) : $now->timestamp; $retryAfter = max(1, ($oldestHit + $window) - $now->timestamp); + $resetsAt = $this->calculateResetTime($hits, $window, $effectiveLimit); return RateLimitResult::denied($limit, $retryAfter, $resetsAt); } @@ if (! $this->storeWindowHits($cacheKey, $hits, $window)) { return RateLimitResult::denied($limit, 1, $now->copy()->addSecond()); } $remaining = max(0, $effectiveLimit - count($hits)); + $resetsAt = $this->calculateResetTime($hits, $window, $effectiveLimit); return RateLimitResult::allowed($limit, $remaining, $resetsAt); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/RateLimit/RateLimitService.php` around lines 264 - 297, The reset time is computed before the accepted hit is appended, which can return a later reset timestamp than the actual bucket; in hitWithoutLock(), after pushing $now->timestamp into $hits and successfully calling storeWindowHits($cacheKey, $hits, $window), recompute $resetsAt by calling calculateResetTime($hits, $window, $effectiveLimit) (or equivalent) using the updated $hits before computing $remaining and returning RateLimitResult::allowed($limit, $remaining, $resetsAt); ensure the recompute happens only after storeWindowHits returns true so denied paths remain unchanged.modernization_test.go-195-203 (1)
195-203:⚠️ Potential issue | 🟡 MinorUse a fatal length assertion before indexing
descs.Lines 198 and 201 index
descseven when the length check at Line 195 fails, becauset.Errorfis non-fatal.🔧 Suggested fix
- if len(descs) != 2 { - t.Errorf("DescribeIter failed: expected listing + 1 tool, got %v", descs) - } + if len(descs) != 2 { + t.Fatalf("DescribeIter failed: expected listing + 1 tool, got %v", descs) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modernization_test.go` around lines 195 - 203, The test currently uses t.Errorf to check len(descs) and then indexes descs[0] and descs[1], which can panic if the length check fails; change the length assertion to a fatal assertion (e.g., t.Fatalf) so the test stops immediately on failure before indexing descs, keeping the subsequent checks for descs[0] and descs[1] the same; reference the variable descs and the DescribeIter-related assertions in the test when making this change.src/php/src/Api/Boot.php-247-262 (1)
247-262:⚠️ Potential issue | 🟡 MinorPotential duplicate OAuth route registration.
registerOAuthRoutes()may be invoked twice: once viaonApiRoutes()(line 211) and again viaregisterFallbackApiRoutes()(line 260). Although the fallback path checks!Route::has('passport.token'), this guard only prevents duplication when the event-driven path runs first.If
registerFallbackApiRoutes()executes before theApiRoutesRegisteringevent fires, the routes will be registered twice, potentially causing route conflicts.🛡️ Suggested guard in onApiRoutes
if (class_exists(Passport::class)) { - $event->routes(fn () => $this->registerOAuthRoutes()); + $event->routes(fn () => Route::has('passport.token') ? null : $this->registerOAuthRoutes()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/Boot.php` around lines 247 - 262, The onApiRoutes() path can call registerOAuthRoutes() duplicate to registerFallbackApiRoutes(); add the same existence guard used in registerFallbackApiRoutes() so onApiRoutes() checks Route::has('passport.token') (and Passport::class existence) before calling registerOAuthRoutes(), or otherwise make registerOAuthRoutes() idempotent; update the onApiRoutes() method to bail out if the 'passport.token' route already exists to prevent double registration.src/php/src/Api/Models/WebhookDelivery.php-199-220 (1)
199-220:⚠️ Potential issue | 🟡 MinorAccessor may throw
JsonExceptionduring model hydration.
getPayloadAttribute()usesJSON_THROW_ON_ERROR(line 214) which can throwJsonExceptionwhen decoding malformed JSON. Since Eloquent accessors run during model hydration, a corrupted payload in the database could cause unexpected exceptions when loading anyWebhookDeliveryrecord.Consider catching and handling the exception gracefully:
🛡️ Suggested defensive handling
public function getPayloadAttribute(mixed $value): array { if (is_array($value)) { return $value; } if ($value === null || $value === '') { return []; } if (is_string($value)) { - $decoded = json_decode($value, true, 512, JSON_THROW_ON_ERROR); - - return is_array($decoded) ? $decoded : []; + try { + $decoded = json_decode($value, true, 512, JSON_THROW_ON_ERROR); + return is_array($decoded) ? $decoded : []; + } catch (\JsonException $e) { + Log::warning('Webhook delivery payload decode failed', [ + 'delivery_id' => $this->id ?? null, + 'error' => $e->getMessage(), + ]); + return []; + } } return (array) $value; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/Models/WebhookDelivery.php` around lines 199 - 220, The accessor getPayloadAttribute currently calls json_decode with JSON_THROW_ON_ERROR which can throw JsonException during Eloquent model hydration; wrap the json_decode call in a try/catch that catches JsonException (and any relevant \JsonException) and returns an empty array (or a safe fallback) instead of allowing the exception to bubble, so corrupted/malformed payloads do not break model loading; keep the JSON_THROW_ON_ERROR usage inside the try block (or remove it) and ensure the method still returns [] for null/invalid payloads.src/php/src/Api/Observers/SupportTicketReplyWebhookObserver.php-17-43 (1)
17-43:⚠️ Potential issue | 🟡 MinorConsider whether
messagefield may contain sensitive data.The webhook payload at line 37 includes
'message' => $reply->message. If support ticket messages can contain PII (emails, addresses, credentials shared by users), this could result in sensitive data being transmitted to external webhook endpoints.Consider whether:
- The message should be truncated or redacted
- A
message_previewwith a character limit would be safer- Webhook consumers should fetch the full message via authenticated API call
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/php/src/Api/Observers/SupportTicketReplyWebhookObserver.php` around lines 17 - 43, The webhook currently sends the full support reply text in SupportTicketReplyWebhookObserver::created via webhookService->dispatch (the 'message' => $reply->message entry); change this to avoid leaking PII by removing or redacting the full message and instead include a safe preview (e.g. 'message_preview' => mb_substr(strip_tags($reply->message), 0, 200) and/or a boolean 'full_message_available' => false) so consumers must use an authenticated API to fetch the full body; update the payload keys and docs accordingly and ensure created_at/metadata remain unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4221779c-cdfa-493f-9570-3a98a60a4164
⛔ Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumsrc/php/src/Website/.DS_Storeis excluded by!**/.DS_Store
📒 Files selected for processing (231)
.gitignore.gitleaksignoreCLAUDE.mdRFC.mdapi.goapi_describable_test.goapi_renderable_test.goapi_test.goauthentik.goauthentik_integration_test.goauthentik_test.goauthz_test.gobridge.gobridge_internal_test.gobridge_test.gobrotli.gobrotli_test.gocache.gocache_config_test.gocache_control.gocache_control_test.gocache_test.gochat_completions.gochat_completions_internal_test.gochat_completions_test.goclient.goclient_test.gocmd/api/cmd.gocmd/api/cmd_args.gocmd/api/cmd_args_test.gocmd/api/cmd_sdk.gocmd/api/cmd_sdk_test.gocmd/api/cmd_spec.gocmd/api/cmd_spec_test.gocmd/api/cmd_test.gocmd/api/spec_builder.gocmd/api/spec_groups_iter.gocodegen.gocodegen_test.gocomposer.jsondocs/architecture.mddocs/index.mdexport.goexport_test.goexpvar_test.gogo-io/go.modgo-io/local.gogo-log/error.gogo-log/go.modgo.modgraphql.gographql_config_test.gographql_test.gogroup.gogroup_test.gogzip_test.gohttpsign_test.goi18n.goi18n_test.golocation_test.gomiddleware.gomiddleware_test.gomodernization_test.goopenapi.goopenapi_test.gooptions.gopkg/provider/cache_control_example_test.gopkg/provider/cache_control_test.gopkg/provider/provider.gopkg/provider/proxy.gopkg/provider/proxy_test.gopkg/provider/registry.gopkg/provider/registry_test.gopkg/stream/stream_group.gopkg/stream/stream_group_example_test.gopkg/stream/stream_group_test.gopprof_test.goratelimit.goratelimit_internal_test.goratelimit_test.goresponse_meta.goresponse_meta_test.goresponse_test.goruntime_config_test.goscripts/generate-sdks.shscripts/publish-sdks.shsdk-config/csharp.yamlsdk-config/dart.yamlsdk-config/go.yamlsdk-config/java.yamlsdk-config/kotlin.yamlsdk-config/php.yamlsdk-config/python.yamlsdk-config/ruby.yamlsdk-config/rust.yamlsdk-config/swift.yamlsdk-config/typescript.yamlsecure_test.goservers.goservers_test.gosessions_test.goslog_test.gospec_builder_helper.gospec_builder_helper_test.gospec_registry_test.gosrc/php/phpunit.xmlsrc/php/src/Api/Boot.phpsrc/php/src/Api/Concerns/HasApiResponses.phpsrc/php/src/Api/Concerns/ResolvesWorkspace.phpsrc/php/src/Api/Console/Commands/CleanupExpiredGracePeriods.phpsrc/php/src/Api/Controllers/Api/ApiKeyController.phpsrc/php/src/Api/Controllers/Api/AuthController.phpsrc/php/src/Api/Controllers/Api/BiolinkController.phpsrc/php/src/Api/Controllers/Api/Concerns/SerialisesWorkspaceResource.phpsrc/php/src/Api/Controllers/Api/EntitlementApiController.phpsrc/php/src/Api/Controllers/Api/LinkController.phpsrc/php/src/Api/Controllers/Api/PaymentMethodController.phpsrc/php/src/Api/Controllers/Api/QrCodeController.phpsrc/php/src/Api/Controllers/Api/SeoReportController.phpsrc/php/src/Api/Controllers/Api/TicketController.phpsrc/php/src/Api/Controllers/Api/UnifiedPixelController.phpsrc/php/src/Api/Controllers/Api/WebhookController.phpsrc/php/src/Api/Controllers/Api/WebhookSecretController.phpsrc/php/src/Api/Controllers/Api/WebhookTemplateController.phpsrc/php/src/Api/Controllers/Api/WorkspaceMemberController.phpsrc/php/src/Api/Controllers/McpApiController.phpsrc/php/src/Api/Database/Factories/ApiKeyFactory.phpsrc/php/src/Api/Documentation/Attributes/ApiResponse.phpsrc/php/src/Api/Documentation/DocumentationController.phpsrc/php/src/Api/Documentation/DocumentationServiceProvider.phpsrc/php/src/Api/Documentation/Extensions/ApiKeyAuthExtension.phpsrc/php/src/Api/Documentation/Extensions/RateLimitExtension.phpsrc/php/src/Api/Documentation/Extensions/SunsetExtension.phpsrc/php/src/Api/Documentation/Middleware/ProtectDocumentation.phpsrc/php/src/Api/Documentation/OpenApiBuilder.phpsrc/php/src/Api/Documentation/Routes/docs.phpsrc/php/src/Api/Documentation/config.phpsrc/php/src/Api/Guards/AccessTokenGuard.phpsrc/php/src/Api/Jobs/DeliverWebhookJob.phpsrc/php/src/Api/Listeners/DispatchSubscriptionWebhookEvents.phpsrc/php/src/Api/Middleware/ApiCacheControl.phpsrc/php/src/Api/Middleware/AuthenticateApiKey.phpsrc/php/src/Api/Middleware/RateLimitApi.phpsrc/php/src/Api/Migrations/2026_04_15_000000_create_api_resource_tables.phpsrc/php/src/Api/Migrations/2026_04_15_000001_create_support_ticket_tables.phpsrc/php/src/Api/Models/ApiKey.phpsrc/php/src/Api/Models/Biolink.phpsrc/php/src/Api/Models/Concerns/BelongsToWorkspace.phpsrc/php/src/Api/Models/Link.phpsrc/php/src/Api/Models/QrCode.phpsrc/php/src/Api/Models/SupportTicket.phpsrc/php/src/Api/Models/SupportTicketReply.phpsrc/php/src/Api/Models/WebhookDelivery.phpsrc/php/src/Api/Models/WebhookEndpoint.phpsrc/php/src/Api/Observers/BiolinkWebhookObserver.phpsrc/php/src/Api/Observers/LinkWebhookObserver.phpsrc/php/src/Api/Observers/SupportTicketReplyWebhookObserver.phpsrc/php/src/Api/Observers/SupportTicketWebhookObserver.phpsrc/php/src/Api/Observers/WorkspaceWebhookObserver.phpsrc/php/src/Api/RateLimit/RateLimitService.phpsrc/php/src/Api/Resources/ApiKeyResource.phpsrc/php/src/Api/Routes/admin.phpsrc/php/src/Api/Routes/api.phpsrc/php/src/Api/Services/ApiKeyService.phpsrc/php/src/Api/Services/SeoReportService.phpsrc/php/src/Api/Services/WebhookService.phpsrc/php/src/Api/Services/WebhookTemplateService.phpsrc/php/src/Api/Tests/Feature/ApiKeyRotationTest.phpsrc/php/src/Api/Tests/Feature/ApiKeySecurityTest.phpsrc/php/src/Api/Tests/Feature/ApiKeyTest.phpsrc/php/src/Api/Tests/Feature/ApiScopeEnforcementTest.phpsrc/php/src/Api/Tests/Feature/AuthenticateApiKeyTest.phpsrc/php/src/Api/Tests/Feature/DocumentationControllerTest.phpsrc/php/src/Api/Tests/Feature/DocumentationStoplightTest.phpsrc/php/src/Api/Tests/Feature/McpApiControllerTest.phpsrc/php/src/Api/Tests/Feature/McpResourceTest.phpsrc/php/src/Api/Tests/Feature/McpServerAccessTest.phpsrc/php/src/Api/Tests/Feature/McpServerDetailTest.phpsrc/php/src/Api/Tests/Feature/RateLimitTest.phpsrc/php/src/Api/Tests/Feature/RateLimitingTest.phpsrc/php/src/Api/Tests/Feature/SeoReportEndpointTest.phpsrc/php/src/Api/Tests/Feature/SeoReportServiceTest.phpsrc/php/src/Api/Tests/Feature/WebhookDeliveryTest.phpsrc/php/src/Api/Tests/Feature/WebhookEndpointTest.phpsrc/php/src/Api/Tests/Feature/WebhookTemplateServiceTest.phpsrc/php/src/Api/config.phpsrc/php/src/Front/Api/Middleware/ApiSunset.phpsrc/php/src/Front/Api/Middleware/ApiVersion.phpsrc/php/src/Front/Api/VersionedRoutes.phpsrc/php/src/Front/Api/config.phpsrc/php/src/Website/Api/Controllers/DocsController.phpsrc/php/src/Website/Api/Routes/web.phpsrc/php/src/Website/Api/View/Blade/changelog.blade.phpsrc/php/src/Website/Api/View/Blade/guides/authentication.blade.phpsrc/php/src/Website/Api/View/Blade/guides/index.blade.phpsrc/php/src/Website/Api/View/Blade/guides/quickstart.blade.phpsrc/php/src/Website/Api/View/Blade/guides/rate-limits.blade.phpsrc/php/src/Website/Api/View/Blade/index.blade.phpsrc/php/src/Website/Api/View/Blade/sdks.blade.phpsrc/php/tests/Feature/ApiSunsetTest.phpsrc/php/tests/Feature/ApiVersionParsingTest.phpsrc/php/tests/Feature/ApiVersionServiceTest.phpsrc/php/tests/Feature/AuthenticationGuideTest.phpsrc/php/tests/Feature/DocsControllerTest.phpsrc/php/tests/Feature/VersionedRoutesTest.phpsse.gosse_internal_test.gosse_test.gossrf_guard.gossrf_guard_internal_test.gostatic_test.gosunset.gosunset_internal_test.gosunset_test.goswagger.goswagger_test.gotests/Pest.phptests/cli/api/Taskfile.yamltimeout_test.gotracing.gotracing_test.gotransport.gotransport_client.gotransport_client_test.gotransport_test.gowebhook.gowebhook_internal_test.gowebhook_test.gowebsocket.gowebsocket_internal_test.gowebsocket_test.go
💤 Files with no reviewable changes (4)
- go-io/go.mod
- go-log/error.go
- go-io/local.go
- go-log/go.mod
| outputDir: ./sdks/go | ||
| additionalProperties: | ||
| packageName: core | ||
| moduleName: github.com/dappcore/core-go |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== SDK config values =="
rg -n "^(generatorName|outputDir|additionalProperties|\\s+packageName|\\s+moduleName):" sdk-config/go.yaml -n -C1
echo
echo "== Cross-repo references to module names =="
rg -n "github.com/dappcore/core-go|dappco\\.re/go/api|dappco\\.re/go/" -C2
echo
echo "== Docs mentioning Go SDK module/import path =="
fd -i "README*" . | xargs -r rg -n "Go SDK|moduleName|import|core-go|dappco\\.re/go" -C2Repository: dAppCore/api
Length of output: 18677
Update Go SDK moduleName to match namespace migration to dappco.re/go.
The moduleName: github.com/dappcore/core-go in sdk-config/go.yaml conflicts with the codebase's migration to the dappco.re/go/* namespace. The repository's own go.mod declares module dappco.re/go/api, and all internal imports throughout the codebase use dappco.re/go/api and dappco.re/go/core. Update moduleName to dappco.re/go/api (or the appropriate generated SDK module name) to maintain consistency and ensure generated imports align with the current namespace strategy.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk-config/go.yaml` at line 5, Update the Go SDK moduleName field in
sdk-config/go.yaml from "github.com/dappcore/core-go" to the current namespace
used by the repo (e.g. "dappco.re/go/api") so generated SDK imports match the
codebase; modify the moduleName entry value to "dappco.re/go/api" (or the
appropriate generated module name) ensuring alignment with the project's go.mod
module declaration and internal imports.
…close Mantis #931 Both workspace and user resolution returning null fell through to an unscoped SupportTicket::find($id), allowing any actor whose request context fails to resolve to read or reply to ANY ticket by ID enumeration (CWE-639 IDOR, REACHABLE-CRITICAL per Cerberus DREAD). Fix per Cerberus's recommendation: - Throw AuthorizationException (not silent null) when both contexts null - Log fail-open attempts with actor IP for ops alerting - Keep existing scoped-lookup logic intact when context resolves Test triad: - AnonymousAccess_Bad — anonymous request returns 401, not a ticket - FailOpenAttempt_Ugly — log.warning fires with actor_ip + ticket_id - AuthenticatedUser_Good — legitimate access still succeeds Filer: CodeRabbit (PR #3 dAppCore/api) DREAD review: Cerberus (Mantis #931 verdict REACHABLE-CRITICAL) Co-Authored-By: CodeRabbit <noreply@coderabbit.ai> Co-Authored-By: Cerberus <cerberus@lthn.ai> Co-Authored-By: Athena <athena@lthn.ai> Co-Authored-By: Virgil <virgil@lethean.io>
…pace The Go SDK codegen config still pointed at the pre-migration github.com/dappcore/core-go module path. CodeRabbit (PR #3, Mantis #932) flagged it as the only sdk-config/* yaml that wasn't updated during the workspace-wide module rename to dappco.re/go. Pattern matches the api repo's own go.mod (`module dappco.re/go/api`) and the Java/TypeScript SDK configs that already use the dappco.re/re.dappco namespaces. SDK consumers will get a Go module declaration that resolves under the canonical dappco.re proxy. Closes Mantis #932 on PR #3 dAppCore/api. Co-authored-by: Cerberus <cerberus@lthn.ai> Co-authored-by: Hephaestus <hephaestus@lthn.ai>
Main moved forward independently while the release branch sat
since the v0.8.0-alpha.1 cut. Main carries the canonical:
- flat namespace `dappco.re/go` (release used `dappco.re/go/core`)
- reorganised `go/`, `go/pkg/provider/`, `php/` paths
(release was still on `./`, `pkg/provider/`, `src/php/`)
- own SSRF guard + Mantis #931 IDOR fix at higher coverage
- sonar BLOCKER clears (#1287) + CI infra (#1217)
- service.go consolidation (#1336)
Conflict resolution policy: accept main's version where main's
work is at-or-ahead of release's parallel implementation
(security hardening, namespace, directory layout). Accept the
deletions main made (proxy.go etc — moved/refactored). Release's
genuinely-new test files moved to their canonical post-reorg
paths.
28 conflicts resolved:
4 modify/delete (release's mods to files main deleted) → accept delete
4 add/add (same path, different content) → main's namespace
1 add unresolved (path rename) → file added at new path
19 both modified (mostly namespace + dir migration) → main's version
|
The Engine builds gin's NoRoute hook from a new noRouteHandler
field, set via WithNoRoute(h gin.HandlerFunc). Mounted after every
explicit route + group so it never shadows real handlers; nil
preserves gin's default 404 behaviour (back-compat with pre-option
callers).
Use case: lthn/desktop and lthn.ai-hosted Engines both serve an
SPA whose client-router owns the URL space. Today they wrap the
gin engine via Handler() and call NoRoute() directly, which means
they bypass any auth/SSRF/cache middleware composed into the
Engine. With WithNoRoute the SPA fallback becomes part of the
canonical Engine build and inherits the middleware chain.
go:
- pkg/api: Engine gains `noRouteHandler gin.HandlerFunc`
- new option WithNoRoute(h) sets it
- build() registers it via r.NoRoute(...) when non-nil
- TestWithNoRoute_{Good,Bad,Ugly} AX-7 triplet
- ExampleWithNoRoute() showing the SPA-host pattern
Also dedupes accidentally-duplicated TestSDKGenerator_Generate_
PackageName{Accepted_Good,Rejected_Bad} from the PR #3 merge
(two identical copies of the same two tests at 199-269 and
270-339 caused the test binary to fail compile).
2 participants
Summary
Consolidates the v0.8.0-alpha.1 release work for core/api. Module path migration, new SDK-codegen interfaces, and significant security hardening across the request boundary.
Headline changes
forge.lthn.ai/core/api→dappco.re/go/api. All siblingdappco.re/go/*deps pinned tov0.8.0-alpha.1. PHP-sidededoc/scramblebumped^0.12→^0.13.Describableinterface — providers expose human-readable metadataRenderableinterface — renderers expose templates and hintsStreamGroupinterface — SSE endpoints declare event shapesRouteGroupIsValidMCPServerIDvalidates server identifiers at the boundarydoHTTPClientRequestchoke point with allow-list and unit + integration coveragePackageNamehardened against flag-injection (G204 false-positive annotated)schemaIntbound-checksuint/uint64→intcoercion (G115)newChatCompletionIDG404 false-positive annotated.gitleaksignoreadded for one documented false-positivetests/cli/api/Taskfile drivers cover the CLI surface..DS_Storeundersrc/php/src/Website; dropped empty.core/TODO.md.Refs
RFC-CORE-008-AGENT-EXPERIENCE.md(AX-1, AX-6, AX-10)Test plan
go test ./...passestests/cli/apiTaskfile drivers exercise the CLI surfaceCo-authored-by: Athena athena@lthn.ai
Co-authored-by: Cerberus cerberus@lthn.ai
Co-authored-by: Hephaestus hephaestus@lthn.ai
Co-authored-by: Cladius Maximus cladius@lthn.ai
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests