Skip to content

Fix effective SDK platform availability in OpenAPI specs #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
anurag6569201 wants to merge 1 commit into
base: qa/agent-appwrite-appwrite/pr-09-13502/base
Choose a base branch
from
Open

Fix effective SDK platform availability in OpenAPI specs #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 25 additions & 43 deletions src/Appwrite/Platform/Tasks/Specs.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,23 +414,15 @@ protected function getKeys(): array

public function getSDKPlatformsForRouteSecurity(array $routeSecurity): array
{
$sdkPlatforms = [];
foreach ($routeSecurity as $value) {
switch ($value) {
case AuthType::SESSION:
$sdkPlatforms[] = APP_SDK_PLATFORM_CLIENT;
break;
case AuthType::JWT:
case AuthType::KEY:
$sdkPlatforms[] = APP_SDK_PLATFORM_SERVER;
break;
case AuthType::ADMIN:
$sdkPlatforms[] = APP_SDK_PLATFORM_CONSOLE;
break;
$platforms = [];
foreach ($routeSecurity as $auth) {
$platform = $auth instanceof AuthType ? $auth->getPlatform() : null;
if ($platform !== null) {
$platforms[] = $platform;
}
}

return $sdkPlatforms;
return $platforms;
}

public function action(string $version, string $mode, ?string $git, ?string $message, ?string $branch): void
Expand Down Expand Up @@ -479,6 +471,18 @@ public function action(string $version, string $mode, ?string $git, ?string $mes
throw new Exception('Failed to create specs directory: ' . $specsDir);
}

// Resolve full auth arrays through the active task (including subclass platforms).

@shipwright-agent shipwright-agent Bot Sep 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shipwright · HIGH

The new pre-resolution loop in Specs::action() calls setPlatforms() on every SDK Method before the per-platform loop, but getPlatforms() can still return an empty list when a Metho

Impact: The new pre-resolution loop in Specs::action() calls setPlatforms() on every SDK Method before the per-platform loop, but getPlatforms() can still return an empty list when a Method has no namespace or is hidden. The per-platform loop then skips the route, silently dropping it from all generated specs. If any route relies on the old behavior where namespace/hide checks happened later or differently, that route disap…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

@shipwright-agent shipwright-agent Bot Sep 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shipwright · LOW

The new pre-resolution loop mutates SDK Method objects in place while iterating over $appRoutes.

Impact: The new pre-resolution loop mutates SDK Method objects in place while iterating over $appRoutes. This side effect is not obvious from the surrounding code and makes the action() method harder to reason about because platform resolution now happens in two places: the pre-loop and getPlatforms() fallback.

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

foreach ($appRoutes as $method) {
foreach ($method as $route) {
$sdks = $route->getLabel('sdk', []);
foreach (\is_array($sdks) ? $sdks : [$sdks] as $sdk) {
if ($sdk instanceof Method) {
$sdk->setPlatforms($this->getSDKPlatformsForRouteSecurity($sdk->getAuth()));
}
}
}
}

foreach ($platforms as $platform) {
$routes = [];
$models = [];
Expand All @@ -487,6 +491,10 @@ public function action(string $version, string $mode, ?string $git, ?string $mes

foreach ($appRoutes as $key => $method) {
foreach ($method as $route) {
if (!$route->getLabel('docs', true) || (bool) $route->getLabel('mock', false) !== $mocks) {
continue;
}

$sdks = $route->getLabel('sdk', false);

if (empty($sdks)) {
Expand All @@ -498,37 +506,11 @@ public function action(string $version, string $mode, ?string $git, ?string $mes
}

foreach ($sdks as $sdk) {
/** @var Method $sdk */
$hide = $sdk->isHidden();

if ($hide === true || (\is_array($hide) && \in_array($platform, $hide))) {
continue;
}

$routeSecurity = $sdk->getAuth();
$sdkPlatforms = $this->getSDKPlatformsForRouteSecurity($routeSecurity);

if (!$route->getLabel('docs', true)) {
continue;
}

if ($route->getLabel('mock', false) && !$mocks) {
continue;
}

if (!$route->getLabel('mock', false) && $mocks) {
continue;
}

if (empty($sdk->getNamespace())) {
continue;
}

if (!\in_array($platform, $sdkPlatforms)) {
if (!\in_array($platform, $sdk->getPlatforms(), true)) {
continue;
}

$routes[] = $route;
$routes[\spl_object_id($route)] = $route;
$routeNamespaces[$sdk->getNamespace()] = true;
}
}
Expand Down Expand Up @@ -603,7 +585,7 @@ public function action(string $version, string $mode, ?string $git, ?string $mes
$models,
$keys[$platform],
$authCounts[$platform] ?? 0,
$platform
$platform,
];

foreach (['open-api3'] as $format) {
Expand Down
10 changes: 10 additions & 0 deletions src/Appwrite/SDK/AuthType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,14 @@ enum AuthType: string
* auth types that make them reachable (ADMIN, KEY, ...).
*/
case ORGANIZATION = APP_AUTH_TYPE_ORGANIZATION;

public function getPlatform(): ?string

@shipwright-agent shipwright-agent Bot Sep 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shipwright · HIGH

getPlatforms() derives platforms from getAuth() when $this->platforms is null, but AuthType::ORGANIZATION maps to null and is filtered out.

Impact: getPlatforms() derives platforms from getAuth() when $this->platforms is null, but AuthType::ORGANIZATION maps to null and is filtered out. A route secured only by ORGANIZATION will produce an empty platform list and be excluded from every SDK spec, even though the old switch also had no case for ORGANIZATION and would have produced an empty list. This is a latent behavior change risk if ORGANIZATION-only routes…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

{
return match ($this) {
self::SESSION => APP_SDK_PLATFORM_CLIENT,
self::JWT, self::KEY => APP_SDK_PLATFORM_SERVER,
self::ADMIN => APP_SDK_PLATFORM_CONSOLE,
self::ORGANIZATION => null,
};
}
}
30 changes: 30 additions & 0 deletions src/Appwrite/SDK/Method.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ class Method

public static array $errors = [];

/** @var list<string>|null Null derives membership from auth; an explicit empty list stays empty. */

@shipwright-agent shipwright-agent Bot Sep 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shipwright · HIGH

The comment 'Null derives membership from auth; an explicit empty list stays empty' describes a subtle distinction that is not enforced by the type system.

Impact: The comment 'Null derives membership from auth; an explicit empty list stays empty' describes a subtle distinction that is not enforced by the type system. A future caller cannot tell from the API whether setPlatforms([]) means 'no platforms' or 'not yet resolved', and getPlatforms() silently falls back to auth-derived platforms only when the property is null. This implicit state machine is easy to misuse.

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

protected ?array $platforms = null;

/**
* Initialise a new SDK method
*
Expand Down Expand Up @@ -202,6 +205,33 @@ public function isHidden(): bool|array
return $this->hide;
}

/**
* @param list<string> $platforms Auth membership resolved by the active specs producer.
*/
public function setPlatforms(array $platforms): self
{
$this->platforms = $platforms;
return $this;
}

/**
* @return list<string> Eligible platforms, independent of the currently selected spec platform.
*/
public function getPlatforms(): array
{
$hide = $this->isHidden();
if ($hide === true || empty($this->getNamespace())) {
return [];
}

$platforms = $this->platforms ?? \array_filter(\array_map(
fn ($auth) => $auth instanceof AuthType ? $auth->getPlatform() : null,
$this->getAuth()
));

return \array_values(\array_unique(\array_diff($platforms, \is_array($hide) ? $hide : [])));
}

public function isPackaging(): bool
{
return $this->packaging;
Expand Down
23 changes: 13 additions & 10 deletions src/Appwrite/SDK/Specification/Format/OpenAPI3.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace Appwrite\SDK\Specification\Format;

use Appwrite\Platform\Tasks\Specs;
use Appwrite\SDK\AuthType;
use Appwrite\SDK\ContentType;
use Appwrite\SDK\Method;
Expand Down Expand Up @@ -245,9 +244,19 @@ public function parse(): array
continue;
}

$sdkPlatforms = [];

@shipwright-agent shipwright-agent Bot Sep 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shipwright · HIGH

The OpenAPI3 parser now trusts Method::getPlatforms() to decide whether a route is emitted for a platform.

Impact: The OpenAPI3 parser now trusts Method::getPlatforms() to decide whether a route is emitted for a platform. Because getPlatforms() can be populated by the active specs producer via setPlatforms(), any producer that fails to call setPlatforms() or calls it with incorrect data will cause routes to be omitted from generated API specs. This is a spec-generation integrity risk rather than a runtime auth bypass, but it can…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

foreach (\is_array($sdk) ? $sdk : [$sdk] as $method) {
$sdkPlatforms = \array_merge($sdkPlatforms, $method->getPlatforms());
}
$sdkPlatforms = \array_values(\array_unique($sdkPlatforms));
if (!\in_array($this->platform, $sdkPlatforms, true)) {
continue;
}

$additionalMethods = null;
if (\is_array($sdk)) {
$additionalMethods = $sdk;
// Keep the original base descriptor's schemas and auth, even when only a sibling is eligible.
$sdk = $sdk[0];
}

Expand All @@ -260,12 +269,6 @@ public function parse(): array

$desc = $sdk->getDescriptionFilePath() ?: $sdk->getDescription();
$produces = ($sdk->getContentType())->value;
$routeSecurity = $sdk->getAuth();

$specs = new Specs();
$sdkPlatforms = $specs->getSDKPlatformsForRouteSecurity($routeSecurity);

$sdkPlatforms = array_values(array_unique($sdkPlatforms));
$namespace = $sdk->getNamespace();

$descContents = $this->getDescriptionContents($desc);
Expand Down Expand Up @@ -303,10 +306,9 @@ public function parse(): array
/** @var Method $methodObj */
$desc = $methodObj->getDescriptionFilePath();

$methodSecurities = $methodObj->getAuth();
$methodSdkPlatforms = $specs->getSDKPlatformsForRouteSecurity($methodSecurities);
$methodSdkPlatforms = $methodObj->getPlatforms();

if (!\in_array($this->platform, $methodSdkPlatforms)) {
if (!\in_array($this->platform, $methodSdkPlatforms, true)) {
continue;
}

Expand All @@ -320,6 +322,7 @@ public function parse(): array
$additionalMethod = [
'name' => $methodObj->getMethodName(),
'namespace' => $methodObj->getNamespace(),
'platforms' => $methodSdkPlatforms,
'desc' => $methodObj->getDesc(),
'auth' => \array_slice($methodSecurities, 0, $this->authCount),
'parameters' => [],
Expand Down
Loading