From e03dac8612cd6903649363e41ddd9f1c05c202e9 Mon Sep 17 00:00:00 2001 From: Moritz Date: Mon, 12 Oct 2026 08:31:26 +0000 Subject: [PATCH 1/2] Accept and expose package attestation bundles First PR in the stack for package attestation support: - Adds `AssetKind.attestation` for storing attestation bundles as `PackageVersionAsset` entities in Datastore. - `GET /api/packages/versions/new` now returns an `attestationUrl`, pointing at `POST /api/packages/versions/newUploadAttestation/`. Repositories that do not support publishing with attestations omit the property, which is how `pub` detects support before uploading the archive (dart-lang/pub#4876). - The new endpoint requires an authenticated client, validates that the upload id is a uuid, that the bundle is at most `maxAttestationContentLength` (128 KB), and that it parses as a JSON object, and then stores it as `tmp/.attestation.json` in the incoming bucket. - `packageBackend.publishUploadedBlob` reads that object if it is present, saves it as an attestation asset, and removes it together with the other temporary objects. `newUploadFinish` remains a `GET`-only endpoint. - Attestations are not passed through `capContent()`: oversized bundles are rejected when they are uploaded rather than silently truncated. - Exposes `GET /api/packages//versions//attestation` for client retrieval, updates `PubApiClientExt` and adds unit tests. Squash of the previous commits of this branch, rebased onto `main`. --- app/lib/frontend/handlers/pubapi.client.dart | 21 ++ app/lib/frontend/handlers/pubapi.dart | 58 ++++++ app/lib/frontend/handlers/pubapi.g.dart | 35 ++++ app/lib/package/backend.dart | 109 +++++++++- app/lib/package/models.dart | 1 + app/lib/tool/utils/pub_api_client.dart | 61 +++--- .../handlers/invalid_package_url_test.dart | 4 +- app/test/package/upload_test.dart | 189 +++++++++++++++++- pkg/_pub_shared/lib/data/package_api.dart | 12 +- pkg/_pub_shared/lib/data/package_api.g.dart | 7 +- pkg/_pub_shared/lib/src/pubapi.client.dart | 21 ++ pkg/api_builder/lib/_client_utils.dart | 22 ++ .../lib/src/client_library_generator.dart | 13 ++ 13 files changed, 522 insertions(+), 31 deletions(-) diff --git a/app/lib/frontend/handlers/pubapi.client.dart b/app/lib/frontend/handlers/pubapi.client.dart index 4b8d5d009c..bce44f4fb8 100644 --- a/app/lib/frontend/handlers/pubapi.client.dart +++ b/app/lib/frontend/handlers/pubapi.client.dart @@ -31,6 +31,8 @@ class PubApiClient { final _i2.Client _client; + _i2.Client get client => _client; + Future> listVersions(String package) async { return await _client.requestBytes( verb: 'get', @@ -60,6 +62,16 @@ class PubApiClient { ); } + Future> getPackageVersionAttestation( + String package, + String version, + ) async { + return await _client.requestBytes( + verb: 'get', + path: '/api/packages/$package/versions/$version/attestation', + ); + } + Future> fetchPackage(String package, String version) async { return await _client.requestBytes( verb: 'get', @@ -76,6 +88,15 @@ class PubApiClient { ); } + Future<_i3.SuccessMessage> uploadPackageAttestation(String uploadId) async { + return _i3.SuccessMessage.fromJson( + await _client.requestJson( + verb: 'post', + path: '/api/packages/versions/newUploadAttestation/$uploadId', + ), + ); + } + Future<_i3.SuccessMessage> packageUploadCallback() async { return _i3.SuccessMessage.fromJson( await _client.requestJson( diff --git a/app/lib/frontend/handlers/pubapi.dart b/app/lib/frontend/handlers/pubapi.dart index 4affc652b7..5cf495af52 100644 --- a/app/lib/frontend/handlers/pubapi.dart +++ b/app/lib/frontend/handlers/pubapi.dart @@ -17,6 +17,7 @@ import 'package:shelf_router/shelf_router.dart'; import '../../account/consent_backend.dart'; import '../../admin/backend.dart'; import '../../package/backend.dart' hide InviteStatus; +import '../../package/models.dart' show AssetKind; import '../../publisher/backend.dart'; import '../../shared/exceptions.dart'; import '../../shared/handlers.dart'; @@ -80,6 +81,31 @@ class PubApi { ); } + /// Fetches the Sigstore attestation bundle for a specific (package, version) pair. + @EndPoint.get('/api/packages//versions//attestation') + Future getPackageVersionAttestation( + Request request, + String package, + String version, + ) async { + checkPackageVersionParams(package, version); + final asset = await packageBackend.lookupPackageVersionAsset( + package, + version, + AssetKind.attestation, + ); + if (asset == null || asset.textContent == null) { + throw NotFoundException.resource('attestation for $package $version'); + } + return Response.ok( + asset.textContent, + headers: { + 'content-type': 'application/json; charset="utf-8"', + ...CacheControl.clientApi.headers, + }, + ); + } + /// Downloading package. /// /// This is the endpoint we link to from the version listing. @@ -138,7 +164,39 @@ class PubApi { // integration tests before we switch traffic. await packageBackend.startUpload( request.requestedUri.resolve('/api/packages/versions/newUploadFinish'), + attestationUrlPrefix: request.requestedUri.resolve( + '/api/packages/versions/newUploadAttestation', + ), + ); + + /// Upload an attestation for a package archive that is being uploaded. + /// + /// The attestation is uploaded before the package archive, and is verified + /// and stored when the upload is finished. + /// https://github.com/dart-lang/pub/blob/master/doc/repository-spec-v2.md#publishing-with-an-attestation + /// + /// POST /api/packages/versions/newUploadAttestation/ + /// + /// [200 OK] + /// { + /// "success" : { + /// "message": "Attestation uploaded.", + /// }, + /// } + @EndPoint.post('/api/packages/versions/newUploadAttestation/') + Future uploadPackageAttestation( + Request request, + String uploadId, + ) async { + final bytes = await request.read().expand((i) => i).toList(); + if (bytes.isEmpty) { + throw PackageRejectedException( + 'Invalid attestation bundle format: the request body is empty.', ); + } + await packageBackend.uploadAttestation(uploadId, bytes); + return SuccessMessage(success: Message(message: 'Attestation uploaded.')); + } /// Finish async upload. /// TODO: Link to the spec once it has the details updated: diff --git a/app/lib/frontend/handlers/pubapi.g.dart b/app/lib/frontend/handlers/pubapi.g.dart index b0b0e7bc87..78242d07e9 100644 --- a/app/lib/frontend/handlers/pubapi.g.dart +++ b/app/lib/frontend/handlers/pubapi.g.dart @@ -75,6 +75,24 @@ Router _$PubApiRouter(PubApi service) { return $utilities.unhandledError(e, st); } }); + router.add('GET', r'/api/packages//versions//attestation', ( + Request request, + String package, + String version, + ) async { + try { + final _$result = await service.getPackageVersionAttestation( + request, + package, + version, + ); + return _$result; + } on ApiResponseException catch (e) { + return e.asApiResponse(); + } catch (e, st) { + return $utilities.unhandledError(e, st); + } + }); router.add('GET', r'/api/archives/-.tar.gz', ( Request request, String package, @@ -99,6 +117,23 @@ Router _$PubApiRouter(PubApi service) { return $utilities.unhandledError(e, st); } }); + router.add( + 'POST', + r'/api/packages/versions/newUploadAttestation/', + (Request request, String uploadId) async { + try { + final _$result = await service.uploadPackageAttestation( + request, + uploadId, + ); + return $utilities.jsonResponse(_$result.toJson()); + } on ApiResponseException catch (e) { + return e.asApiResponse(); + } catch (e, st) { + return $utilities.unhandledError(e, st); + } + }, + ); router.add('GET', r'/api/packages/versions/newUploadFinish', ( Request request, ) async { diff --git a/app/lib/package/backend.dart b/app/lib/package/backend.dart index a14a3ba91e..2a729adb1a 100644 --- a/app/lib/package/backend.dart +++ b/app/lib/package/backend.dart @@ -3,7 +3,7 @@ // BSD-style license that can be found in the LICENSE file. import 'dart:async'; - +import 'dart:convert'; import 'dart:io'; import 'package:_pub_shared/data/account_api.dart' as account_api; @@ -63,10 +63,22 @@ import 'upload_signer_service.dart'; // that is stored separately in the database. final maxAssetContentLength = 256 * 1024; +// The maximum length of an attestation bundle. +// +// Note: unlike other assets, attestations are rejected when they are longer +// than this, as a truncated attestation could never be verified. +final maxAttestationContentLength = 128 * 1024; + /// The maximum number of versions a package is allowed to have. final _defaultMaxVersionsPerPackage = 1000; final Logger _logger = Logger('pub.cloud_repository'); + +/// Matches the UUIDs created by `createUuid()`, used to identify uploads. +final _uuidRegExp = RegExp( + r'^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$', + caseSensitive: false, +); final _validGitHubUserOrRepoRegExp = RegExp( r'^[a-z0-9\-\._]+$', caseSensitive: false, @@ -1254,7 +1266,16 @@ class PackageBackend { return pv.toApiVersionInfo(); } - Future startUpload(Uri redirectUrl) async { + /// Starts an upload, returning the parameters the client needs to upload the + /// package archive. + /// + /// [redirectUrl] is the URL the client is redirected to when the archive has + /// been uploaded, and [attestationUrlPrefix] is the URL prefix where the + /// client may upload an attestation of the archive. + Future startUpload( + Uri redirectUrl, { + required Uri attestationUrlPrefix, + }) async { final restriction = await getUploadRestrictionStatus(); if (restriction == UploadRestrictionStatus.noUploads) { throw PackageRejectedException.uploadRestricted(); @@ -1276,12 +1297,57 @@ class PackageBackend { _logger.info( 'Redirecting pub client to google cloud storage (uuid: $guid)', ); - return uploadSigner.buildUpload( + final uploadInfo = await uploadSigner.buildUpload( bucket, object, lifetime, successRedirectUrl: '$url', ); + return api.UploadInfo( + url: uploadInfo.url, + fields: uploadInfo.fields, + attestationUrl: '$attestationUrlPrefix/$guid', + ); + } + + /// Stores the attestation [bytes] uploaded for the upload with [uploadGuid]. + /// + /// The attestation is verified and stored with the package version when the + /// upload is finished, see [publishUploadedBlob]. + Future uploadAttestation(String uploadGuid, List bytes) async { + final restriction = await getUploadRestrictionStatus(); + if (restriction == UploadRestrictionStatus.noUploads) { + throw PackageRejectedException.uploadRestricted(); + } + await requireAuthenticatedClient(); + InvalidInputException.check( + _uuidRegExp.hasMatch(uploadGuid), + 'Invalid upload id.', + ); + if (bytes.length > maxAttestationContentLength) { + throw PackageRejectedException( + 'Attestation bundle is too large ' + '(max $maxAttestationContentLength bytes).', + ); + } + // Verify that the bundle is a JSON object before storing it. It can only be + // verified against the archive when the upload is finished. + try { + final decoded = jsonDecode(utf8.decode(bytes)); + if (decoded is! Map) { + throw FormatException('Attestation bundle must be a JSON object.'); + } + } on FormatException catch (e) { + throw PackageRejectedException('Invalid attestation bundle format: $e'); + } + + _logger.info('Uploading attestation (uuid: $uploadGuid).'); + await uploadWithRetry( + _incomingBucket, + tmpAttestationObjectName(uploadGuid), + bytes.length, + () => Stream.value(bytes), + ); } /// Finishes the upload of a package and returns the list of messages @@ -1298,6 +1364,7 @@ class PackageBackend { return await withTempDirectory((Directory dir) async { // Check the existence of the uploaded file final uploadObjectName = tmpObjectName(uploadGuid); + final attestationObjectName = tmpAttestationObjectName(uploadGuid); final info = await _incomingBucket.tryInfo(uploadObjectName); if (info?.length == null) { throw PackageRejectedException.archiveEmpty(); @@ -1426,12 +1493,26 @@ class PackageBackend { throw PackageRejectedException.dependencyDoesNotExists(name); } + // Read the attestation, if one was uploaded for this upload. + // Note: the content has been validated to be a JSON object when it was + // uploaded, see [uploadAttestation]. + String? attestationContent; + if (await _incomingBucket.tryInfo(attestationObjectName) != null) { + attestationContent = utf8.decode( + await _incomingBucket.readAsBytes( + attestationObjectName, + maxSize: maxAttestationContentLength, + ), + ); + } + sw.reset(); final entities = await _createUploadEntities( db, agent, archive, sha256Hash: sha256Hash, + attestationContent: attestationContent, ); final (version, uploadMessages) = await _performTarballUpload( entities: entities, @@ -1447,6 +1528,9 @@ class PackageBackend { sw.reset(); await _incomingBucket.deleteWithRetry(uploadObjectName); await _incomingBucket.deleteWithRetry(workObjectName); + if (attestationContent != null) { + await _incomingBucket.deleteWithRetry(attestationObjectName); + } _logger.info('Temporary object removed in ${sw.elapsed}.'); return [ 'Successfully uploaded ' @@ -2622,6 +2706,7 @@ Future<_UploadEntities> _createUploadEntities( AuthenticatedAgent agent, PackageSummary archive, { required List sha256Hash, + String? attestationContent, }) async { final pubspec = Pubspec.fromYaml(archive.pubspecContent!); final packageKey = db.emptyKey.append(Package, id: pubspec.name); @@ -2641,6 +2726,7 @@ Future<_UploadEntities> _createUploadEntities( final derived = derivePackageVersionEntities( archive: archive, versionCreated: version.created!, + attestationContent: attestationContent, ); // TODO: verify if assets sizes are within the transaction limit (10 MB) @@ -2651,6 +2737,7 @@ Future<_UploadEntities> _createUploadEntities( DerivedPackageVersionEntities derivePackageVersionEntities({ required PackageSummary archive, required DateTime versionCreated, + String? attestationContent, }) { final pubspec = Pubspec.fromYaml(archive.pubspecContent!); final key = QualifiedVersionKey( @@ -2709,6 +2796,17 @@ DerivedPackageVersionEntities derivePackageVersionEntities({ path: archive.licensePath, textContent: capContent(archive.licenseContent), ), + if (attestationContent != null) + PackageVersionAsset.init( + package: key.package, + version: key.version, + kind: AssetKind.attestation, + versionCreated: versionCreated, + path: '${key.package}-${key.version}.sigstore.json', + // Note: not capped, a truncated attestation could never be verified. + // The length is checked when the attestation is uploaded. + textContent: attestationContent, + ), ]; final versionInfo = PackageVersionInfo() @@ -2727,6 +2825,11 @@ DerivedPackageVersionEntities derivePackageVersionEntities({ @visibleForTesting String tmpObjectName(String guid) => 'tmp/$guid'; +/// The GCS object name of the attestation uploaded for the temporary object +/// [guid] - excluding leading '/'. +@visibleForTesting +String tmpAttestationObjectName(String guid) => 'tmp/$guid.attestation.json'; + /// Verify that the [package] and the optional [version] parameter looks as acceptable input. void checkPackageVersionParams(String package, [String? version]) { InvalidInputException.checkPackageName(package); diff --git a/app/lib/package/models.dart b/app/lib/package/models.dart index 3d244062ba..f984cb1b90 100644 --- a/app/lib/package/models.dart +++ b/app/lib/package/models.dart @@ -761,6 +761,7 @@ abstract class AssetKind { static const changelog = 'changelog'; static const example = 'example'; static const license = 'license'; + static const attestation = 'attestation'; } /// A derived entity that holds extracted asset of a [PackageVersion] archive. diff --git a/app/lib/tool/utils/pub_api_client.dart b/app/lib/tool/utils/pub_api_client.dart index dd9270dde2..9f369b6d94 100644 --- a/app/lib/tool/utils/pub_api_client.dart +++ b/app/lib/tool/utils/pub_api_client.dart @@ -175,34 +175,49 @@ bool _retryIf(Exception e) { extension PubApiClientExt on PubApiClient { @visibleForTesting Future preparePackageUpload(List bytes) async { - final uploadInfo = await getPackageUploadUrl(); + return await _uploadArchive(await getPackageUploadUrl(), bytes); + } - final request = http.MultipartRequest('POST', Uri.parse(uploadInfo.url)) - ..headers[fakeClockHeaderName] = clock.now().toIso8601String() - ..fields.addAll(uploadInfo.fields!) - ..files.add(http.MultipartFile.fromBytes('file', bytes)) - ..followRedirects = false; - final uploadRs = await request.send(); - if (uploadRs.statusCode != 303) { - // NOTE: There are tests that fail with this on CI. - // TODO: figure out what is causing these issues. - final body = await uploadRs.stream.bytesToString(); - final headers = uploadRs.headers; - throw AssertionError( - 'Expected HTTP redirect, got ${uploadRs.statusCode}.' - '\nbody: $body\nheaders: $headers', + @visibleForTesting + Future uploadPackageBytes( + List bytes, { + List? attestationBytes, + }) async { + final uploadInfo = await getPackageUploadUrl(); + if (attestationBytes != null) { + // The attestation is uploaded before the archive. + await client.sendRaw( + verb: 'post', + path: Uri.parse(uploadInfo.attestationUrl!).path, + headers: {'content-type': 'application/json; charset="utf-8"'}, + bodyBytes: attestationBytes, ); } + final uploadId = await _uploadArchive(uploadInfo, bytes); + return await finishPackageUpload(uploadId); + } +} - final callbackUri = Uri.parse( - uploadInfo.fields!['success_action_redirect']!, +/// Uploads the package archive [bytes] with the parameters from [uploadInfo], +/// and returns the upload id to finish the upload with. +Future _uploadArchive(UploadInfo uploadInfo, List bytes) async { + final request = http.MultipartRequest('POST', Uri.parse(uploadInfo.url)) + ..headers[fakeClockHeaderName] = clock.now().toIso8601String() + ..fields.addAll(uploadInfo.fields!) + ..files.add(http.MultipartFile.fromBytes('file', bytes)) + ..followRedirects = false; + final uploadRs = await request.send(); + if (uploadRs.statusCode != 303) { + // NOTE: There are tests that fail with this on CI. + // TODO: figure out what is causing these issues. + final body = await uploadRs.stream.bytesToString(); + final headers = uploadRs.headers; + throw AssertionError( + 'Expected HTTP redirect, got ${uploadRs.statusCode}.' + '\nbody: $body\nheaders: $headers', ); - return callbackUri.queryParameters['upload_id']!; } - @visibleForTesting - Future uploadPackageBytes(List bytes) async { - final uploadId = await preparePackageUpload(bytes); - return await finishPackageUpload(uploadId); - } + final callbackUri = Uri.parse(uploadInfo.fields!['success_action_redirect']!); + return callbackUri.queryParameters['upload_id']!; } diff --git a/app/test/frontend/handlers/invalid_package_url_test.dart b/app/test/frontend/handlers/invalid_package_url_test.dart index d4ee331c38..9c49005ac5 100644 --- a/app/test/frontend/handlers/invalid_package_url_test.dart +++ b/app/test/frontend/handlers/invalid_package_url_test.dart @@ -34,7 +34,7 @@ void main() { expect(urls, contains('/packages//versions/')); expect(urls, contains('/api/packages/')); // this a naive assertion that fails, if new end-points are introduced! - expect(urls, hasLength(44), reason: 'check if new end-points was added'); + expect(urls, hasLength(45), reason: 'check if new end-points was added'); }); testWithProfile( @@ -71,7 +71,7 @@ void main() { (url) => url .replaceAll('', 'oxygen') .replaceAll('', '1.2.0'), - {200, 303, 401}, + {200, 303, 401, 404}, ); await check( diff --git a/app/test/package/upload_test.dart b/app/test/package/upload_test.dart index bb8eb2f39a..8530a63fe6 100644 --- a/app/test/package/upload_test.dart +++ b/app/test/package/upload_test.dart @@ -3,6 +3,7 @@ // BSD-style license that can be found in the LICENSE file. import 'dart:async'; +import 'dart:convert'; import 'package:_pub_shared/data/package_api.dart'; import 'package:clock/clock.dart'; @@ -44,6 +45,7 @@ void main() { fn: () async { final rs = packageBackend.startUpload( Uri.parse('http://example.com/'), + attestationUrlPrefix: Uri.parse('http://example.com/attestation'), ); await expectLater(rs, throwsA(isA())); }, @@ -53,14 +55,26 @@ void main() { 'successful', fn: () async { final redirectUri = Uri.parse('http://blobstore.com/upload'); + final attestationUrlPrefix = Uri.parse( + 'http://blobstore.com/attestation', + ); await accountBackend.withBearerToken(userClientToken, () async { - final info = await packageBackend.startUpload(redirectUri); + final info = await packageBackend.startUpload( + redirectUri, + attestationUrlPrefix: attestationUrlPrefix, + ); expect(info.url, startsWith('http://localhost:')); expect(info.url, contains('/fake-incoming-packages/tmp/')); expect(info.fields, { 'key': startsWith('fake-incoming-packages/tmp/'), 'success_action_redirect': startsWith('$redirectUri?upload_id='), }); + // The attestation is uploaded with the same upload id as the + // archive. + final uploadId = Uri.parse( + info.fields!['success_action_redirect']!, + ).queryParameters['upload_id']; + expect(info.attestationUrl, '$attestationUrlPrefix/$uploadId'); }); }, ); @@ -1696,5 +1710,178 @@ void main() { ); }, ); + + testWithProfile( + 'successful upload with attestation bundle and api retrieval', + fn: () async { + final pubspecContent = + 'name: attested_pkg\nversion: 1.0.0\ndescription: A package with attestation.\nenvironment:\n sdk: ">=2.12.0 <4.0.0"\n'; + final archiveBytes = await packageArchiveBytes( + pubspecContent: pubspecContent, + ); + final bundleJson = { + 'mediaType': 'application/vnd.dev.sigstore.bundle.v0.3+json', + 'verificationMaterial': {}, + 'dsseEnvelope': { + 'payloadType': 'application/vnd.in-toto+json', + 'payload': base64Encode(utf8.encode('{}')), + 'signatures': [], + }, + }; + final attestationBytes = utf8.encode(jsonEncode(bundleJson)); + + final client = createPubApiClient(authToken: adminClientToken); + final message = await client.uploadPackageBytes( + archiveBytes, + attestationBytes: attestationBytes, + ); + expect(message.success.message, contains('Successfully uploaded')); + + // Verify attestation asset was stored in Datastore + final asset = await packageBackend.lookupPackageVersionAsset( + 'attested_pkg', + '1.0.0', + AssetKind.attestation, + ); + expect(asset, isNotNull); + expect(asset!.textContent, isNotNull); + final storedJson = + jsonDecode(asset.textContent!) as Map; + expect( + storedJson['mediaType'], + equals('application/vnd.dev.sigstore.bundle.v0.3+json'), + ); + + // Verify attestation can be retrieved via the API endpoint + final retrievedBytes = await client.getPackageVersionAttestation( + 'attested_pkg', + '1.0.0', + ); + final retrievedJson = + jsonDecode(utf8.decode(retrievedBytes)) as Map; + expect( + retrievedJson['mediaType'], + equals('application/vnd.dev.sigstore.bundle.v0.3+json'), + ); + }, + ); + + testWithProfile( + 'retrieving attestation of a package without attestation returns 404', + fn: () async { + final pubspecContent = + 'name: unattested_pkg\nversion: 1.0.0\ndescription: A package without attestation.\nenvironment:\n sdk: ">=2.12.0 <4.0.0"\n'; + final archiveBytes = await packageArchiveBytes( + pubspecContent: pubspecContent, + ); + + final client = createPubApiClient(authToken: adminClientToken); + final message = await client.uploadPackageBytes(archiveBytes); + expect(message.success.message, contains('Successfully uploaded')); + + final rs = client.getPackageVersionAttestation( + 'unattested_pkg', + '1.0.0', + ); + await expectApiException( + rs, + status: 404, + code: 'NotFound', + message: 'Could not find `attestation for unattested_pkg 1.0.0`.', + ); + }, + ); + + testWithProfile( + 'upload fails when attestation bundle has invalid JSON or invalid bytes', + fn: () async { + final pubspecContent = + 'name: bad_attested_pkg\nversion: 1.0.0\ndescription: A package with bad attestation.\nenvironment:\n sdk: ">=2.12.0 <4.0.0"\n'; + final archiveBytes = await packageArchiveBytes( + pubspecContent: pubspecContent, + ); + + // 1. Invalid non-UTF8 / tampered raw bytes + final rs1 = createPubApiClient(authToken: adminClientToken) + .uploadPackageBytes( + archiveBytes, + attestationBytes: [0xFF, 0xFE, 0xFD], + ); + await expectApiException( + rs1, + status: 400, + code: 'PackageRejected', + message: 'Invalid attestation bundle format', + ); + + // 2. Invalid non-JSON string + final rs2 = createPubApiClient(authToken: adminClientToken) + .uploadPackageBytes( + archiveBytes, + attestationBytes: utf8.encode('this is not json'), + ); + await expectApiException( + rs2, + status: 400, + code: 'PackageRejected', + message: 'Invalid attestation bundle format', + ); + + // 3. Non-object JSON + final rs3 = createPubApiClient(authToken: adminClientToken) + .uploadPackageBytes( + archiveBytes, + attestationBytes: utf8.encode('[1, 2, 3]'), + ); + await expectApiException( + rs3, + status: 400, + code: 'PackageRejected', + message: 'Invalid attestation bundle format', + ); + }, + ); + testWithProfile( + 'attestation upload fails with an invalid upload id', + fn: () async { + final rs = createPubApiClient(authToken: adminClientToken).client + .sendRaw( + verb: 'post', + path: '/api/packages/versions/newUploadAttestation/not-a-uuid', + headers: {'content-type': 'application/json; charset="utf-8"'}, + bodyBytes: utf8.encode('{}'), + ); + await expectApiException( + rs, + status: 400, + code: 'InvalidInput', + message: 'Invalid upload id.', + ); + }, + ); + + testWithProfile( + 'upload fails when attestation bundle is too large', + fn: () async { + final pubspecContent = + 'name: large_attested_pkg\nversion: 1.0.0\ndescription: A package with a large attestation.\nenvironment:\n sdk: ">=2.12.0 <4.0.0"\n'; + final archiveBytes = await packageArchiveBytes( + pubspecContent: pubspecContent, + ); + final attestationBytes = utf8.encode( + jsonEncode({'payload': 'x' * maxAttestationContentLength}), + ); + + final rs = createPubApiClient( + authToken: adminClientToken, + ).uploadPackageBytes(archiveBytes, attestationBytes: attestationBytes); + await expectApiException( + rs, + status: 400, + code: 'PackageRejected', + message: 'Attestation bundle is too large', + ); + }, + ); }); } diff --git a/pkg/_pub_shared/lib/data/package_api.dart b/pkg/_pub_shared/lib/data/package_api.dart index 1d40bcb372..f345180986 100644 --- a/pkg/_pub_shared/lib/data/package_api.dart +++ b/pkg/_pub_shared/lib/data/package_api.dart @@ -18,7 +18,17 @@ class UploadInfo { /// The fields the uploader should add to the multipart upload. final Map? fields; - UploadInfo({required this.url, required this.fields}); + /// The endpoint where an attestation of the uploaded package may be posted. + /// + /// The attestation is uploaded with a `POST` of the JSON bundle to + /// [attestationUrl], before the package archive is uploaded to [url]. + /// + /// If this is `null`, the repository does not support publishing with + /// attestations. + @JsonKey(includeIfNull: false) + final String? attestationUrl; + + UploadInfo({required this.url, required this.fields, this.attestationUrl}); factory UploadInfo.fromJson(Map json) => _$UploadInfoFromJson(json); diff --git a/pkg/_pub_shared/lib/data/package_api.g.dart b/pkg/_pub_shared/lib/data/package_api.g.dart index 8d18301bbe..7a075bc4b9 100644 --- a/pkg/_pub_shared/lib/data/package_api.g.dart +++ b/pkg/_pub_shared/lib/data/package_api.g.dart @@ -11,10 +11,15 @@ UploadInfo _$UploadInfoFromJson(Map json) => UploadInfo( fields: (json['fields'] as Map?)?.map( (k, e) => MapEntry(k, e as String), ), + attestationUrl: json['attestationUrl'] as String?, ); Map _$UploadInfoToJson(UploadInfo instance) => - {'url': instance.url, 'fields': instance.fields}; + { + 'url': instance.url, + 'fields': instance.fields, + 'attestationUrl': ?instance.attestationUrl, + }; PkgOptions _$PkgOptionsFromJson(Map json) => PkgOptions( isDiscontinued: json['isDiscontinued'] as bool?, diff --git a/pkg/_pub_shared/lib/src/pubapi.client.dart b/pkg/_pub_shared/lib/src/pubapi.client.dart index 4b8d5d009c..bce44f4fb8 100644 --- a/pkg/_pub_shared/lib/src/pubapi.client.dart +++ b/pkg/_pub_shared/lib/src/pubapi.client.dart @@ -31,6 +31,8 @@ class PubApiClient { final _i2.Client _client; + _i2.Client get client => _client; + Future> listVersions(String package) async { return await _client.requestBytes( verb: 'get', @@ -60,6 +62,16 @@ class PubApiClient { ); } + Future> getPackageVersionAttestation( + String package, + String version, + ) async { + return await _client.requestBytes( + verb: 'get', + path: '/api/packages/$package/versions/$version/attestation', + ); + } + Future> fetchPackage(String package, String version) async { return await _client.requestBytes( verb: 'get', @@ -76,6 +88,15 @@ class PubApiClient { ); } + Future<_i3.SuccessMessage> uploadPackageAttestation(String uploadId) async { + return _i3.SuccessMessage.fromJson( + await _client.requestJson( + verb: 'post', + path: '/api/packages/versions/newUploadAttestation/$uploadId', + ), + ); + } + Future<_i3.SuccessMessage> packageUploadCallback() async { return _i3.SuccessMessage.fromJson( await _client.requestJson( diff --git a/pkg/api_builder/lib/_client_utils.dart b/pkg/api_builder/lib/_client_utils.dart index b799b28119..a22f4ddd3a 100644 --- a/pkg/api_builder/lib/_client_utils.dart +++ b/pkg/api_builder/lib/_client_utils.dart @@ -85,6 +85,28 @@ class Client { } throw RequestException(res.statusCode, res.headers, res.bodyBytes); }); + + Future> sendRaw({ + required String verb, + required String path, + Map? query, + Map? headers, + List? bodyBytes, + }) => _withClient((client) async { + final u = Uri.parse(_baseUrl + path).replace(queryParameters: query); + final req = http.Request(verb, u); + if (headers != null) { + req.headers.addAll(headers); + } + if (bodyBytes != null) { + req.bodyBytes = bodyBytes; + } + final res = await http.Response.fromStream(await client.send(req)); + if (200 <= res.statusCode && res.statusCode < 300) { + return json.fuse(utf8).decode(res.bodyBytes) as Map; + } + throw RequestException(res.statusCode, res.headers, res.bodyBytes); + }); } /// Utility method exported for use in generated code. diff --git a/pkg/api_builder/lib/src/client_library_generator.dart b/pkg/api_builder/lib/src/client_library_generator.dart index 218d213bb9..f7f219a5d8 100644 --- a/pkg/api_builder/lib/src/client_library_generator.dart +++ b/pkg/api_builder/lib/src/client_library_generator.dart @@ -118,6 +118,19 @@ code.Class _buildClientClass(ClassElement cls, List handlers) => ), ), ) + ..methods.add( + code.Method( + (b) => b + ..name = 'client' + ..type = code.MethodType.getter + ..returns = code.refer( + 'Client', + 'package:api_builder/_client_utils.dart', + ) + ..lambda = true + ..body = code.refer('_client').code, + ), + ) ..methods.addAll( _removeDuplicateHandlers(handlers).map(_buildClientMethod), ), From 55e7be0b8540d1cbe66af89827c42f7b6e99cbac Mon Sep 17 00:00:00 2001 From: Moritz Date: Mon, 12 Oct 2026 12:48:38 +0000 Subject: [PATCH 2/2] Store attestations in the canonical bucket and export them Review follow-ups on the storage and serving of attestation bundles: - The attestation is canonical data (uploaded alongside the archive, not extracted from it), so `_performTarballUpload` copies it into the canonical bucket next to the archive, as `packages/-.tar.gz.sigstore.json`, and it is no longer stored as a `PackageVersionAsset`. `removePackageTarball` deletes both objects, `listVersionsInCanonicalBucket` filters by the `.tar.gz` suffix and `listAttestationsInCanonicalBucket` lists the bundles. - `ApiExporter.synchronizePackage` exports the attestations to the exported API bucket as `/api/packages//versions//attestation`, like the archives (shared implementation with `synchronizeTarballs`), including GC and deletion on moderation. Attestations are synchronized before the version listing is written, as clients fetch the attestation when the listing advertises one and do not retry. - The `GET .../attestation` handler is kept for hosts that do not route the path to the bucket; it now checks package and version visibility and serves the canonical copy. - The attestation upload handler reads the request body with a size cap (`foldBytes(maxSize:)`) instead of buffering it before checking. --- app/lib/frontend/handlers/pubapi.dart | 50 ++++- app/lib/package/api_export/api_exporter.dart | 26 ++- app/lib/package/api_export/exported_api.dart | 131 +++++++++++-- app/lib/package/backend.dart | 178 ++++++++++++++---- app/lib/package/models.dart | 1 - app/lib/shared/exceptions.dart | 9 + app/lib/shared/utils.dart | 17 +- .../package/api_export/exported_api_test.dart | 81 ++++++++ app/test/package/upload_test.dart | 155 +++++++++++++-- app/test/shared/utils_test.dart | 35 ++++ 10 files changed, 601 insertions(+), 82 deletions(-) diff --git a/app/lib/frontend/handlers/pubapi.dart b/app/lib/frontend/handlers/pubapi.dart index 5cf495af52..5082d065b3 100644 --- a/app/lib/frontend/handlers/pubapi.dart +++ b/app/lib/frontend/handlers/pubapi.dart @@ -9,6 +9,7 @@ import 'package:_pub_shared/data/package_api.dart'; import 'package:_pub_shared/data/publisher_api.dart'; import 'package:_pub_shared/data/task_api.dart'; import 'package:api_builder/api_builder.dart'; +import 'package:chunked_stream/chunked_stream.dart' show MaximumSizeExceeded; import 'package:pub_dev/database/database.dart'; import 'package:pub_dev/frontend/handlers/atom_feed.dart'; import 'package:shelf/shelf.dart'; @@ -17,11 +18,11 @@ import 'package:shelf_router/shelf_router.dart'; import '../../account/consent_backend.dart'; import '../../admin/backend.dart'; import '../../package/backend.dart' hide InviteStatus; -import '../../package/models.dart' show AssetKind; import '../../publisher/backend.dart'; import '../../shared/exceptions.dart'; import '../../shared/handlers.dart'; import '../../shared/urls.dart' as urls; +import '../../shared/utils.dart' show canonicalizeVersion, ByteFolderExt; import '../../task/backend.dart' show taskBackend; import 'account.dart'; import 'cache_control.dart'; @@ -82,6 +83,12 @@ class PubApi { } /// Fetches the Sigstore attestation bundle for a specific (package, version) pair. + /// https://github.com/dart-lang/pub/blob/master/doc/repository-spec-v2.md#fetching-attestations + /// + /// The attestation is also exported to the exported API bucket (see + /// `ExportedPackage.attestation`), from where it is meant to be served + /// directly, like the version listing and the archives. This handler + /// serves the canonical copy for hosts that do not route the path there. @EndPoint.get('/api/packages//versions//attestation') Future getPackageVersionAttestation( Request request, @@ -89,18 +96,30 @@ class PubApi { String version, ) async { checkPackageVersionParams(package, version); - final asset = await packageBackend.lookupPackageVersionAsset( + final canonicalVersion = canonicalizeVersion(version); + InvalidInputException.checkSemanticVersion(canonicalVersion); + // Moderated or deleted packages and versions must not expose their + // attestation either. + final pv = await packageBackend.lookupPackageVersion( + package, + canonicalVersion!, + ); + if (pv == null || + pv.isNotVisible || + !await packageBackend.isPackageVisible(package)) { + throw NotFoundException.resource('version "$package $version"'); + } + final bytes = await packageBackend.readCanonicalBucketAttestation( package, - version, - AssetKind.attestation, + canonicalVersion, ); - if (asset == null || asset.textContent == null) { + if (bytes == null) { throw NotFoundException.resource('attestation for $package $version'); } return Response.ok( - asset.textContent, + bytes, headers: { - 'content-type': 'application/json; charset="utf-8"', + 'content-type': attestationContentType, ...CacheControl.clientApi.headers, }, ); @@ -171,8 +190,8 @@ class PubApi { /// Upload an attestation for a package archive that is being uploaded. /// - /// The attestation is uploaded before the package archive, and is verified - /// and stored when the upload is finished. + /// The attestation is uploaded before the package archive, and is stored + /// next to the archive when the upload is finished. /// https://github.com/dart-lang/pub/blob/master/doc/repository-spec-v2.md#publishing-with-an-attestation /// /// POST /api/packages/versions/newUploadAttestation/ @@ -188,7 +207,18 @@ class PubApi { Request request, String uploadId, ) async { - final bytes = await request.read().expand((i) => i).toList(); + final List bytes; + try { + // Stop reading as soon as the body is too large, instead of buffering + // the whole request before checking its size. + bytes = await request.read().foldBytes( + maxSize: maxAttestationContentLength, + ); + } on MaximumSizeExceeded { + throw PackageRejectedException.attestationTooLarge( + maxAttestationContentLength, + ); + } if (bytes.isEmpty) { throw PackageRejectedException( 'Invalid attestation bundle format: the request body is empty.', diff --git a/app/lib/package/api_export/api_exporter.dart b/app/lib/package/api_export/api_exporter.dart index 0e8d6b2b3c..63e627c8fa 100644 --- a/app/lib/package/api_export/api_exporter.dart +++ b/app/lib/package/api_export/api_exporter.dart @@ -195,7 +195,8 @@ final class ApiExporter { /// * Version listing for [package] is up-to-date, /// * Advisories for [package] is up-to-date, /// * Tarballs for each version of [package] is up-to-date, - /// * Delete tarballs from old versions that no-longer exist. + /// * Attestations for each attested version of [package] is up-to-date, + /// * Delete tarballs and attestations from old versions that no-longer exist. /// /// This is intended when: /// * Running a full background synchronization. @@ -238,15 +239,14 @@ final class ApiExporter { ); if (!skipArchives) { - final versions = await packageBackend.listVersionsInCanonicalBucket( - package, - ); + final (:versions, :attestations) = await packageBackend + .listVersionsAndAttestationsInCanonicalBucket(package); // Remove versions that are not exposed in the public API. - versions.removeWhere( - (version, _) => - !versionListing.versions.any((v) => v.version == version), - ); + bool isListed(String version) => + versionListing.versions.any((v) => v.version == version); + versions.removeWhere((version, _) => !isListed(version)); + attestations.removeWhere((version, _) => !isListed(version)); await _api .package(package) @@ -255,6 +255,16 @@ final class ApiExporter { forceWrite: forceWrite, forceDelete: forceDelete, ); + // Note: this must happen before the version listing is written below, + // as clients fetch the attestation of a version when the listing + // says there is one (and fail the download if it is missing). + await _api + .package(package) + .synchronizeAttestations( + attestations, + forceWrite: forceWrite, + forceDelete: forceDelete, + ); } await _api diff --git a/app/lib/package/api_export/exported_api.dart b/app/lib/package/api_export/exported_api.dart index 1d3c4cd33c..1a849b17c2 100644 --- a/app/lib/package/api_export/exported_api.dart +++ b/app/lib/package/api_export/exported_api.dart @@ -309,6 +309,26 @@ final class ExportedPackage { Duration(hours: 2), ); + /// Interface for writing + /// `/api/packages//versions//attestation`. + /// + /// This is the attestation bundle that was uploaded with the archive of + /// [version], served verbatim. + ExportedBlob attestation(String version) => ExportedBlob._( + _owner, + _attestationObjectName(version), + null, + 'application/json; charset="utf-8"', + Duration(hours: 2), + ); + + /// The object name prefix (without the `latest/` or `/` + /// prefix) that all attestations of this package share. + String get _attestationsPrefix => '/api/packages/$_package/versions/'; + + String _attestationObjectName(String version) => + '$_attestationsPrefix$version$_attestationSuffix'; + /// Synchronize tarballs from [versions]. /// /// [versions] is a map from version number to [SourceObjectInfo], where @@ -327,10 +347,58 @@ final class ExportedPackage { Map versions, { bool forceWrite = false, bool forceDelete = false, + }) async { + await _synchronizeVersionedBlobs( + versions, + pathPrefix: '/api/archives/$_package-', + suffix: '.tar.gz', + blob: tarball, + forceWrite: forceWrite, + forceDelete: forceDelete, + ); + } + + /// Synchronize attestations from [versions]. + /// + /// [versions] is a map from version number to [SourceObjectInfo], where + /// the [SourceObjectInfo] is the GCS object from which the attestation can + /// be copied. Versions without an attestation have no entry. + /// + /// This will **delete** attestations for versions that do not exist in + /// [versions], otherwise this behaves like [synchronizeTarballs]. + /// + /// Note: this must complete before a version listing advertising the + /// attestations is written, as clients fetch an attestation when the + /// listing says there is one, and do not retry if it is missing. + Future synchronizeAttestations( + Map versions, { + bool forceWrite = false, + bool forceDelete = false, + }) async { + await _synchronizeVersionedBlobs( + versions, + pathPrefix: _attestationsPrefix, + suffix: _attestationSuffix, + blob: attestation, + forceWrite: forceWrite, + forceDelete: forceDelete, + ); + } + + /// Synchronize the objects under [pathPrefix] (in each of the prefixes) with + /// [versions], where the object of a version is named + /// `` and is created by [blob]. + Future _synchronizeVersionedBlobs( + Map versions, { + required String pathPrefix, + required String suffix, + required ExportedBlob Function(String version) blob, + required bool forceWrite, + required bool forceDelete, }) async { await Future.wait([ ..._owner._prefixes.map((prefix) async { - final pfx = '$prefix/api/archives/$_package-'; + final pfx = '$prefix$pathPrefix'; final versionsForUpload = versions.keys.toSet(); await _owner._listBucket(prefix: pfx, delimiter: '', (entry) async { @@ -338,7 +406,7 @@ final class ExportedPackage { final BucketDirectoryEntry _ => throw AssertionError('unreachable'), final BucketObjectEntry item => item, }; - if (!item.name.endsWith('.tar.gz')) { + if (!item.name.endsWith(suffix)) { _log.pubNoticeShout( 'stray-file', 'Found stray file "${item.name}" in ExportedApi' @@ -346,11 +414,11 @@ final class ExportedPackage { ); return; } - final version = item.name.without(prefix: pfx, suffix: '.tar.gz'); + final version = item.name.without(prefix: pfx, suffix: suffix); final info = versions[version]; if (info != null && !forceWrite) { - await tarball( + await blob( version, )._copyToPrefixFromIfNotContentEquals(prefix, info, item); // This version needs not be uploaded again @@ -373,7 +441,7 @@ final class ExportedPackage { await Future.wait( versionsForUpload.map((v) async { await _owner._pool.withResource(() async { - await tarball(v)._copyToPrefixFromIfNotContentEquals( + await blob(v)._copyToPrefixFromIfNotContentEquals( prefix, versions[v]!, null, @@ -390,12 +458,33 @@ final class ExportedPackage { /// /// [allVersionNumbers] must be encoded as canonical versions. Future garbageCollect(Set allVersionNumbers) async { + await Future.wait([ + _garbageCollectVersionedBlobs( + allVersionNumbers, + pathPrefix: '/api/archives/$_package-', + suffix: '.tar.gz', + ), + _garbageCollectVersionedBlobs( + allVersionNumbers, + pathPrefix: _attestationsPrefix, + suffix: _attestationSuffix, + ), + ]); + } + + /// Garbage collect the objects under [pathPrefix] (in each of the prefixes) + /// whose version is not in [allVersionNumbers]. + Future _garbageCollectVersionedBlobs( + Set allVersionNumbers, { + required String pathPrefix, + required String suffix, + }) async { await Future.wait([ ..._owner._prefixes.map((prefix) async { - final pfx = prefix + '/api/archives/$_package-'; + final pfx = '$prefix$pathPrefix'; await _owner._listBucket(prefix: pfx, delimiter: '', (item) async { assert(item.isObject); - if (!item.name.endsWith('.tar.gz')) { + if (!item.name.endsWith(suffix)) { _log.pubNoticeShout( 'stray-file', 'Found stray file "${item.name}" in ExportedApi' @@ -403,7 +492,7 @@ final class ExportedPackage { ); return; } - final version = item.name.without(prefix: pfx, suffix: '.tar.gz'); + final version = item.name.without(prefix: pfx, suffix: suffix); if (allVersionNumbers.contains(version)) { return; } @@ -442,10 +531,24 @@ final class ExportedPackage { }, ); }), + ..._owner._prefixes.map((prefix) async { + await _owner._listBucket( + prefix: prefix + _attestationsPrefix, + delimiter: '', + (item) async { + _log.shout('Deleting object from public bucket: "${item.name}".'); + await _owner._bucket.deleteWithRetry(item.name); + }, + ); + }), ]); } } +/// The suffix of the object name of an exported attestation, after the +/// version. +const _attestationSuffix = '/attestation'; + /// Information about an object to be used as source in a copy operation. /// /// The [absoluteObjectName] must be a `gs://` style URL. @@ -607,12 +710,15 @@ final class ExportedAtomFeedFile extends ExportedObject { /// /// This will write a binary blob as is, adding headers for /// * `Content-Type`, -/// * `Content-Disposition`, and, +/// * `Content-Disposition` (if a filename is given), and, /// * `Cache-Control`. final class ExportedBlob extends ExportedObject { final String _contentType; final Duration _maxAge; - final String _filename; + + /// The filename for the `Content-Disposition: attachment` header, or `null` + /// to serve the blob inline. + final String? _filename; ExportedBlob._( super._owner, @@ -623,10 +729,13 @@ final class ExportedBlob extends ExportedObject { ) : super._(); ObjectMetadata _metadata() { + final filename = _filename; return ObjectMetadata( contentType: _contentType, cacheControl: 'public, max-age=${_maxAge.inSeconds}', - contentDisposition: 'attachment; filename="$_filename"', + contentDisposition: filename == null + ? null + : 'attachment; filename="$filename"', custom: {_validatedCustomHeader: clock.now().toIso8601String()}, ); } diff --git a/app/lib/package/backend.dart b/app/lib/package/backend.dart index 2a729adb1a..67de342840 100644 --- a/app/lib/package/backend.dart +++ b/app/lib/package/backend.dart @@ -65,10 +65,14 @@ final maxAssetContentLength = 256 * 1024; // The maximum length of an attestation bundle. // -// Note: unlike other assets, attestations are rejected when they are longer -// than this, as a truncated attestation could never be verified. +// Note: unlike the assets extracted from an archive, attestations are rejected +// (rather than truncated) when they are longer than this, as a truncated +// attestation could never be verified. final maxAttestationContentLength = 128 * 1024; +/// The `Content-Type` of an attestation bundle, as stored and as served. +const attestationContentType = 'application/json; charset="utf-8"'; + /// The maximum number of versions a package is allowed to have. final _defaultMaxVersionsPerPackage = 1000; @@ -1312,8 +1316,8 @@ class PackageBackend { /// Stores the attestation [bytes] uploaded for the upload with [uploadGuid]. /// - /// The attestation is verified and stored with the package version when the - /// upload is finished, see [publishUploadedBlob]. + /// The attestation is stored next to the archive in the canonical bucket + /// when the upload is finished, see [publishUploadedBlob]. Future uploadAttestation(String uploadGuid, List bytes) async { final restriction = await getUploadRestrictionStatus(); if (restriction == UploadRestrictionStatus.noUploads) { @@ -1325,9 +1329,8 @@ class PackageBackend { 'Invalid upload id.', ); if (bytes.length > maxAttestationContentLength) { - throw PackageRejectedException( - 'Attestation bundle is too large ' - '(max $maxAttestationContentLength bytes).', + throw PackageRejectedException.attestationTooLarge( + maxAttestationContentLength, ); } // Verify that the bundle is a JSON object before storing it. It can only be @@ -1347,6 +1350,7 @@ class PackageBackend { tmpAttestationObjectName(uploadGuid), bytes.length, () => Stream.value(bytes), + metadata: ObjectMetadata(contentType: attestationContentType), ); } @@ -1512,7 +1516,6 @@ class PackageBackend { agent, archive, sha256Hash: sha256Hash, - attestationContent: attestationContent, ); final (version, uploadMessages) = await _performTarballUpload( entities: entities, @@ -1521,6 +1524,9 @@ class PackageBackend { objectName: workObjectName, hasCanonicalArchiveObject: canonicalContentMatch == ContentMatchStatus.same, + attestationObjectName: attestationContent == null + ? null + : attestationObjectName, ); _logger.info('Tarball uploaded in ${sw.elapsed}.'); _logger.info('Removing temporary object $uploadGuid.'); @@ -1612,12 +1618,19 @@ class PackageBackend { } } + /// Stores the uploaded archive (and its attestation, if any) in the + /// canonical bucket and creates the Datastore entities of the new version. + /// + /// [attestationObjectName] is the object in the incoming bucket that holds + /// the attestation uploaded for the archive, or `null` when the archive was + /// published without one. Future<(PackageVersion, List)> _performTarballUpload({ required _UploadEntities entities, required AuthenticatedAgent agent, required PackageSummary archive, required String objectName, required bool hasCanonicalArchiveObject, + required String? attestationObjectName, }) async { final sw = Stopwatch()..start(); final uploadMessages = []; @@ -1779,6 +1792,21 @@ class PackageBackend { version: newVersion.version!, ); } + if (attestationObjectName != null) { + // The attestation is uploaded alongside the archive, not extracted + // from it, so it is canonical data too: keep it next to the archive. + // The copy is repeated even if the archive was already present from an + // earlier attempt of this very upload, it is for the same archive. + await _storage.copyObjectWithRetry( + _incomingBucket.absoluteObjectName(attestationObjectName), + _canonicalBucket.absoluteObjectName( + canonicalAttestationObjectName( + newVersion.package, + newVersion.version!, + ), + ), + ); + } final ghPayload = agent is AuthenticatedGitHubAction ? agent.payload @@ -2352,10 +2380,13 @@ class PackageBackend { return UploadRestrictionStatus.noRestriction; } - /// Deletes the tarball of a [package] in the given [version] permanently from the canonical bucket. + /// Deletes the tarball of a [package] in the given [version] permanently from + /// the canonical bucket, together with its attestation (if any). Future removePackageTarball(String package, String version) async { - final objectName = tarballObjectName(package, version); - await _canonicalBucket.deleteWithRetry(objectName); + await _canonicalBucket.deleteWithRetry(tarballObjectName(package, version)); + await _canonicalBucket.deleteWithRetry( + canonicalAttestationObjectName(package, version), + ); } void _updatePackageAutomatedPublishingLock( @@ -2391,6 +2422,36 @@ class PackageBackend { return await _canonicalBucket.tryInfo(objectName); } + /// Gets the object info of the attestation of [version] from the canonical + /// bucket, or `null` if the version was published without an attestation. + Future getCanonicalBucketAttestationInfo( + String package, + String version, + ) async { + return await _canonicalBucket.tryInfo( + canonicalAttestationObjectName(package, version), + ); + } + + /// Reads the attestation of [version] from the canonical bucket, or returns + /// `null` if the version was published without an attestation. + /// + /// Note: this does not check the visibility of the package or the version, + /// callers that serve the content must do that. + Future?> readCanonicalBucketAttestation( + String package, + String version, + ) async { + final objectName = canonicalAttestationObjectName(package, version); + if (await _canonicalBucket.tryInfo(objectName) == null) { + return null; + } + return await _canonicalBucket.readAsBytes( + objectName, + maxSize: maxAttestationContentLength, + ); + } + /// Gets `gs://` for [package] and [version] in the /// canonical bucket. /// @@ -2419,20 +2480,61 @@ class PackageBackend { Future> listVersionsInCanonicalBucket( String package, ) async { + return (await listVersionsAndAttestationsInCanonicalBucket( + package, + )).versions; + } + + /// Get map from `version` to [SourceObjectInfo] for each version of [package] + /// that has an attestation in the canonical bucket. + Future> listAttestationsInCanonicalBucket( + String package, + ) async { + return (await listVersionsAndAttestationsInCanonicalBucket( + package, + )).attestations; + } + + /// Lists the archives and the attestations of [package] in the canonical + /// bucket, each as a map from `version` to [SourceObjectInfo]. + /// + /// The archive and the attestation of a version share the + /// `packages/-` name prefix, so both come from a single + /// listing and are told apart by their suffix. + Future< + ({ + Map versions, + Map attestations, + }) + > + listVersionsAndAttestationsInCanonicalBucket(String package) async { final prefix = _tarballObjectNamePackagePrefix(package); final items = await _canonicalBucket.listAllItemsWithRetry( prefix: prefix, delimiter: '', ); - return Map.fromEntries( - items.whereType().map((item) { - final version = item.name.without(prefix: prefix, suffix: '.tar.gz'); - return MapEntry( - version, - SourceObjectInfo.fromObjectInfo(_canonicalBucket, item), + final versions = {}; + final attestations = {}; + for (final item in items.whereType()) { + final name = item.name; + final info = SourceObjectInfo.fromObjectInfo(_canonicalBucket, item); + if (name.endsWith(_attestationSuffix)) { + final version = name.without( + prefix: prefix, + suffix: _attestationSuffix, ); - }), - ); + attestations[version] = info; + } else if (name.endsWith(_tarballSuffix)) { + final version = name.without(prefix: prefix, suffix: _tarballSuffix); + versions[version] = info; + } else { + _logger.pubNoticeShout( + 'stray-file', + 'Found stray file "$name" in the canonical bucket (ignoring it!)', + ); + } + } + return (versions: versions, attestations: attestations); } /// Gets the object info of the archive file from the exported API bucket. @@ -2444,6 +2546,16 @@ class PackageBackend { return await _exportedApiBucket.tryInfo(objectName); } + /// Gets the object info of the attestation from the exported API bucket. + Future getExportedAttestationInfo( + String package, + String version, + ) async { + final objectName = + 'latest/api/packages/$package/versions/$version/attestation'; + return await _exportedApiBucket.tryInfo(objectName); + } + /// Verifies the content of an archive in the canonical bucket. Future matchArchiveContentInCanonical( String package, @@ -2706,7 +2818,6 @@ Future<_UploadEntities> _createUploadEntities( AuthenticatedAgent agent, PackageSummary archive, { required List sha256Hash, - String? attestationContent, }) async { final pubspec = Pubspec.fromYaml(archive.pubspecContent!); final packageKey = db.emptyKey.append(Package, id: pubspec.name); @@ -2726,7 +2837,6 @@ Future<_UploadEntities> _createUploadEntities( final derived = derivePackageVersionEntities( archive: archive, versionCreated: version.created!, - attestationContent: attestationContent, ); // TODO: verify if assets sizes are within the transaction limit (10 MB) @@ -2737,7 +2847,6 @@ Future<_UploadEntities> _createUploadEntities( DerivedPackageVersionEntities derivePackageVersionEntities({ required PackageSummary archive, required DateTime versionCreated, - String? attestationContent, }) { final pubspec = Pubspec.fromYaml(archive.pubspecContent!); final key = QualifiedVersionKey( @@ -2796,17 +2905,6 @@ DerivedPackageVersionEntities derivePackageVersionEntities({ path: archive.licensePath, textContent: capContent(archive.licenseContent), ), - if (attestationContent != null) - PackageVersionAsset.init( - package: key.package, - version: key.version, - kind: AssetKind.attestation, - versionCreated: versionCreated, - path: '${key.package}-${key.version}.sigstore.json', - // Note: not capped, a truncated attestation could never be verified. - // The length is checked when the attestation is uploaded. - textContent: attestationContent, - ), ]; final versionInfo = PackageVersionInfo() @@ -2965,9 +3063,21 @@ class _VersionTransactionDataAcccess { /// The GCS object name of a tarball object - excluding leading '/'. @visibleForTesting String tarballObjectName(String package, String version) => - '${_tarballObjectNamePackagePrefix(package)}$version.tar.gz'; + '${_tarballObjectNamePackagePrefix(package)}$version$_tarballSuffix'; + +/// The GCS object name of the attestation of a tarball object in the +/// canonical bucket - excluding leading '/'. +/// +/// The name follows the Sigstore convention of naming the bundle after the +/// artifact it attests to: `packages/-.tar.gz.sigstore.json`. +@visibleForTesting +String canonicalAttestationObjectName(String package, String version) => + '${_tarballObjectNamePackagePrefix(package)}$version$_attestationSuffix'; /// The GCS object prefix of a tarball object for a given [package] - excluding leading '/'. String _tarballObjectNamePackagePrefix(String package) => 'packages/$package-'; +const _tarballSuffix = '.tar.gz'; +const _attestationSuffix = '$_tarballSuffix.sigstore.json'; + enum ContentMatchStatus { missing, different, same } diff --git a/app/lib/package/models.dart b/app/lib/package/models.dart index f984cb1b90..3d244062ba 100644 --- a/app/lib/package/models.dart +++ b/app/lib/package/models.dart @@ -761,7 +761,6 @@ abstract class AssetKind { static const changelog = 'changelog'; static const example = 'example'; static const license = 'license'; - static const attestation = 'attestation'; } /// A derived entity that holds extracted asset of a [PackageVersion] archive. diff --git a/app/lib/shared/exceptions.dart b/app/lib/shared/exceptions.dart index 1cc70b068a..c378ef723f 100644 --- a/app/lib/shared/exceptions.dart +++ b/app/lib/shared/exceptions.dart @@ -240,6 +240,15 @@ class PackageRejectedException extends ResponseException { PackageRejectedException.archiveTooLarge(int limit) : super._(400, 'PackageRejected', 'Package archive exceeded $limit bytes.'); + /// The attestation bundle uploaded for a package archive is above [limit] + /// bytes. + PackageRejectedException.attestationTooLarge(int limit) + : super._( + 400, + 'PackageRejected', + 'Attestation bundle is too large (max $limit bytes).', + ); + /// The package archive tar.gz file is empty, possibly an error happened /// during the upload or the system was unable to create an archive. PackageRejectedException.archiveEmpty() diff --git a/app/lib/shared/utils.dart b/app/lib/shared/utils.dart index e866399530..62fcffd2e1 100644 --- a/app/lib/shared/utils.dart +++ b/app/lib/shared/utils.dart @@ -9,6 +9,7 @@ import 'dart:io'; import 'dart:math'; import 'dart:typed_data'; +import 'package:chunked_stream/chunked_stream.dart' show MaximumSizeExceeded; import 'package:clock/clock.dart'; import 'package:intl/intl.dart'; // ignore: implementation_imports @@ -302,11 +303,21 @@ extension StringExt on String { } extension ByteFolderExt on Stream> { - Future foldBytes() async { - final contents = await toList(); + /// Collects the stream into a single byte array. + /// + /// If [maxSize] is given, reading stops and a [MaximumSizeExceeded] is + /// thrown as soon as more than [maxSize] bytes have arrived, instead of + /// buffering the rest of the stream. + Future foldBytes({int? maxSize}) async { + if (maxSize != null && maxSize < 0) { + throw ArgumentError.value(maxSize, 'maxSize must be positive, if given'); + } final buffer = BytesBuilder(copy: false); - for (final chunk in contents) { + await for (final chunk in this) { buffer.add(chunk); + if (maxSize != null && buffer.length > maxSize) { + throw MaximumSizeExceeded(maxSize); + } } return buffer.toBytes(); } diff --git a/app/test/package/api_export/exported_api_test.dart b/app/test/package/api_export/exported_api_test.dart index c9d59842cf..a4a06a7da8 100644 --- a/app/test/package/api_export/exported_api_test.dart +++ b/app/test/package/api_export/exported_api_test.dart @@ -432,6 +432,87 @@ void main() { ); }, ); + + testWithProfile( + 'ExportedApi.package().synchronizeAttestations()', + expectedLogMessages: [ + // stale version removed by synchronizeAttestations() + RegExp( + r'SHOUT Deleting object from public bucket: ".*/api/packages/retry/versions/1.0.5/attestation".', + ), + // removed by garbageCollect() + RegExp( + r'SHOUT Deleting object from public bucket: ".*/api/packages/retry/versions/2.0.0/attestation".', + ), + // removed by delete() + RegExp( + r'SHOUT Deleting object from public bucket: ".*/api/packages/retry/versions/1.0.0/attestation".', + ), + ], + fn: () async { + await storageService.createBucket('exported-api'); + final bucket = storageService.bucket('exported-api'); + final exportedApi = ExportedApi(storageService, bucket); + + await storageService.createBucket('canonical-packages'); + final canonical = storageService.bucket('canonical-packages'); + + final src1 = SourceObjectInfo.fromObjectInfo( + canonical, + await canonical.writeBytes( + 'packages/retry-1.0.0.tar.gz.sigstore.json', + [1, 0, 0], + ), + ); + final src2 = SourceObjectInfo.fromObjectInfo( + canonical, + await canonical.writeBytes( + 'packages/retry-2.0.0.tar.gz.sigstore.json', + [2, 0, 0], + ), + ); + + await exportedApi.package('retry').attestation('1.0.0').copyFrom(src1); + await exportedApi.package('retry').attestation('1.0.5').copyFrom(src2); + + const prefix = 'latest/api/packages/retry/versions'; + expect(await bucket.readBytes('$prefix/1.0.0/attestation'), [1, 0, 0]); + expect(await bucket.readBytes('$prefix/1.0.5/attestation'), [2, 0, 0]); + final info = await bucket.info('$prefix/1.0.0/attestation'); + expect(info.metadata.contentType, 'application/json; charset="utf-8"'); + expect(info.metadata.contentDisposition, isNull); + + await exportedApi.package('retry').synchronizeAttestations({ + '1.0.0': src1, + '2.0.0': src2, + }); + + expect(await bucket.readBytes('$prefix/1.0.0/attestation'), [1, 0, 0]); + // Too young to be deleted. + expect(await bucket.readBytes('$prefix/1.0.5/attestation'), [2, 0, 0]); + expect(await bucket.readBytes('$prefix/2.0.0/attestation'), [2, 0, 0]); + + clockControl.elapse(days: 2); + + await exportedApi.package('retry').synchronizeAttestations({ + '1.0.0': src1, + '2.0.0': src2, + }); + expect(await bucket.readBytes('$prefix/1.0.0/attestation'), [1, 0, 0]); + expect(await bucket.readBytes('$prefix/1.0.5/attestation'), isNull); + expect(await bucket.readBytes('$prefix/2.0.0/attestation'), [2, 0, 0]); + + // Garbage collecting versions removes attestations too. + clockControl.elapse(days: 2); + await exportedApi.package('retry').garbageCollect({'1.0.0'}); + expect(await bucket.readBytes('$prefix/1.0.0/attestation'), [1, 0, 0]); + expect(await bucket.readBytes('$prefix/2.0.0/attestation'), isNull); + + // Deleting the package removes the remaining attestations. + await exportedApi.package('retry').delete(); + expect(await bucket.readBytes('$prefix/1.0.0/attestation'), isNull); + }, + ); } extension on Bucket { diff --git a/app/test/package/upload_test.dart b/app/test/package/upload_test.dart index 8530a63fe6..b7c9380d8e 100644 --- a/app/test/package/upload_test.dart +++ b/app/test/package/upload_test.dart @@ -5,6 +5,7 @@ import 'dart:async'; import 'dart:convert'; +import 'package:_pub_shared/data/admin_api.dart'; import 'package:_pub_shared/data/package_api.dart'; import 'package:clock/clock.dart'; import 'package:gcloud/db.dart'; @@ -1713,6 +1714,14 @@ void main() { testWithProfile( 'successful upload with attestation bundle and api retrieval', + expectedLogMessages: [ + RegExp( + r'SHOUT Deleting object from public bucket: ".*/api/archives/attested_pkg-1.0.0.tar.gz".', + ), + RegExp( + r'SHOUT Deleting object from public bucket: ".*/api/packages/attested_pkg/versions/1.0.0/attestation".', + ), + ], fn: () async { final pubspecContent = 'name: attested_pkg\nversion: 1.0.0\ndescription: A package with attestation.\nenvironment:\n sdk: ">=2.12.0 <4.0.0"\n'; @@ -1728,40 +1737,156 @@ void main() { 'signatures': [], }, }; - final attestationBytes = utf8.encode(jsonEncode(bundleJson)); + // Verbatim is what the spec promises: keep the odd formatting. + final attestationBytes = utf8.encode( + ' ${jsonEncode(bundleJson).replaceAll(',', ',\n ')}\n', + ); final client = createPubApiClient(authToken: adminClientToken); + // A moderated version must not be the only one, see below. + await client.uploadPackageBytes( + await packageArchiveBytes( + pubspecContent: pubspecContent.replaceFirst('1.0.0', '0.9.0'), + ), + ); final message = await client.uploadPackageBytes( archiveBytes, attestationBytes: attestationBytes, ); expect(message.success.message, contains('Successfully uploaded')); + await asyncQueue.ongoingProcessing; - // Verify attestation asset was stored in Datastore - final asset = await packageBackend.lookupPackageVersionAsset( + // The attestation is canonical data: it is stored next to the archive. + expect( + await packageBackend.getCanonicalBucketAttestationInfo( + 'attested_pkg', + '1.0.0', + ), + isNotNull, + ); + expect( + await packageBackend.listAttestationsInCanonicalBucket( + 'attested_pkg', + ), + {'1.0.0': isNotNull}, + ); + // ... and does not show up as a version. + expect( + await packageBackend.listVersionsInCanonicalBucket('attested_pkg'), + {'0.9.0': isNotNull, '1.0.0': isNotNull}, + ); + // It is not a derived asset. + final info = await packageBackend.lookupPackageVersionInfo( 'attested_pkg', '1.0.0', - AssetKind.attestation, ); - expect(asset, isNotNull); - expect(asset!.textContent, isNotNull); - final storedJson = - jsonDecode(asset.textContent!) as Map; + expect(info!.assets, isNot(contains('attestation'))); + + // The attestation is exported, to be served from the bucket. + final exported = await packageBackend.getExportedAttestationInfo( + 'attested_pkg', + '1.0.0', + ); + expect(exported, isNotNull); + expect(exported!.length, attestationBytes.length); expect( - storedJson['mediaType'], - equals('application/vnd.dev.sigstore.bundle.v0.3+json'), + exported.metadata.contentType, + 'application/json; charset="utf-8"', ); + expect(exported.metadata.contentDisposition, isNull); - // Verify attestation can be retrieved via the API endpoint + // Verify attestation can be retrieved verbatim via the API endpoint. final retrievedBytes = await client.getPackageVersionAttestation( 'attested_pkg', '1.0.0', ); - final retrievedJson = - jsonDecode(utf8.decode(retrievedBytes)) as Map; + expect(retrievedBytes, attestationBytes); + + // Moderated versions must not expose their attestation. + await createPubApiClient(authToken: siteAdminToken).adminInvokeAction( + 'moderate-package-version', + AdminInvokeActionArguments( + arguments: { + 'case': 'none', + 'package': 'attested_pkg', + 'version': '1.0.0', + 'state': 'true', + }, + ), + ); + await asyncQueue.ongoingProcessing; + await expectApiException( + client.getPackageVersionAttestation('attested_pkg', '1.0.0'), + status: 404, + code: 'NotFound', + ); + // The export is removed, the canonical copy is kept (like the archive). expect( - retrievedJson['mediaType'], - equals('application/vnd.dev.sigstore.bundle.v0.3+json'), + await packageBackend.getExportedAttestationInfo( + 'attested_pkg', + '1.0.0', + ), + isNull, + ); + expect( + await packageBackend.getCanonicalBucketAttestationInfo( + 'attested_pkg', + '1.0.0', + ), + isNotNull, + ); + }, + ); + + testWithProfile( + 'attestation of a moderated package is not exposed', + expectedLogMessages: [ + RegExp( + r'SHOUT Deleting object from public bucket: ".*/api/archives/attested_pkg-1.0.0.tar.gz".', + ), + RegExp( + r'SHOUT Deleting object from public bucket: ".*/api/packages/attested_pkg/versions/1.0.0/attestation".', + ), + ], + fn: () async { + final pubspecContent = + 'name: attested_pkg\nversion: 1.0.0\ndescription: A package with attestation.\nenvironment:\n sdk: ">=2.12.0 <4.0.0"\n'; + final archiveBytes = await packageArchiveBytes( + pubspecContent: pubspecContent, + ); + final client = createPubApiClient(authToken: adminClientToken); + await client.uploadPackageBytes( + archiveBytes, + attestationBytes: utf8.encode('{"mediaType": "test"}'), + ); + await asyncQueue.ongoingProcessing; + expect( + await client.getPackageVersionAttestation('attested_pkg', '1.0.0'), + utf8.encode('{"mediaType": "test"}'), + ); + + await createPubApiClient(authToken: siteAdminToken).adminInvokeAction( + 'moderate-package', + AdminInvokeActionArguments( + arguments: { + 'case': 'none', + 'package': 'attested_pkg', + 'state': 'true', + }, + ), + ); + await asyncQueue.ongoingProcessing; + await expectApiException( + client.getPackageVersionAttestation('attested_pkg', '1.0.0'), + status: 404, + code: 'NotFound', + ); + expect( + await packageBackend.getExportedAttestationInfo( + 'attested_pkg', + '1.0.0', + ), + isNull, ); }, ); diff --git a/app/test/shared/utils_test.dart b/app/test/shared/utils_test.dart index 258ee1b0e7..8f247f0334 100644 --- a/app/test/shared/utils_test.dart +++ b/app/test/shared/utils_test.dart @@ -2,6 +2,7 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +import 'package:chunked_stream/chunked_stream.dart' show MaximumSizeExceeded; import 'package:clock/clock.dart'; import 'package:pub_dev/shared/utils.dart'; import 'package:pub_semver/pub_semver.dart'; @@ -100,4 +101,38 @@ void main() { expect(tracker.getLatency().inMilliseconds, greaterThan(15000)); }); }); + + group('foldBytes', () { + Stream> chunks() => Stream.fromIterable([ + [1, 2, 3], + [4, 5], + [6], + ]); + + test('collects all chunks', () async { + expect(await chunks().foldBytes(), [1, 2, 3, 4, 5, 6]); + expect(await chunks().foldBytes(maxSize: 6), [1, 2, 3, 4, 5, 6]); + }); + + test('stops reading when maxSize is exceeded', () async { + var chunksRead = 0; + final counted = chunks().map((c) { + chunksRead++; + return c; + }); + await expectLater( + counted.foldBytes(maxSize: 4), + throwsA(isA()), + ); + // The limit is exceeded by the second chunk, the third is never read. + expect(chunksRead, 2); + }); + + test('rejects negative maxSize', () async { + await expectLater( + chunks().foldBytes(maxSize: -1), + throwsA(isA()), + ); + }); + }); }