-
Notifications
You must be signed in to change notification settings - Fork 12
mock keystone forwarder with deployment scripts #239
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: develop
Are you sure you want to change the base?
Conversation
src/v0.8/keystone/MockKeystoneForwarder.sol:MockKeystoneForwarder`; | ||
|
||
console.log("\nRunning verification..."); | ||
execSync(cmd, { stdio: 'inherit' }); |
Check warning
Code scanning / CodeQL
Indirect uncontrolled command line Medium
environment variable
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 days ago
The problem can be fixed by avoiding passing a single shell command string to execSync
, and instead using an API that invokes a binary directly with list-of-arguments, such as execFileSync
. This prevents any shell metacharacter interpretation and so avoids injection. In this case, we replace:
const cmd = `cd contracts && forge verify-contract ...`
execSync(cmd, { stdio: 'inherit' });
with
execFileSync('forge', [...args], { cwd: 'contracts', stdio: 'inherit' });
where args
is an array of all the arguments previously included as part of the string, including all flags and parameter values. Arguments are split exactly as would have been split by the shell, except that environment variable interpolation is replaced by direct use.
If the use of cd contracts && ...
was simply to change the working directory, the equivalent for execFileSync
is to set the third argument's cwd
property to contracts
.
Thus, the only changes required are:
- Require
execFileSync
fromchild_process
(possibly already available asexecSync
is already imported; add if not). - Construct the arguments for the
forge verify-contract
call as an array. - Invoke
execFileSync
instead ofexecSync
, passing in 'forge' and the argument array, with{ cwd: 'contracts', stdio: 'inherit' }
.
No new dependencies are needed.
-
Copy modified line R2 -
Copy modified lines R47-R55 -
Copy modified line R58
@@ -1,5 +1,5 @@ | ||
const fs = require("fs"); | ||
const { execSync } = require("child_process"); | ||
const { execSync, execFileSync } = require("child_process"); | ||
|
||
// Try to load dotenv if available | ||
try { | ||
@@ -44,16 +44,18 @@ | ||
console.log("\nWaiting 3 seconds to avoid rate limit..."); | ||
await new Promise(resolve => setTimeout(resolve, 3000)); | ||
|
||
const cmd = `cd contracts && forge verify-contract \ | ||
--chain sepolia \ | ||
--etherscan-api-key ${process.env.ETHERSCAN_API_KEY} \ | ||
--watch \ | ||
--retry 3 \ | ||
${contractAddress} \ | ||
src/v0.8/keystone/MockKeystoneForwarder.sol:MockKeystoneForwarder`; | ||
const forgeArgs = [ | ||
'verify-contract', | ||
'--chain', 'sepolia', | ||
'--etherscan-api-key', process.env.ETHERSCAN_API_KEY, | ||
'--watch', | ||
'--retry', '3', | ||
contractAddress, | ||
'src/v0.8/keystone/MockKeystoneForwarder.sol:MockKeystoneForwarder' | ||
]; | ||
|
||
console.log("\nRunning verification..."); | ||
execSync(cmd, { stdio: 'inherit' }); | ||
execFileSync('forge', forgeArgs, { cwd: 'contracts', stdio: 'inherit' }); | ||
|
||
console.log("\n✅ Contract verified successfully!"); | ||
console.log(`View on Sepolia Etherscan: https://sepolia.etherscan.io/address/${contractAddress}#code`); |
Static analysis results are availableHey @akhilchainani, you can view Slither reports in the job summary here or download them as artifact here. |
No description provided.