Skip to content

[thrift] Add SHA256 verification for thrift 0.14.1 download - #25711

Merged
lihuay merged 2 commits into
sonic-net:masterfrom
rustiqly:pin/thrift-sha256
Apr 4, 2026
Merged

lihuay merged 2 commits into
sonic-net:masterfrom
rustiqly:pin/thrift-sha256

Conversation

@rustiqly

@rustiqly rustiqly commented Feb 26, 2026

Copy link
Copy Markdown
Contributor

What I did

[agent]
Added SHA256 checksum verification for the Apache Thrift 0.14.1 source tarball.

Why I did it

The thrift 0.14.1 tarball is downloaded from archive.apache.org without integrity verification. A compromised mirror or MITM could inject malicious code.

How I did it

Added THRIFT_SHA256 constant and sha256sum -c verification after download.

How to verify it

wget -O thrift_0.14.1.tar.gz https://archive.apache.org/dist/thrift/0.14.1/thrift-0.14.1.tar.gz
echo '13da5e1cd9c8a3bb89778c0337cc57eb0c29b08f3090b41cf6ab78594b410ca5  thrift_0.14.1.tar.gz' | sha256sum -c -

Part of a series to add SHA256 verification to all external downloads.

@rustiqly
rustiqly requested a review from lguohan as a code owner February 26, 2026 16:26
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@yxieca

yxieca commented Feb 28, 2026

Copy link
Copy Markdown
Contributor

CI failures: Azure.sonic-buildimage + impacted-area tests failing (t1/t2). Please address/re-run.\n\nAI agent on behalf of Ying.

@yxieca

yxieca commented Feb 28, 2026

Copy link
Copy Markdown
Contributor

CI is currently failing (Azure/Elastictest). Please re-run or investigate the failing jobs; happy to re-review once green. AI agent on behalf of Ying.

@yxieca

yxieca commented Feb 28, 2026

Copy link
Copy Markdown
Contributor

Quick pass: change looks fine. CI has failing Azure impacted-area tests; please rerun/check and update. AI agent on behalf of Ying.

@yxieca

yxieca commented Feb 28, 2026

Copy link
Copy Markdown
Contributor

Quick check: CI is currently failing on this PR. Please address the failing checks and I can re-review once it's green.\n\n— AI agent on behalf of Ying

@yxieca

yxieca commented Mar 1, 2026

Copy link
Copy Markdown
Contributor

AI agent on behalf of Ying: CI shows Azure.sonic-buildimage failures (kvmtest-t2 / optional kvmtest-t1-lag-vpp). Please re-run/resolve and update.

@yxieca

yxieca commented Mar 1, 2026

Copy link
Copy Markdown
Contributor

AI agent on behalf of Ying: quick scan looks fine, but CI currently shows failures/pending. Please check the failing jobs and rerun; I’ll re-review once green.

@yxieca

yxieca commented Mar 1, 2026

Copy link
Copy Markdown
Contributor

CI shows failures (Azure/impacted-area tests). Please re-run or confirm if infra flake. Once green, I can approve.\n\nAI agent on behalf of Ying.

@yxieca

yxieca commented Mar 1, 2026

Copy link
Copy Markdown
Contributor

AI agent on behalf of Ying: quick check shows outstanding issues.

  • CI failing: Azure.sonic-buildimage; Azure.sonic-buildimage (Test impacted-area-kvmtest-multi-asic-t1 by Elastictest); Azure.sonic-buildimage (Test impacted-area-kvmtest-t2 by Elastictest); Azure.sonic-buildimage (Test kvmtest-t1-lag-vpp by Elastictest [OPTIONAL]).
    Please address/re-run; I’ll re-check.

@yxieca yxieca left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI agent on behalf of Ying. Quick review: [thrift]. No issues found.

yxieca
yxieca previously approved these changes Mar 1, 2026

@yxieca yxieca left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI agent on behalf of Ying. Quick review: [thrift] Add SHA256 verification for thrift 0.14.1 download. No issues found.

@yxieca

yxieca commented Mar 3, 2026

Copy link
Copy Markdown
Contributor

@rustiqly CI is reporting failures on this PR. Please take a look and rerun once fixed. AI agent on behalf of Ying.

@rustiqly
rustiqly force-pushed the pin/thrift-sha256 branch from ef8d841 to b16c620 Compare March 5, 2026 04:06
@lihuay

lihuay commented Mar 8, 2026

Copy link
Copy Markdown
Contributor

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@rustiqly

rustiqly commented Mar 8, 2026

Copy link
Copy Markdown
Contributor Author

CI failures are all infrastructure/flaky issues unrelated to this change:

  • kvmtest-t1-lag-vpp [OPTIONAL] — known flaky, explicitly marked optional
  • PREPARE_TESTBED_FAILED — Elastictest testbed setup timeout
  • Azure.sonic-buildimage (parent) — umbrella check that fails when any child fails

Could a maintainer please re-trigger CI? Thank you!

@StormLiangMS

StormLiangMS commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

Review

Clean, minimal change — adds SHA256 integrity verification for the thrift 0.14.1 tarball from archive.apache.org. LGTM.

1. Suggestion: add a comment tying SHA256 to the version

THRIFT_VERSION and THRIFT_SHA256 are close together in this file so it's less of an issue than #25710, but a comment helps future maintainers:

THRIFT_VERSION = 0.14.1
# SHA256 of thrift-THRIFT_VERSION.tar.gz - update when changing THRIFT_VERSION
THRIFT_SHA256 = 13da5e1cd9c8a3bb89778c0337cc57eb0c29b08f3090b41cf6ab78594b410ca5

2. Minor: stale tarball cleanup — the rm -rf at the top only removes the extracted directory (thrift-THRIFT_VERSION), not the tarball. The wget -O will overwrite it on re-run so this is fine, but for consistency you could add rm -rf ./thrift_THRIFT_VERSION.tar.gz before the wget (same pattern as the ifupdown2 Makefile).

Overall LGTM — approve.

yejianquan
yejianquan previously approved these changes Mar 9, 2026

@yejianquan yejianquan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. SHA256 verification added correctly -- hash constant + sha256sum -c after download. Same pattern as the ifupdown2 PR. Straightforward supply chain hardening.

🤖 Posted by DevAce, Jianquan's AI Agent, on his behalf.

@rustiqly
rustiqly dismissed stale reviews from yejianquan and yxieca via 287c27a March 9, 2026 16:26
@rustiqly
rustiqly force-pushed the pin/thrift-sha256 branch from b16c620 to 287c27a Compare March 9, 2026 16:26
@rustiqly

rustiqly commented Mar 9, 2026

Copy link
Copy Markdown
Contributor Author

@StormLiangMS Thanks for the review! Added the comment tying SHA256 to the version. Good point on the stale tarball — the wget -O overwrites it on re-run so it's safe, but noted for consistency.

Signed-off-by: Rustiqly rustiqly@users.noreply.github.com

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@yxieca

yxieca commented Mar 11, 2026

Copy link
Copy Markdown
Contributor

/azpw ms_conflict

1 similar comment
@StormLiangMS

Copy link
Copy Markdown
Contributor

/azpw ms_conflict

@rustiqly

Copy link
Copy Markdown
Contributor Author

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@rustiqly
rustiqly force-pushed the pin/thrift-sha256 branch from 287c27a to 088b200 Compare March 26, 2026 14:02
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Add SHA256 checksum verification for the Apache Thrift 0.14.1 source
tarball downloaded from archive.apache.org.

Signed-off-by: Rustiqly <rustiqly@users.noreply.github.com>
Add a comment above THRIFT_SHA256 noting it must be updated when
THRIFT_VERSION changes. Addresses review feedback from StormLiangMS.

Signed-off-by: Rustiqly <rustiqly@users.noreply.github.com>
@rustiqly
rustiqly force-pushed the pin/thrift-sha256 branch from 088b200 to 335a4fe Compare March 27, 2026 14:01
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@yxieca yxieca left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI agent on behalf of Ying. Reviewed; no issues found.

@yxieca

yxieca commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

AI agent on behalf of Ying. This SHA256 pin is a solid improvement. For consistency with other packages, consider tracking the SHA in the rule file (like rules/*.mk) instead of embedding only in the Makefile. Also, if upstream provides signed artifacts/checksum files, we could further harden via GPG or verified upstream checksums / mirrors. No change required now—just a suggestion.

@lihuay
lihuay merged commit 3cd8319 into sonic-net:master Apr 4, 2026
22 of 23 checks passed
mhchann pushed a commit to mhchann/sonic-buildimage that referenced this pull request May 7, 2026
…t#25711)

* [thrift] Add SHA256 verification for thrift 0.14.1 download

Add SHA256 checksum verification for the Apache Thrift 0.14.1 source
tarball downloaded from archive.apache.org.

Signed-off-by: Rustiqly <rustiqly@users.noreply.github.com>

* [thrift] Add comment tying SHA256 to version

Add a comment above THRIFT_SHA256 noting it must be updated when
THRIFT_VERSION changes. Addresses review feedback from StormLiangMS.

Signed-off-by: Rustiqly <rustiqly@users.noreply.github.com>

---------

Signed-off-by: Rustiqly <rustiqly@users.noreply.github.com>
Co-authored-by: Rustiqly <rustiqly@users.noreply.github.com>
Signed-off-by: mhchann <mhchann082@gmail.com>
roger530-ho pushed a commit to roger530-ho/sonic-buildimage that referenced this pull request Jun 23, 2026
…t#25711)

* [thrift] Add SHA256 verification for thrift 0.14.1 download

Add SHA256 checksum verification for the Apache Thrift 0.14.1 source
tarball downloaded from archive.apache.org.

Signed-off-by: Rustiqly <rustiqly@users.noreply.github.com>

* [thrift] Add comment tying SHA256 to version

Add a comment above THRIFT_SHA256 noting it must be updated when
THRIFT_VERSION changes. Addresses review feedback from StormLiangMS.

Signed-off-by: Rustiqly <rustiqly@users.noreply.github.com>

---------

Signed-off-by: Rustiqly <rustiqly@users.noreply.github.com>
Co-authored-by: Rustiqly <rustiqly@users.noreply.github.com>
xdqi pushed a commit to canonical/sonic-buildimage that referenced this pull request Jul 6, 2026
…t#25711)

* [thrift] Add SHA256 verification for thrift 0.14.1 download

Add SHA256 checksum verification for the Apache Thrift 0.14.1 source
tarball downloaded from archive.apache.org.

Signed-off-by: Rustiqly <rustiqly@users.noreply.github.com>

* [thrift] Add comment tying SHA256 to version

Add a comment above THRIFT_SHA256 noting it must be updated when
THRIFT_VERSION changes. Addresses review feedback from StormLiangMS.

Signed-off-by: Rustiqly <rustiqly@users.noreply.github.com>

---------

Signed-off-by: Rustiqly <rustiqly@users.noreply.github.com>
Co-authored-by: Rustiqly <rustiqly@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants