From 6871160623ae5a42a3b1581de4ca4bccf78f0603 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 2 Jul 2026 15:50:43 +0200 Subject: [PATCH] fix(api): gate sensitive storage and GitHub fields Expose GitHub app secrets and file storage content only when the request has sensitive read access. Hide LocalFileVolume content by default and resolve application UUIDs from route parameters. --- .../Api/ApplicationsController.php | 22 +++-- .../Controllers/Api/DatabasesController.php | 14 ++- app/Http/Controllers/Api/GithubController.php | 15 ++- .../Controllers/Api/ServicesController.php | 19 +++- app/Models/Application.php | 5 - app/Models/LocalFileVolume.php | 4 + tests/Feature/Api/GithubAppsListApiTest.php | 66 ++++++++++--- tests/Feature/StorageApiTest.php | 96 ++++++++++++++++++- .../Unit/Models/SensitiveFieldsHiddenTest.php | 9 +- 9 files changed, 214 insertions(+), 36 deletions(-) diff --git a/app/Http/Controllers/Api/ApplicationsController.php b/app/Http/Controllers/Api/ApplicationsController.php index 8c8006973..e06f195a5 100644 --- a/app/Http/Controllers/Api/ApplicationsController.php +++ b/app/Http/Controllers/Api/ApplicationsController.php @@ -33,6 +33,15 @@ class ApplicationsController extends Controller { + private function exposeFileStorageContentIfAllowed(LocalFileVolume|LocalPersistentVolume $storage): LocalFileVolume|LocalPersistentVolume + { + if (request()->attributes->get('can_read_sensitive', false) === true) { + $storage->makeVisible(['content']); + } + + return $storage; + } + private function removeSensitiveData($application) { $application->makeHidden([ @@ -2031,7 +2040,7 @@ public function application_by_uuid(Request $request) if (! $uuid) { return response()->json(['message' => 'UUID is required.'], 400); } - $application = Application::ownedByCurrentTeamAPI($teamId)->where('uuid', $request->uuid)->first(); + $application = Application::ownedByCurrentTeamAPI($teamId)->where('uuid', $request->route('uuid'))->first(); if (! $application) { return response()->json(['message' => 'Application not found.'], 404); } @@ -2112,7 +2121,7 @@ public function logs_by_uuid(Request $request) if (! $uuid) { return response()->json(['message' => 'UUID is required.'], 400); } - $application = Application::ownedByCurrentTeamAPI($teamId)->where('uuid', $request->uuid)->first(); + $application = Application::ownedByCurrentTeamAPI($teamId)->where('uuid', $request->route('uuid'))->first(); if (! $application) { return response()->json(['message' => 'Application not found.'], 404); } @@ -2205,7 +2214,7 @@ public function delete_by_uuid(Request $request) if (! $request->uuid) { return response()->json(['message' => 'UUID is required.'], 404); } - $application = Application::ownedByCurrentTeamAPI($teamId)->where('uuid', $request->uuid)->first(); + $application = Application::ownedByCurrentTeamAPI($teamId)->where('uuid', $request->route('uuid'))->first(); if (! $application) { return response()->json([ @@ -2416,7 +2425,7 @@ public function update_by_uuid(Request $request) return $return; } - $application = Application::ownedByCurrentTeamAPI($teamId)->where('uuid', $request->uuid)->first(); + $application = Application::ownedByCurrentTeamAPI($teamId)->where('uuid', $request->route('uuid'))->first(); if (! $application) { return response()->json([ 'message' => 'Application not found', @@ -4028,6 +4037,7 @@ public function storages(Request $request): JsonResponse $persistentStorages = $application->persistentStorages->sortBy('id')->values(); $fileStorages = $application->fileStorages->sortBy('id')->values(); + $fileStorages->each(fn (LocalFileVolume $storage) => $this->exposeFileStorageContentIfAllowed($storage)); return response()->json([ 'persistent_storages' => $persistentStorages, @@ -4242,7 +4252,7 @@ public function update_storage(Request $request): JsonResponse 'mount_path' => $storage->mount_path ?? null, ]); - return response()->json($storage); + return response()->json($this->exposeFileStorageContentIfAllowed($storage)); } #[OA\Post( @@ -4429,7 +4439,7 @@ public function create_storage(Request $request): JsonResponse 'mount_path' => $storage->mount_path, ]); - return response()->json($storage, 201); + return response()->json($this->exposeFileStorageContentIfAllowed($storage), 201); } #[OA\Delete( diff --git a/app/Http/Controllers/Api/DatabasesController.php b/app/Http/Controllers/Api/DatabasesController.php index b36e557c0..75a99d783 100644 --- a/app/Http/Controllers/Api/DatabasesController.php +++ b/app/Http/Controllers/Api/DatabasesController.php @@ -28,6 +28,15 @@ class DatabasesController extends Controller { + private function exposeFileStorageContentIfAllowed(LocalFileVolume|LocalPersistentVolume $storage): LocalFileVolume|LocalPersistentVolume + { + if (request()->attributes->get('can_read_sensitive', false) === true) { + $storage->makeVisible(['content']); + } + + return $storage; + } + private function removeSensitiveData($database) { $database->makeHidden([ @@ -3664,6 +3673,7 @@ public function storages(Request $request): JsonResponse $persistentStorages = $database->persistentStorages->sortBy('id')->values(); $fileStorages = $database->fileStorages->sortBy('id')->values(); + $fileStorages->each(fn (LocalFileVolume $storage) => $this->exposeFileStorageContentIfAllowed($storage)); return response()->json([ 'persistent_storages' => $persistentStorages, @@ -3855,7 +3865,7 @@ public function create_storage(Request $request): JsonResponse 'mount_path' => $storage->mount_path, ]); - return response()->json($storage, 201); + return response()->json($this->exposeFileStorageContentIfAllowed($storage), 201); } #[OA\Patch( @@ -4062,7 +4072,7 @@ public function update_storage(Request $request): JsonResponse 'mount_path' => $storage->mount_path ?? null, ]); - return response()->json($storage); + return response()->json($this->exposeFileStorageContentIfAllowed($storage)); } #[OA\Delete( diff --git a/app/Http/Controllers/Api/GithubController.php b/app/Http/Controllers/Api/GithubController.php index 150743f99..eba16a67b 100644 --- a/app/Http/Controllers/Api/GithubController.php +++ b/app/Http/Controllers/Api/GithubController.php @@ -17,10 +17,17 @@ class GithubController extends Controller { private function removeSensitiveData($githubApp) { - $githubApp->makeHidden([ - 'client_secret', - 'webhook_secret', - ]); + if (request()->attributes->get('can_read_sensitive', false) === true) { + $githubApp->makeVisible([ + 'client_secret', + 'webhook_secret', + ]); + } else { + $githubApp->makeHidden([ + 'client_secret', + 'webhook_secret', + ]); + } return serializeApiResponse($githubApp); } diff --git a/app/Http/Controllers/Api/ServicesController.php b/app/Http/Controllers/Api/ServicesController.php index fb03c9e1a..d970b7ba0 100644 --- a/app/Http/Controllers/Api/ServicesController.php +++ b/app/Http/Controllers/Api/ServicesController.php @@ -24,8 +24,21 @@ class ServicesController extends Controller { + private function exposeFileStorageContentIfAllowed(LocalFileVolume|LocalPersistentVolume $storage): LocalFileVolume|LocalPersistentVolume + { + if (request()->attributes->get('can_read_sensitive', false) === true) { + $storage->makeVisible(['content']); + } + + return $storage; + } + private function removeSensitiveData($service) { + if ($service instanceof Collection) { + return $service->map(fn (Service $item) => $this->removeSensitiveData($item)); + } + $service->makeHidden([ 'id', 'resourceable', @@ -2065,6 +2078,8 @@ public function storages(Request $request): JsonResponse ); } + $fileStorages->each(fn (LocalFileVolume $storage) => $this->exposeFileStorageContentIfAllowed($storage)); + return response()->json([ 'persistent_storages' => $persistentStorages->sortBy('id')->values(), 'file_storages' => $fileStorages->sortBy('id')->values(), @@ -2265,7 +2280,7 @@ public function create_storage(Request $request): JsonResponse 'mount_path' => $storage->mount_path, ]); - return response()->json($storage, 201); + return response()->json($this->exposeFileStorageContentIfAllowed($storage), 201); } #[OA\Patch( @@ -2502,7 +2517,7 @@ public function update_storage(Request $request): JsonResponse 'mount_path' => $storage->mount_path ?? null, ]); - return response()->json($storage); + return response()->json($this->exposeFileStorageContentIfAllowed($storage)); } #[OA\Delete( diff --git a/app/Models/Application.php b/app/Models/Application.php index ebfa2cd9b..d844dd480 100644 --- a/app/Models/Application.php +++ b/app/Models/Application.php @@ -176,11 +176,8 @@ class Application extends BaseModel 'manual_webhook_secret_bitbucket', 'manual_webhook_secret_gitea', 'docker_compose_location', - 'docker_compose_pr_location', 'docker_compose', - 'docker_compose_pr', 'docker_compose_raw', - 'docker_compose_pr_raw', 'docker_compose_domains', 'docker_compose_custom_start_command', 'docker_compose_custom_build_command', @@ -232,9 +229,7 @@ class Application extends BaseModel 'manual_webhook_secret_gitea', 'dockerfile', 'docker_compose', - 'docker_compose_pr', 'docker_compose_raw', - 'docker_compose_pr_raw', 'custom_labels', ]; diff --git a/app/Models/LocalFileVolume.php b/app/Models/LocalFileVolume.php index 627750232..a5f95138a 100644 --- a/app/Models/LocalFileVolume.php +++ b/app/Models/LocalFileVolume.php @@ -24,6 +24,10 @@ class LocalFileVolume extends BaseModel 'is_preview_suffix_enabled' => 'boolean', ]; + protected $hidden = [ + 'content', + ]; + use HasFactory; protected $fillable = [ diff --git a/tests/Feature/Api/GithubAppsListApiTest.php b/tests/Feature/Api/GithubAppsListApiTest.php index a6ce59dca..66d3d6908 100644 --- a/tests/Feature/Api/GithubAppsListApiTest.php +++ b/tests/Feature/Api/GithubAppsListApiTest.php @@ -1,6 +1,7 @@ set('app.maintenance.driver', 'file'); + config()->set('cache.default', 'array'); + + InstanceSettings::query()->delete(); + $settings = new InstanceSettings; + $settings->id = 0; + $settings->save(); + // Create a team with owner $this->team = Team::factory()->create(); $this->user = User::factory()->create(); $this->team->members()->attach($this->user->id, ['role' => 'owner']); + session(['currentTeam' => $this->team]); // Create an API token for the user - $this->token = $this->user->createToken('test-token', ['*'], $this->team->id); + $this->token = $this->user->createToken('test-token', ['*']); $this->bearerToken = $this->token->plainTextToken; // Create a private key for the team - $this->privateKey = PrivateKey::create([ - 'name' => 'Test Key', - 'private_key' => 'test-private-key-content', + $this->privateKey = PrivateKey::factory()->create([ 'team_id' => $this->team->id, ]); }); +function createGithubAppsApiToken($context, array $abilities): string +{ + session(['currentTeam' => $context->team]); + + return $context->user->createToken('github-apps-test-token', $abilities)->plainTextToken; +} + describe('GET /api/v1/github-apps', function () { test('returns 401 when not authenticated', function () { $response = $this->getJson('/api/v1/github-apps'); @@ -71,7 +86,7 @@ ]); }); - test('does not return sensitive data', function () { + test('does not return sensitive data for read tokens', function () { // Create a GitHub app GithubApp::create([ 'name' => 'Test GitHub App', @@ -86,8 +101,10 @@ 'team_id' => $this->team->id, ]); + $readToken = createGithubAppsApiToken($this, ['read']); + $response = $this->withHeaders([ - 'Authorization' => 'Bearer '.$this->bearerToken, + 'Authorization' => 'Bearer '.$readToken, ])->getJson('/api/v1/github-apps'); $response->assertStatus(200); @@ -98,6 +115,33 @@ expect($json[0])->not->toHaveKey('webhook_secret'); }); + test('returns sensitive data for read sensitive tokens', function () { + GithubApp::create([ + 'name' => 'Sensitive GitHub App', + 'api_url' => 'https://api.github.com', + 'html_url' => 'https://github.com', + 'app_id' => 12345, + 'installation_id' => 67890, + 'client_id' => 'test-client-id', + 'client_secret' => 'secret-should-be-visible', + 'webhook_secret' => 'webhook-secret-should-be-visible', + 'private_key_id' => $this->privateKey->id, + 'team_id' => $this->team->id, + ]); + + $sensitiveToken = createGithubAppsApiToken($this, ['read', 'read:sensitive']); + + $response = $this->withHeaders([ + 'Authorization' => 'Bearer '.$sensitiveToken, + ])->getJson('/api/v1/github-apps'); + + $response->assertSuccessful(); + $response->assertJsonFragment([ + 'client_secret' => 'secret-should-be-visible', + 'webhook_secret' => 'webhook-secret-should-be-visible', + ]); + }); + test('returns system-wide github apps', function () { // Create a system-wide GitHub app $systemApp = GithubApp::create([ @@ -118,7 +162,8 @@ $otherTeam = Team::factory()->create(); $otherUser = User::factory()->create(); $otherTeam->members()->attach($otherUser->id, ['role' => 'owner']); - $otherToken = $otherUser->createToken('other-token', ['*'], $otherTeam->id); + session(['currentTeam' => $otherTeam]); + $otherToken = $otherUser->createToken('other-token', ['*']); // System-wide apps should be visible to other teams $response = $this->withHeaders([ @@ -150,11 +195,6 @@ // Create another team with a GitHub app $otherTeam = Team::factory()->create(); - $otherPrivateKey = PrivateKey::create([ - 'name' => 'Other Key', - 'private_key' => 'other-key', - 'team_id' => $otherTeam->id, - ]); GithubApp::create([ 'name' => 'Team 2 App', 'api_url' => 'https://api.github.com', @@ -164,7 +204,7 @@ 'client_id' => 'team2-client-id', 'client_secret' => 'team2-secret', 'webhook_secret' => 'team2-webhook', - 'private_key_id' => $otherPrivateKey->id, + 'private_key_id' => $this->privateKey->id, 'team_id' => $otherTeam->id, 'is_system_wide' => false, ]); diff --git a/tests/Feature/StorageApiTest.php b/tests/Feature/StorageApiTest.php index 75357e41e..3d112e5a6 100644 --- a/tests/Feature/StorageApiTest.php +++ b/tests/Feature/StorageApiTest.php @@ -18,8 +18,14 @@ uses(RefreshDatabase::class); beforeEach(function () { + config()->set('app.maintenance.driver', 'file'); + config()->set('cache.default', 'array'); + Bus::fake(); - InstanceSettings::updateOrCreate(['id' => 0]); + InstanceSettings::query()->delete(); + $settings = new InstanceSettings; + $settings->id = 0; + $settings->save(); $this->team = Team::factory()->create(); $this->user = User::factory()->create(); @@ -42,8 +48,15 @@ function createTestApplication($context): Application { - return Application::factory()->create([ + return Application::create([ + 'name' => 'test-storage-app', + 'git_repository' => 'https://github.com/test/test', + 'git_branch' => 'main', + 'build_pack' => 'nixpacks', + 'ports_exposes' => '3000', 'environment_id' => $context->environment->id, + 'destination_id' => $context->destination->id, + 'destination_type' => $context->destination->getMorphClass(), ]); } @@ -61,6 +74,19 @@ function createTestDatabase($context): StandalonePostgresql ]); } +function createStorageApiToken($context, array $abilities): string +{ + $plainTextToken = Str::random(40); + $token = $context->user->tokens()->create([ + 'name' => 'storage-test-token', + 'token' => hash('sha256', $plainTextToken), + 'abilities' => $abilities, + 'team_id' => $context->team->id, + ]); + + return $token->getKey().'|'.$plainTextToken; +} + // ────────────────────────────────────────────────────────────── // Application Storage Endpoints // ────────────────────────────────────────────────────────────── @@ -92,6 +118,39 @@ function createTestDatabase($context): StandalonePostgresql $response->assertStatus(404); }); + + test('hides file storage content for read tokens and reveals it for sensitive tokens', function () { + $app = createTestApplication($this); + + LocalFileVolume::create([ + 'fs_path' => '/tmp/config.json', + 'mount_path' => '/app/config.json', + 'content' => '{"secret": "hidden"}', + 'is_directory' => false, + 'resource_id' => $app->id, + 'resource_type' => $app->getMorphClass(), + ]); + + $readToken = createStorageApiToken($this, ['read']); + $sensitiveToken = createStorageApiToken($this, ['read', 'read:sensitive']); + + $readResponse = $this->withHeaders([ + 'Authorization' => 'Bearer '.$readToken, + ])->getJson("/api/v1/applications/{$app->uuid}/storages"); + + auth()->forgetGuards(); + + $sensitiveResponse = $this->withHeaders([ + 'Authorization' => 'Bearer '.$sensitiveToken, + ])->getJson("/api/v1/applications/{$app->uuid}/storages"); + + $readResponse->assertSuccessful(); + $sensitiveResponse->assertSuccessful(); + + expect($readResponse->getContent())->not->toContain('"content":') + ->and($sensitiveResponse->getContent())->toContain('"content":') + ->and($sensitiveResponse->getContent())->toContain('hidden'); + }); }); describe('POST /api/v1/applications/{uuid}/storages', function () { @@ -142,6 +201,39 @@ function createTestDatabase($context): StandalonePostgresql expect($vol->is_directory)->toBeFalse(); }); + test('hides created file storage content for write tokens and reveals it for sensitive tokens', function () { + $app = createTestApplication($this); + $writeToken = createStorageApiToken($this, ['write']); + $sensitiveToken = createStorageApiToken($this, ['write', 'read:sensitive']); + + $writeResponse = $this->withHeaders([ + 'Authorization' => 'Bearer '.$writeToken, + 'Content-Type' => 'application/json', + ])->postJson("/api/v1/applications/{$app->uuid}/storages", [ + 'type' => 'file', + 'mount_path' => '/app/hidden.json', + 'content' => '{"secret": "write-hidden"}', + ]); + + auth()->forgetGuards(); + + $sensitiveResponse = $this->withHeaders([ + 'Authorization' => 'Bearer '.$sensitiveToken, + 'Content-Type' => 'application/json', + ])->postJson("/api/v1/applications/{$app->uuid}/storages", [ + 'type' => 'file', + 'mount_path' => '/app/visible.json', + 'content' => '{"secret": "write-visible"}', + ]); + + $writeResponse->assertCreated(); + $sensitiveResponse->assertCreated(); + + expect($writeResponse->getContent())->not->toContain('"content":') + ->and($sensitiveResponse->getContent())->toContain('"content":') + ->and($sensitiveResponse->getContent())->toContain('write-visible'); + }); + test('rejects persistent storage without name', function () { $app = createTestApplication($this); diff --git a/tests/Unit/Models/SensitiveFieldsHiddenTest.php b/tests/Unit/Models/SensitiveFieldsHiddenTest.php index c63a6084d..3faece684 100644 --- a/tests/Unit/Models/SensitiveFieldsHiddenTest.php +++ b/tests/Unit/Models/SensitiveFieldsHiddenTest.php @@ -8,6 +8,7 @@ use App\Models\EmailNotificationSettings; use App\Models\EnvironmentVariable; use App\Models\InstanceSettings; +use App\Models\LocalFileVolume; use App\Models\OauthSetting; use App\Models\PrivateKey; use App\Models\PushoverNotificationSettings; @@ -63,9 +64,7 @@ 'manual_webhook_secret_gitea', 'dockerfile', 'docker_compose', - 'docker_compose_pr', 'docker_compose_raw', - 'docker_compose_pr_raw', 'custom_labels', ); }); @@ -94,6 +93,12 @@ expect($hidden)->toContain('logs'); }); + test('LocalFileVolume hides file content', function () { + $hidden = (new LocalFileVolume)->getHidden(); + + expect($hidden)->toContain('content'); + }); + test('PrivateKey hides private key material', function () { $hidden = (new PrivateKey)->getHidden();