Skip to content

Commit

Permalink
Merge branch 'master' into cleanupIsModeratedBackfill
Browse files Browse the repository at this point in the history
  • Loading branch information
isoos authored Mar 14, 2024
2 parents 68141a5 + bb6833e commit b78ea58
Show file tree
Hide file tree
Showing 36 changed files with 763 additions and 208 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@ Important changes to data models, configuration, and migrations between each
AppEngine version, listed here to ease deployment and troubleshooting.

## Next Release (replace with git tag when deployed)

## `20240314t121900-all`
* Bumped runtimeVersion to `2024.03.12`.
* Upgraded dartdoc to `8.0.7`.
* Upgraded dependencies including `markdown 7.2.2`.
* Note: dynamic SDK selection may download latest stable, beta or master branches.

## `20240307t093000-all`
Expand Down
3 changes: 3 additions & 0 deletions app/lib/account/models.dart
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ class User extends db.ExpandoModel<String> {
isDeleted = false;
isModerated = false;
}

late final isVisible = !isBlocked && !(isModerated ?? false);
late final isNotVisible = !isVisible;
}

/// Maps Oauth user_id to User.id
Expand Down
2 changes: 2 additions & 0 deletions app/lib/admin/actions/actions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'package:pub_dev/admin/actions/create_publisher.dart';
import 'package:pub_dev/admin/actions/delete_publisher.dart';

import 'package:pub_dev/admin/actions/merge_moderated_package_into_existing.dart';
import 'package:pub_dev/admin/actions/moderate_package.dart';
import 'package:pub_dev/admin/actions/publisher_block.dart';
import 'package:pub_dev/admin/actions/publisher_members_list.dart';
import 'package:pub_dev/admin/actions/remove_package_from_publisher.dart';
Expand Down Expand Up @@ -71,6 +72,7 @@ final class AdminAction {
createPublisher,
deletePublisher,
mergeModeratedPackageIntoExisting,
moderatePackage,
publisherBlock,
publisherMembersList,
removePackageFromPublisher,
Expand Down
79 changes: 79 additions & 0 deletions app/lib/admin/actions/moderate_package.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// 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/backend.dart';
import '../../package/models.dart';
import '../../shared/datastore.dart';
import '../../task/backend.dart';
import '../../tool/maintenance/update_public_bucket.dart';
import 'actions.dart';

final moderatePackage = AdminAction(
name: 'moderate-package',
summary: 'Set the moderated flag on a package (making it not visible).',
description: '''
Set the moderated flag on a package (updating the flag and the timestamp).
''',
options: {
'package': 'The package name to be moderated',
'state':
'Set moderated state true / false. Returns current state if omitted.',
},
invoke: (options) async {
final package = options['package'];
InvalidInputException.check(
package != null && package.isNotEmpty,
'package must be given',
);

final state = options['state'];
bool? valueToSet;
switch (state) {
case 'true':
valueToSet = true;
break;
case 'false':
valueToSet = false;
break;
}

final p = await packageBackend.lookupPackage(package!);
if (p == null) {
throw NotFoundException.resource(package);
}

Package? p2;
if (valueToSet != null) {
p2 = await withRetryTransaction(dbService, (tx) async {
final pkg = await tx.lookupValue<Package>(p.key);
pkg.updateIsModerated(isModerated: valueToSet!);
tx.insert(pkg);
return pkg;
});

// retract or re-populate public archive files
await updatePublicArchiveBucket(
package: package,
ageCheckThreshold: Duration.zero,
deleteIfOlder: Duration.zero,
);

await taskBackend.trackPackage(package);
await purgePackageCache(package);
}

return {
'package': p.name,
'before': {
'isModerated': p.isModerated,
'moderatedAt': p.moderatedAt?.toIso8601String(),
},
if (p2 != null)
'after': {
'isModerated': p2.isModerated,
'moderatedAt': p2.moderatedAt?.toIso8601String(),
},
};
},
);
4 changes: 4 additions & 0 deletions app/lib/frontend/handlers/package.dart
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,10 @@ Future<shelf.Response> _handlePackagePage({
if (cachedPage == null) {
final package = await packageBackend.lookupPackage(packageName);
if (package == null || !package.isVisible) {
if (package?.isModerated ?? false) {
final content = renderModeratedPackagePage(packageName);
return htmlResponse(content, status: 404);
}
if (await packageBackend.isPackageModerated(packageName)) {
final content = renderModeratedPackagePage(packageName);
return htmlResponse(content, status: 404);
Expand Down
2 changes: 1 addition & 1 deletion app/lib/frontend/templates/package.dart
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,6 @@ List<Tab> buildPackageTabs({

/// Renders the package page when the package has been moderated.
String renderModeratedPackagePage(String packageName) {
final message = 'The package `$packageName` has been removed.';
final message = 'The package `$packageName` has been moderated.';
return renderErrorPage(default404NotFound, message);
}
2 changes: 1 addition & 1 deletion app/lib/package/backend.dart
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ class PackageBackend {
/// Returns false if the user is not an admin.
/// Returns false if the package is not visible e.g. blocked.
Future<bool> isPackageAdmin(Package p, String userId) async {
if (p.isBlocked) {
if (p.isNotVisible) {
return false;
}
if (p.publisherId == null) {
Expand Down
11 changes: 10 additions & 1 deletion app/lib/package/models.dart
Original file line number Diff line number Diff line change
Expand Up @@ -202,12 +202,13 @@ class Package extends db.ExpandoModel<String> {

// Convenience Fields:

bool get isVisible => !isBlocked;
bool get isVisible => !isBlocked && !(isModerated ?? false);
bool get isNotVisible => !isVisible;

bool get isIncludedInRobots {
final now = clock.now();
return isVisible &&
!(isModerated ?? false) &&
!isDiscontinued &&
!isUnlisted &&
now.difference(created!) > robotsVisibilityMinAge &&
Expand Down Expand Up @@ -413,6 +414,14 @@ class Package extends db.ExpandoModel<String> {
blocked = isBlocked ? clock.now().toUtc() : null;
updated = clock.now().toUtc();
}

void updateIsModerated({
required bool isModerated,
}) {
this.isModerated = isModerated;
moderatedAt = isModerated ? clock.now().toUtc() : null;
updated = clock.now().toUtc();
}
}

/// Describes the various categories of latest releases.
Expand Down
3 changes: 3 additions & 0 deletions app/lib/scorecard/backend.dart
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@ class ScoreCardBackend {
if (package == null) {
throw NotFoundException('Package "$packageName" does not exist.');
}
if (package.isNotVisible) {
throw ModeratedException.package(packageName);
}
final version =
await packageBackend.lookupPackageVersion(packageName, packageVersion);
if (version == null) {
Expand Down
1 change: 1 addition & 0 deletions app/lib/search/backend.dart
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ class SearchBackend {
Stream<PackageDocument> loadMinimumPackageIndex() async* {
final query = _db.query<Package>();
await for (final p in query.run()) {
if (p.isNotVisible) continue;
final releases = await packageBackend.latestReleases(p);
yield PackageDocument(
package: p.name!,
Expand Down
17 changes: 11 additions & 6 deletions app/lib/service/security_advisories/backend.dart
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,15 @@ class SecurityAdvisoryBackend {
return errors.isEmpty;
}

/// Overwrites existing advisory with the same id, if [osv] is newer.
/// Overwrites existing advisory with the same id, if [osv] is newer or
/// unconditionally if [resync] is set.
///
/// If id is already listed as `alias` for another advisory, no action will be
/// taken to resolve this. Instead both advisories will be stored and served.
/// It's assumed that security advisory database owners take care to keep the
/// security advisories sound, and that inconsistencies are intentional.
Future<SecurityAdvisory?> ingestSecurityAdvisory(
OSV osv, DateTime syncTime) async {
Future<SecurityAdvisory?> ingestSecurityAdvisory(OSV osv, DateTime syncTime,
{bool resync = false}) async {
return await withRetryTransaction(_db, (tx) async {
DateTime modified;
try {
Expand All @@ -81,7 +82,9 @@ class SecurityAdvisoryBackend {

final oldAdvisory = await lookupById(osv.id);

if (oldAdvisory != null && oldAdvisory.modified!.isAtOrAfter(modified)) {
if (!resync &&
oldAdvisory != null &&
oldAdvisory.modified!.isAtOrAfter(modified)) {
return oldAdvisory;
}

Expand All @@ -99,8 +102,10 @@ class SecurityAdvisoryBackend {
..parentKey = _db.emptyKey
..osv = osv
..aliases = idAndAliases
..affectedPackages =
(osv.affected ?? []).map((a) => a.package.name).toList()
..affectedPackages = (osv.affected ?? [])
.where((a) => a.package.ecosystem.toLowerCase() == 'pub')
.map((a) => a.package.name)
.toList()
..published =
osv.published != null ? DateTime.parse(osv.published!) : modified
..syncTime = syncTime;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ Future<(Map<String, OSV>, List<String>)> loadAdvisoriesFromDir(
return (osvs, failedFiles);
}

Future<void> updateAdvisories(Map<String, OSV> osvs) async {
Future<void> updateAdvisories(Map<String, OSV> osvs,
{bool resync = false}) async {
final syncTime = clock.now();

final oldAdvisories = await securityAdvisoryBackend.listAdvisories();
Expand All @@ -69,19 +70,20 @@ Future<void> updateAdvisories(Map<String, OSV> osvs) async {
}

for (final osv in osvs.values) {
await securityAdvisoryBackend.ingestSecurityAdvisory(osv, syncTime);
await securityAdvisoryBackend.ingestSecurityAdvisory(osv, syncTime,
resync: resync);
}
}

/// Synchronizes the security advisory backend with security advisories from
/// osv.dev, overwriting existing advisories with the same id, if the fetched
/// advisories are newer.
Future<void> syncSecurityAdvisories() async {
/// advisories are newer or overwriting unconditionally if [resync] is set.
Future<void> syncSecurityAdvisories({bool resync = false}) async {
final tempDir = await Directory.systemTemp.createTemp();
try {
await fetchAdvisories(tempDir);
final (osvs, failedFiles) = await loadAdvisoriesFromDir(tempDir);
await updateAdvisories(osvs);
await updateAdvisories(osvs, resync: resync);

if (failedFiles.isNotEmpty) {
throw Exception(
Expand Down
6 changes: 6 additions & 0 deletions app/lib/shared/exceptions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,12 @@ class RemovedPackageException extends NotFoundException {
RemovedPackageException() : super('Package has been removed.');
}

/// Thrown when package, versions, user or publisher is moderated.
class ModeratedException extends NotFoundException {
ModeratedException.package(String package)
: super('Package "$package" has been moderated.');
}

/// Thrown when API endpoint is not implemented.
class NotImplementedException extends ResponseException {
NotImplementedException([String? message])
Expand Down
51 changes: 48 additions & 3 deletions app/lib/shared/integrity.dart
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,16 @@ class IntegrityChecker {
yield 'User "${user.userId}" is recently created, but has no `oauthUserId`.';
}

if (user.isBlocked) {
if (user.isNotVisible) {
_blockedUsers.add(user.userId);
}

yield* _checkModeratedFlags(
kind: 'User',
id: user.userId,
isModerated: user.isModerated,
moderatedAt: user.moderatedAt,
);
});
}

Expand Down Expand Up @@ -197,6 +204,13 @@ class IntegrityChecker {
_logger.info('Scanning Publishers...');
yield* _queryWithPool<Publisher>((p) async* {
publisherAttributes.addPublisher(p);

yield* _checkModeratedFlags(
kind: 'Publisher',
id: p.publisherId,
isModerated: p.isModerated,
moderatedAt: p.moderatedAt,
);
});
}

Expand Down Expand Up @@ -286,13 +300,13 @@ class IntegrityChecker {
// empty uploaders
if (p.uploaders == null || p.uploaders!.isEmpty) {
// no publisher
if (p.publisherId == null && !p.isBlocked && !p.isDiscontinued) {
if (p.publisherId == null && p.isVisible && !p.isDiscontinued) {
yield 'Package "${p.name}" has no uploaders, must be marked discontinued.';
}

if (p.publisherId != null &&
publisherAttributes.isAbandoned(p.publisherId!) &&
!p.isBlocked &&
p.isVisible &&
!p.isDiscontinued) {
yield 'Package "${p.name}" has an abandoned publisher, must be marked discontinued.';
}
Expand Down Expand Up @@ -399,6 +413,12 @@ class IntegrityChecker {
} else if (!versionKeys.contains(p.latestPreviewVersionKey)) {
yield 'Package "${p.name}" has missing `latestPreviewVersionKey`: "${p.latestPreviewVersionKey!.id}".';
}
yield* _checkModeratedFlags(
kind: 'Package',
id: p.name!,
isModerated: p.isModerated,
moderatedAt: p.moderatedAt,
);

// Checking if PackageVersionInfo is referenced by a PackageVersion entity.
final pviQuery = _db.query<PackageVersionInfo>()
Expand Down Expand Up @@ -536,6 +556,13 @@ class IntegrityChecker {
yield* Stream.fromIterable(tarballItems);
}

yield* _checkModeratedFlags(
kind: 'PackageVersion',
id: pv.qualifiedVersionKey.toString(),
isModerated: pv.isModerated,
moderatedAt: pv.moderatedAt,
);

// TODO: remove null check after the backfill should have filled the property.
final sha256Hash = pv.sha256;
if (sha256Hash == null || sha256Hash.length != 32) {
Expand Down Expand Up @@ -879,3 +906,21 @@ class _PublisherAttributes {
_memberCount.clear();
}
}

/// Check that `isModerated` and `moderatedAt` are consistent.
Stream<String> _checkModeratedFlags({
required String kind,
required String id,
required bool? isModerated,
required DateTime? moderatedAt,
}) async* {
if (isModerated == null) {
yield '$kind "$id" has an `isModerated` property which is null.';
}
if ((isModerated ?? false) && moderatedAt == null) {
yield '$kind "$id" has `isModerated = true` but `moderatedAt` is null.';
}
if (!(isModerated ?? false) && moderatedAt != null) {
yield '$kind "$id" has `isModerated = false` but `moderatedAt` is not null.';
}
}
Loading

0 comments on commit b78ea58

Please sign in to comment.