Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@ jobs:
cat NOTES.md

- name: Publish
id: publish
env:
GH_TOKEN: ${{ github.token }}
run: |
Expand All @@ -386,3 +387,38 @@ jobs:
--notes-file NOTES.md \
--prerelease="${{ needs.version.outputs.prerelease }}" \
artifacts/*
echo "tag=$tag" >> "$GITHUB_OUTPUT"

# Here rather than in a workflow of its own listening on `release`.
# GitHub refuses to start a workflow run from an event its own
# GITHUB_TOKEN caused, and the release above is created with exactly that
# token — an `on: release: [published]` workflow would sit silent forever
# and look like a Discord problem.
#
# `published` would also have been the only safe type to listen for:
# publishing a pre-release fires `published` *and* `prereleased`, so
# asking for both announces every snapshot twice.
- name: Announce on Discord
if: always() && steps.publish.outputs.tag != ''
env:
DISCORD_WEBHOOK: ${{ secrets.DISCORD_WEBHOOK }}
# Not secrets: a custom emoji's id is visible to everyone who can see
# the server, and it is useless without the webhook that is. Here
# rather than as defaults in the script, so a fork announcing to its
# own server falls back to the unicode pair instead of referencing
# emoji it does not have.
DPIP_EMOJI_ANDROID: "<:android:1539201870935892028>"
DPIP_EMOJI_IOS: "<:ios:1539201897766981692>"
# Without it the release lookup runs unauthenticated, at 60 requests
# an hour shared across everything on this runner's IP.
GITHUB_TOKEN: ${{ github.token }}
run: |
if [ -z "${DISCORD_WEBHOOK:-}" ]; then
echo "::notice::DISCORD_WEBHOOK is not set — nothing announced"
exit 0
fi
# A warning, not a failure. The build is published and the artifacts
# are downloadable by the time this runs; turning the job red over an
# announcement would say the release did not happen.
python3 tool/release/discord.py "${{ steps.publish.outputs.tag }}" ||
echo "::warning::Discord announcement failed — the release itself is fine"
Comment on lines +423 to +424

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

security · medium
建議將 ${{ steps.publish.outputs.tag }} 透過環境變數傳遞,而非直接在 run 腳本中使用 GitHub Actions 表達式。這可以避免潛在的指令注入(Script Injection)風險。雖然 release 的 tag 通常是可信的,但為了符合安全最佳實踐,建議將所有外部輸入(包含 step outputs)透過 env 區塊傳遞,以確保 shell 執行時將其視為純字串。

Suggestion:

Suggested change
python3 tool/release/discord.py "${{ steps.publish.outputs.tag }}" ||
echo "::warning::Discord announcement failed — the release itself is fine"
env:
DISCORD_WEBHOOK: ${{ secrets.DISCORD_WEBHOOK }}
DPIP_EMOJI_ANDROID: "<:android:1539201870935892028>"
DPIP_EMOJI_IOS: "<:ios:1539201897766981692>"
GITHUB_TOKEN: ${{ github.token }}
TAG: ${{ steps.publish.outputs.tag }}
run: |
if [ -z "${DISCORD_WEBHOOK:-}" ]; then
echo "::notice::DISCORD_WEBHOOK is not set — nothing announced"
exit 0
fi
python3 tool/release/discord.py "$TAG" ||
echo "::warning::Discord announcement failed — the release itself is fine"

Comment on lines +423 to +424

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

security · medium
建議將 ${{ steps.publish.outputs.tag }} 透過環境變數傳遞,而非直接在 run 腳本中使用 GitHub Actions 表達式。這可以避免潛在的指令注入(Script Injection)風險。雖然 release 的 tag 通常是可信的,但為了符合安全最佳實踐,建議將所有外部輸入(包含 step outputs)透過 env 區塊傳遞,以確保 shell 執行時將其視為純字串。

Suggestion:

Suggested change
python3 tool/release/discord.py "${{ steps.publish.outputs.tag }}" ||
echo "::warning::Discord announcement failed — the release itself is fine"
env:
DISCORD_WEBHOOK: ${{ secrets.DISCORD_WEBHOOK }}
DPIP_EMOJI_ANDROID: "<:android:1539201870935892028>"
DPIP_EMOJI_IOS: "<:ios:1539201897766981692>"
GITHUB_TOKEN: ${{ github.token }}
TAG: ${{ steps.publish.outputs.tag }}
run: |
if [ -z "${DISCORD_WEBHOOK:-}" ]; then
echo "::notice::DISCORD_WEBHOOK is not set — nothing announced"
exit 0
fi
python3 tool/release/discord.py "$TAG" ||
echo "::warning::Discord announcement failed — the release itself is fine"

107 changes: 107 additions & 0 deletions test/tool/discord_payload_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/// `tool/release/discord.py` — the release announcement, inside Discord's
/// limits.
///
/// The truncation path only runs on a release big enough to overflow, which is
/// exactly the release nobody wants to discover it on. These force the overflow
/// with a smaller budget so the behaviour is exercised on every run, and read a
/// committed fixture rather than the API so they are neither flaky nor
/// rate-limited.
library;

import 'dart:convert';
import 'dart:io';

import 'package:flutter_test/flutter_test.dart';

const _android = '<:android:1539201870935892028>';
const _ios = '<:ios:1539201897766981692>';

Map<String, Object?> payload({int? limit, bool short = false}) {
final root = Directory.current.path;
final result = Process.runSync(
'python3',
['$root/tool/release/discord.py', '26w34c', '--dry-run'],
environment: {
'DPIP_RELEASE_JSON': '$root/test/tool/fixtures/release_26w34c.json',
'DPIP_EMOJI_ANDROID': short ? '<:a:1>' : _android,
'DPIP_EMOJI_IOS': short ? '<:i:2>' : _ios,
if (limit != null) 'DPIP_DISCORD_LIMIT': '$limit',
},
);
expect(result.exitCode, 0, reason: result.stderr.toString());
return jsonDecode(result.stdout as String) as Map<String, Object?>;
}

List<Object?> embeds(Map<String, Object?> p) => p['embeds']! as List<Object?>;
String description(Map<String, Object?> p) =>
(embeds(p).single as Map<String, Object?>)['description']! as String;

/// `… (15/22)` → 15.
int shown(String text) =>
RegExp(r'… \((\d+)/\d+\)')
.allMatches(text)
.map((m) => int.parse(m.group(1)!))
.fold(0, (a, b) => a + b);

void main() {
test('one embed, always inside the description limit', () {
for (final limit in [4096, 3000, 2000, 1200]) {
final p = payload(limit: limit);
expect(embeds(p), hasLength(1), reason: 'limit $limit');
expect(
description(p).length,
lessThanOrEqualTo(limit),
reason: 'limit $limit',
);
}
});

test('the whole note when it fits, nothing marked missing', () {
// Short ids stand in for a guild whose emoji names are brief; the note then
// fits and no category should claim to be cut. Shortening only one of the
// two still overflows — 55 entries pay for both on almost every line.
final text = description(payload(short: true));

expect(text, isNot(contains('… (')));
expect(text, contains('修正換行的更新日誌條目在發布時被從中間截斷'));
});

test('hashes are dropped before any entry is', () {
// The order matters: a hash costs a click on the link at the bottom, a
// dropped entry means a change shipped and nobody was told.
final full = description(payload(short: true));
final tight = description(payload());

expect(full, contains('`c5fdbd31`'));
expect(tight, isNot(contains('`c5fdbd31`')));
});

test('a cut is split evenly between the categories', () {
// Filling from the top until the budget runs out would spend it all on
// 新功能 and post 錯誤修正 as an empty heading — a release where the fixes
// are the point would read as a release with no fixes.
final text = description(payload(limit: 1500));
final counts = RegExp(r'… \((\d+)/(\d+)\)')
.allMatches(text)
.map((m) => int.parse(m.group(1)!))
.toList();

expect(counts, hasLength(greaterThanOrEqualTo(2)));
expect(counts.toSet(), hasLength(1), reason: 'categories got $counts');
});

test('a smaller budget never shows more', () {
expect(
shown(description(payload(limit: 1200))),
lessThanOrEqualTo(shown(description(payload(limit: 2000)))),
);
});

test('the heading, the subtext and the link all survive a cut', () {
final text = description(payload(limit: 1200));

expect(text, startsWith('# 26w34c'));
expect(text, contains('-# 快照,取自 main 的'));
expect(text, contains('releases/tag/26w34c'));
});
}
116 changes: 116 additions & 0 deletions test/tool/fixtures/release_26w34c.json

Large diffs are not rendered by default.

Loading
Loading