Skip to content

fix(syn): use AST-based bytemuck serialization detection#4215

Merged
jamie-osec merged 1 commit into
otter-sec:masterfrom
raushan728:feature-branch
Jun 4, 2026
Merged

fix(syn): use AST-based bytemuck serialization detection#4215
jamie-osec merged 1 commit into
otter-sec:masterfrom
raushan728:feature-branch

Conversation

@raushan728

@raushan728 raushan728 commented Feb 4, 2026

Copy link
Copy Markdown
Contributor

Replace string-based bytemuck detection with AST parsing. Only bytemuck::Pod sets safe serialization; only bytemuck::Unsafe sets unsafe serialization. Unqualified and non-Pod derives are ignored to prevent false positives from unrelated crates.

@vercel

vercel Bot commented Feb 4, 2026

Copy link
Copy Markdown

@raushan728 is attempting to deploy a commit to the Solana Foundation Team on Vercel.

A member of the Team first needs to authorize it.

@raushan728 raushan728 force-pushed the feature-branch branch 2 times, most recently from afd83b4 to c88ed57 Compare April 16, 2026 10:25
@0x4ka5h

0x4ka5h commented Apr 17, 2026

Copy link
Copy Markdown
Collaborator

can you fix the CI?

@raushan728

Copy link
Copy Markdown
Contributor Author

can you fix the CI?

I will fix it today. I looked at it yesterday, but I had my college exams going on

@raushan728 raushan728 changed the title fix(syn): robust bytemuck attribute parsing fix(syn): replace string-based bytemuck detection with AST parsing Apr 18, 2026
@raushan728 raushan728 force-pushed the feature-branch branch 2 times, most recently from a5d7bc4 to 8b58ebc Compare April 18, 2026 06:44
@raushan728

Copy link
Copy Markdown
Contributor Author

@0x4ka5h Thanks for the patience! Could you take a look at the updated approach when you get a chance?

@raushan728

raushan728 commented Apr 18, 2026

Copy link
Copy Markdown
Contributor Author

@0x4ka5h I'm a bit confused about the fmt issue. My local nightly rustfmt wants __erase last, but CI wants it first different nightly versions sorting differently. I've manually fixed it to match what CI expects

raushan@DESKTOP-RU5BVRN:~/anchor$ cargo +nightly fmt -- --check
Diff in /home/raushan/anchor/lang/src/lib.rs:62:
     anchor_attribute_event::{emit, event},
     anchor_attribute_program::{declare_program, instruction, program},
     anchor_derive_accounts::Accounts,
-    anchor_derive_serde::{__erase, AnchorDeserialize, AnchorSerialize},
+    anchor_derive_serde::{AnchorDeserialize, AnchorSerialize, __erase},
     anchor_derive_space::InitSpace,
     borsh::ser::BorshSerialize as AnchorSerialize,
     const_crypto::ed25519::derive_program_address,

The TransactionExpiredTimeoutError in the misc test appears to be a flaky network timeout. Could you please re-run jobs?

@jamie-osec

Copy link
Copy Markdown
Collaborator

For future reference, CI uses the latest nightly version, and this setting changed recently.

@raushan728

Copy link
Copy Markdown
Contributor Author

@jamie-osec ido-pool failures are just timing/network flakes, can you re-run?

Comment thread lang/syn/src/idl/defined.rs Outdated
@jamie-osec

Copy link
Copy Markdown
Collaborator

I think we should revert the unqualified matchers i.e. derives must have bytemuck::. Please try and keep the semantics as similar as possible

@raushan728 raushan728 changed the title fix(syn): replace string-based bytemuck detection with AST parsing fix(syn): restrict bytemuck serialization to namespaced derives Apr 21, 2026
@raushan728

Copy link
Copy Markdown
Contributor Author

I think we should revert the unqualified matchers i.e. derives must have bytemuck::. Please try and keep the semantics as similar as possible

Done, restricted to bytemuck:: namespaced derives only.

@raushan728 raushan728 requested a review from jamie-osec April 21, 2026 13:00
@jamie-osec

Copy link
Copy Markdown
Collaborator

I'm not sure where unsafe_impl_pod has come from; isn't this code to handle the specifically generated fake derive(bytemuck::Unsafe)? This is still a significant change to semantics
https://github.com/solana-foundation/anchor/blob/cfeec7dc7581e5180d7c01d0bcba12c7bbe04740/lang/attribute/account/src/lib.rs#L602

@raushan728

raushan728 commented Apr 22, 2026

Copy link
Copy Markdown
Contributor Author

Thanks you @jamie-osec to point me I removed is_unsafe_impl_pod entirely simplified to
has_unsafe_segment only since bytemuck::Unsafe is the only generated derive

@0x4ka5h 0x4ka5h self-requested a review June 2, 2026 07:50
Comment thread lang/syn/src/idl/defined.rs Outdated
@raushan728 raushan728 changed the title fix(syn): restrict bytemuck serialization to namespaced derives fix(syn): use AST-based bytemuck serialization detection Jun 2, 2026
@raushan728 raushan728 requested a review from chen-robert as a code owner June 2, 2026 13:32
@raushan728 raushan728 requested a review from 0x4ka5h June 2, 2026 13:35
0x4ka5h
0x4ka5h previously approved these changes Jun 2, 2026

@0x4ka5h 0x4ka5h left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

cc. @jamie-osec needs second review here

Comment thread lang/syn/src/idl/defined.rs Outdated
@jamie-osec jamie-osec merged commit 0d27859 into otter-sec:master Jun 4, 2026
103 of 104 checks passed
@raushan728 raushan728 deleted the feature-branch branch June 5, 2026 07:05
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.

4 participants