-
Notifications
You must be signed in to change notification settings - Fork 177
[Stellar] security contact as contract metadata #679
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 51 commits
27ebf04
7c7828e
ce5fcd3
3cd0b59
31f0c78
365421b
cedaeaa
98bd8af
a9098d0
6e9df26
574a739
c0e9002
86c65dc
a1111d3
abbd5a4
beffa34
d6bec2a
315b775
6ed6e4f
ea90cd1
abf687a
426b62d
ea25cc1
0911f87
4914083
5ce527f
03a32fc
b3c0347
0a52a65
9e74342
d727f51
60fb18f
cbb7631
0f1267f
acc5e6e
e3b74c8
16ba867
10442ac
f3d5bea
4427128
e4734df
52bd1e0
0de594f
5bac310
63a50d5
a10f142
8a4840e
c8bc5cf
46ed7c0
53ee786
0091f40
c5d0aa8
f994389
8c207e6
7348fa1
37b9e33
389df3d
80dd878
5132555
a7db183
40d3488
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@openzeppelin/wizard-stellar': patch | ||
--- | ||
|
||
Add contract metadata on contract model | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -89,15 +89,27 @@ test('contract with documentation', t => { | |
t.snapshot(printContract(Foo)); | ||
}); | ||
|
||
test('contract with security info', t => { | ||
test('contract with security contact metadata', t => { | ||
const Foo = new ContractBuilder('Foo'); | ||
Foo.addSecurityTag('[email protected]'); | ||
Foo.addContractMetadata({ key: 'contact', value: '[email protected]' }); | ||
t.snapshot(printContract(Foo)); | ||
}); | ||
|
||
test('contract with security info and documentation', t => { | ||
test('contract with multiple metadata', t => { | ||
const Foo = new ContractBuilder('Foo'); | ||
Foo.addSecurityTag('[email protected]'); | ||
Foo.addContractMetadata([ | ||
{ key: 'contact', value: '[email protected]' }, | ||
{ key: 'meta', value: 'data' }, | ||
]); | ||
t.snapshot(printContract(Foo)); | ||
}); | ||
|
||
test('contract with multiple metadata and documentation', t => { | ||
const Foo = new ContractBuilder('Foo'); | ||
Foo.addContractMetadata([ | ||
{ key: 'contact', value: '[email protected]' }, | ||
{ key: 'meta', value: 'data' }, | ||
]); | ||
Foo.addDocumentation('Some documentation'); | ||
t.snapshot(printContract(Foo)); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -155,36 +155,54 @@ Generated by [AVA](https://avajs.dev). | |
pub struct Foo;␊ | ||
` | ||
|
||
## contract with security info | ||
## contract with security contact metadata | ||
|
||
> Snapshot 1 | ||
|
||
`// SPDX-License-Identifier: MIT␊ | ||
// Compatible with OpenZeppelin Stellar Soroban Contracts ^0.4.1␊ | ||
#![no_std]␊ | ||
␊ | ||
use soroban_sdk::contractmeta;␊ | ||
␊ | ||
contractmeta!(key="contact", val="[email protected]");␊ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've noticed in the EVM communities it is common to have a regular contact and a security contact. Would it be beneficial to do that in this ecosystem as well? We don't have to add the general contact now, but it might be a good idea to differentiate the security contact now so as to make space for including a general contact in the future. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Noted, that make sense, |
||
␊ | ||
//! # Security␊ | ||
//!␊ | ||
//! For security issues, please contact: [email protected]␊ | ||
#[contract]␊ | ||
pub struct Foo;␊ | ||
` | ||
|
||
## contract with multiple metadata | ||
|
||
> Snapshot 1 | ||
|
||
`// SPDX-License-Identifier: MIT␊ | ||
// Compatible with OpenZeppelin Stellar Soroban Contracts ^0.4.1␊ | ||
#![no_std]␊ | ||
␊ | ||
use soroban_sdk::contractmeta;␊ | ||
␊ | ||
contractmeta!(key="contact", val="[email protected]");␊ | ||
contractmeta!(key="meta", val="data");␊ | ||
␊ | ||
#[contract]␊ | ||
pub struct Foo;␊ | ||
` | ||
|
||
## contract with security info and documentation | ||
## contract with multiple metadata and documentation | ||
|
||
> Snapshot 1 | ||
|
||
`// SPDX-License-Identifier: MIT␊ | ||
// Compatible with OpenZeppelin Stellar Soroban Contracts ^0.4.1␊ | ||
␊ | ||
//! Some documentation␊ | ||
␊ | ||
//! # Security␊ | ||
//!␊ | ||
//! For security issues, please contact: [email protected]␊ | ||
#![no_std]␊ | ||
␊ | ||
use soroban_sdk::contractmeta;␊ | ||
␊ | ||
contractmeta!(key="contact", val="[email protected]");␊ | ||
contractmeta!(key="meta", val="data");␊ | ||
␊ | ||
#[contract]␊ | ||
pub struct Foo;␊ | ||
` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -290,3 +290,24 @@ test.serial( | |
{ snapshotResult: false }, | ||
), | ||
); | ||
|
||
test.serial( | ||
'compilation fungible upgradable mintable pausable with security contact metadata', | ||
runRustCompilationTest( | ||
buildFungible, | ||
{ | ||
kind: 'Fungible', | ||
name: 'MyToken', | ||
symbol: 'MTK', | ||
premint: '2000', | ||
burnable: false, | ||
mintable: true, | ||
pausable: true, | ||
upgradeable: true, | ||
info: { | ||
securityContact: '[email protected]', | ||
}, | ||
}, | ||
{ snapshotResult: false }, | ||
), | ||
); |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -24,11 +24,11 @@ export function printContract(contract: Contract): string { | |||||||||||||||||||||||||||||||
`// SPDX-License-Identifier: ${contract.license}`, | ||||||||||||||||||||||||||||||||
`// Compatible with OpenZeppelin Stellar Soroban Contracts ${compatibleContractsSemver}`, | ||||||||||||||||||||||||||||||||
...(contract.documentations.length ? ['', ...printDocumentations(contract.documentations)] : []), | ||||||||||||||||||||||||||||||||
...(contract.securityContact ? ['', ...printSecurityTag(contract.securityContact)] : []), | ||||||||||||||||||||||||||||||||
...createLevelAttributes, | ||||||||||||||||||||||||||||||||
], | ||||||||||||||||||||||||||||||||
spaceBetween( | ||||||||||||||||||||||||||||||||
printUseClauses(contract), | ||||||||||||||||||||||||||||||||
printMetadata(contract), | ||||||||||||||||||||||||||||||||
printVariables(contract), | ||||||||||||||||||||||||||||||||
printContractStruct(contract), | ||||||||||||||||||||||||||||||||
printContractErrors(contract), | ||||||||||||||||||||||||||||||||
|
@@ -367,6 +367,6 @@ function printDocumentations(documentations: string[]): string[] { | |||||||||||||||||||||||||||||||
return documentations.map(documentation => `//! ${documentation}`); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
function printSecurityTag(securityContact: string) { | ||||||||||||||||||||||||||||||||
return ['//! # Security', '//!', `//! For security issues, please contact: ${securityContact}`]; | ||||||||||||||||||||||||||||||||
function printMetadata(contract: Contract) { | ||||||||||||||||||||||||||||||||
return Array.from(contract.metadata.entries()).map(([key, value]) => `contractmeta!(key="${key}", val="${value}");`); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
Comment on lines
371
to
375
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Escape metadata strings to avoid invalid Rust when values contain quotes/backslashes/newlines Unescaped user-supplied key/value can break codegen (e.g., value with a double quote). Apply this diff: -function printMetadata(contract: Contract) {
- return Array.from(contract.metadata.entries()).map(([key, value]) => `contractmeta!(key="${key}", val="${value}");`);
-}
+function printMetadata(contract: Contract) {
+ const esc = (s: string) =>
+ s
+ .replace(/\\/g, '\\\\')
+ .replace(/"/g, '\\"')
+ .replace(/\n/g, '\\n')
+ .replace(/\r/g, '\\r')
+ .replace(/\t/g, '\\t');
+ return Array.from(contract.metadata.entries()).map(
+ ([key, value]) => `contractmeta!(key="${esc(key)}", val="${esc(value)}");`,
+ );
+} Consider adding a unit test that includes quotes/newlines to lock this in (see suggestion on test file). 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @CoveMB We should sanitize the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,5 @@ | ||
import type { ContractBuilder } from './contract'; | ||
|
||
export const TAG_SECURITY_CONTACT = `@custom:security-contact`; | ||
|
||
export const infoOptions = [{}, { license: 'WTFPL' }] as const; | ||
|
||
export const defaults: Info = { license: 'MIT' }; | ||
|
@@ -11,14 +9,8 @@ export type Info = { | |
securityContact?: string; | ||
}; | ||
|
||
export function setInfo(c: ContractBuilder, info: Info): void { | ||
const { securityContact, license } = info; | ||
|
||
if (securityContact) { | ||
c.addSecurityTag(securityContact); | ||
} | ||
export function setInfo(c: ContractBuilder, { securityContact, license }: Info): void { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should add at least 1 snapshot test that covers going through this |
||
if (securityContact) c.addContractMetadata({ key: 'contact', value: securityContact }); | ||
|
||
if (license) { | ||
c.license = license; | ||
} | ||
if (license) c.license = license; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,7 @@ | |
<span class="flex justify-between pr-2"> | ||
Security Contact | ||
<HelpTooltip> | ||
Where people can contact you to report security issues. Will only be visible if contract metadata is verified. | ||
A contact point (email or URL) for reporting security issues. This will be added to the generated contract as documentation comments. | ||
ericglau marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
</HelpTooltip> | ||
</span> | ||
<input bind:value={info.securityContact} placeholder="[email protected]" /> | ||
|
Uh oh!
There was an error while loading. Please reload this page.