-
Notifications
You must be signed in to change notification settings - Fork 199
feat(ci): verifies that the committed Go FFI binding #7478
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: main
Are you sure you want to change the base?
Changes from all commits
061242e
702ce1c
df711c2
d77c7df
ff1592f
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 |
|---|---|---|
|
|
@@ -33,17 +33,23 @@ fn main() { | |
| // See <https://github.com/status-im/status-mobile/issues/20135#issuecomment-2137400475> | ||
| std::env::set_var("GOFLAGS", "-tags=netgo"); | ||
| } | ||
| rust2go::Builder::default() | ||
| .with_go_src("./f3-sidecar") | ||
| // the generated Go file has been committed to the git repository, | ||
| // uncomment to regenerate the code locally | ||
| // .with_regen_arg(rust2go::RegenArgs { | ||
| // src: "./src/f3/go_ffi.rs".into(), | ||
| // dst: "./f3-sidecar/ffi_gen.go".into(), | ||
| // without_main: true, | ||
| // ..Default::default() | ||
| // }) | ||
| .build(); | ||
| println!("cargo:rerun-if-changed=src/f3/go_ffi.rs"); | ||
| println!("cargo:rerun-if-env-changed=FOREST_REGENERATE_GO_FFI"); | ||
|
|
||
| let mut builder = rust2go::Builder::default().with_go_src("./f3-sidecar"); | ||
|
|
||
| // the generated Go file has been committed to the git repository | ||
| // set the var to regenerate the file | ||
| if is_env_truthy("FOREST_REGENERATE_GO_FFI") { | ||
| builder = builder.with_regen_arg(rust2go::RegenArgs { | ||
| src: "./src/f3/go_ffi.rs".into(), | ||
| dst: "./f3-sidecar/ffi_gen.go".into(), | ||
| without_main: true, | ||
| ..Default::default() | ||
| }) | ||
| } | ||
|
Comment on lines
+43
to
+50
Contributor
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. 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 4 \
'FOREST_F3_SIDECAR_FFI_BUILD_OPT_OUT|FOREST_REGENERATE_GO_FFI|lint:go-ffi' \
mise.toml .github/workflows/go-lint.ymlRepository: ChainSafe/forest Length of output: 1477 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- build.rs ---'
sed -n '1,90p' build.rs
printf '%s\n' '--- all opt-out and regeneration references ---'
rg -n -C 5 \
'FOREST_F3_SIDECAR_FFI_BUILD_OPT_OUT|FOREST_REGENERATE_GO_FFI|ffi_gen.go|lint:go-ffi' \
--glob '!target/**' --glob '!node_modules/**' .
printf '%s\n' '--- task and workflow environments ---'
sed -n '85,115p' mise.toml
sed -n '55,80p' .github/workflows/go-lint.yml
printf '%s\n' '--- deterministic environment-flow probe ---'
python3 - <<'PY'
from pathlib import Path
import re
build = Path("build.rs").read_text()
mise = Path("mise.toml").read_text()
workflow = Path(".github/workflows/go-lint.yml").read_text()
guard = re.search(
r'if\s+!is_env_truthy\("FOREST_F3_SIDECAR_FFI_BUILD_OPT_OUT"\)\s*\{(?P<body>.*?)\n\s*\}',
build,
re.S,
)
regen = 'FOREST_REGENERATE_GO_FFI=1 cargo check' in mise
diff = 'git diff --exit-code -- interop-tests/src/tests/go_app/ffi_gen.go f3-sidecar/ffi_gen.go' in mise
workflow_task = 'mise lint:go-ffi' in workflow
print({
"build_guard_found": bool(guard),
"regen_inside_opt_out_guard": bool(
guard and 'FOREST_REGENERATE_GO_FFI' in guard.group("body")
),
"mise_sets_regeneration_for_cargo_checks": regen,
"mise_checks_generated_files_with_git_diff": diff,
"workflow_invokes_lint_task": workflow_task,
"mise_clears_opt_out": bool(re.search(
r'(^|\\n)\\s*(unset|env\\s+-u|FOREST_F3_SIDECAR_FFI_BUILD_OPT_OUT\\s*=)',
mise
)),
})
PYRepository: ChainSafe/forest Length of output: 18156 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- package and build-script ownership ---'
rg -n -C 3 \
'name = "forest-(filecoin|interop-tests)"|build = "build.rs"|path = ".*build.rs"|FOREST_F3_SIDECAR_FFI_BUILD_OPT_OUT' \
Cargo.toml interop-tests/Cargo.toml src f3-sidecar 2>/dev/null || true
printf '%s\n' '--- workflow-level environment declarations ---'
rg -n -C 3 \
'^(env:|[[:space:]]+env:|FOREST_F3_SIDECAR_FFI_BUILD_OPT_OUT|FOREST_REGENERATE_GO_FFI)' \
.github/workflows mise.toml 2>/dev/null || true
printf '%s\n' '--- shell environment semantics used by the task ---'
python3 - <<'PY'
import os, subprocess
env = os.environ.copy()
env["FOREST_F3_SIDECAR_FFI_BUILD_OPT_OUT"] = "1"
env["FOREST_REGENERATE_GO_FFI"] = "1"
result = subprocess.run(
["sh", "-c", "FOREST_REGENERATE_GO_FFI=1 sh -c 'printf \"opt_out=%s regen=%s\\n\" \"$FOREST_F3_SIDECAR_FFI_BUILD_OPT_OUT\" \"$FOREST_REGENERATE_GO_FFI\"'"],
env=env,
text=True,
capture_output=True,
check=True,
)
print(result.stdout, end="")
PYRepository: ChainSafe/forest Length of output: 16134 Make 🤖 Prompt for AI Agents |
||
|
|
||
| builder.build(); | ||
| } | ||
|
|
||
| rpc_regression_tests_gen(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -64,3 +64,5 @@ environment variable `FOREST_F3_SIDECAR_FFI_BUILD_OPT_OUT=1` is set. | |||||
|
|
||||||
| F3 sidecar is not started by default, set `FOREST_F3_SIDECAR_FFI_ENABLED=1` to | ||||||
| opt in. | ||||||
|
|
||||||
| To generate the Go F3-sidecar FFI bindings set `FOREST_REGENERATE_GO_FFI=1` otherwise it will use current/already generated ones | ||||||
|
Contributor
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. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Clarify the fallback binding description. The phrase Proposed wording-To generate the Go F3-sidecar FFI bindings set `FOREST_REGENERATE_GO_FFI=1` otherwise it will use current/already generated ones
+Set `FOREST_REGENERATE_GO_FFI=1` to generate the Go F3-sidecar FFI bindings. Otherwise, the build uses the existing generated bindings.📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
|
EclesioMeloJunior marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,8 +1,6 @@ | ||||||||||||||||||||
| // Copyright 2019-2026 ChainSafe Systems | ||||||||||||||||||||
| // SPDX-License-Identifier: Apache-2.0, MIT | ||||||||||||||||||||
|
|
||||||||||||||||||||
| use crate::prelude::*; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| pub mod binding { | ||||||||||||||||||||
| #![allow(warnings)] | ||||||||||||||||||||
| #![allow(clippy::indexing_slicing)] | ||||||||||||||||||||
|
|
@@ -16,8 +14,8 @@ pub trait GoF3Node { | |||||||||||||||||||
| jwt: String, | ||||||||||||||||||||
| f3_rpc_endpoint: String, | ||||||||||||||||||||
| initial_power_table: String, | ||||||||||||||||||||
| bootstrap_epoch: ChainEpoch, | ||||||||||||||||||||
| finality: ChainEpoch, | ||||||||||||||||||||
| bootstrap_epoch: i64, | ||||||||||||||||||||
| finality: i64, | ||||||||||||||||||||
|
Comment on lines
+17
to
+18
Contributor
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. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win Document the changed public FFI contract. The public Suggested documentation pub trait GoF3Node {
+ /// Runs the F3 node through the Go FFI boundary.
+ ///
+ /// `bootstrap_epoch` and `finality` are Filecoin epochs encoded as `i64`.
+ /// Returns the FFI success status.
fn run(As per coding guidelines: “Document public functions and structs (see Documentation practices).” 📝 Committable suggestion
Suggested change
🤖 Prompt for AI AgentsSource: Coding guidelines |
||||||||||||||||||||
| f3_root: String, | ||||||||||||||||||||
| ) -> bool; | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: ChainSafe/forest
Length of output: 50372
🏁 Script executed:
Repository: ChainSafe/forest
Length of output: 14883
🌐 Web query:
actions/checkout persist-credentials false local git config http extraheader GITHUB_TOKEN documentation💡 Result:
The persist-credentials input in actions/checkout controls whether the action automatically configures the local Git repository with credentials (typically the GITHUB_TOKEN) to enable subsequent authenticated Git operations [1][2][3]. When persist-credentials is set to true (the default), the action configures Git to use an authorization header for requests to GitHub [1][3]. The specific mechanism has evolved: In versions v5 and earlier, the action directly injected an http.extraheader containing the authorization token into the local.git/config file [4][5]. In version v6 and later, the action enhances security by writing the credentials to a separate file within the runner's temporary directory ($RUNNER_TEMP) and adding an includeIf.gitdir configuration to.git/config that points to that file [4][6]. Setting persist-credentials: false opts out of this behavior [1][2]. This is often recommended when: You do not need to perform subsequent Git operations that require the default token [4]. You want to prevent the GITHUB_TOKEN from being persisted in any configuration file, which can be a security best practice to reduce the attack surface for credential leakage [7][4][8]. You are using custom authentication (e.g., a personal access token or SSH key) and want to ensure the default GITHUB_TOKEN does not override or interfere with your configuration [9][5][6]. To use this option in your workflow, add it to the with block of your checkout step: - name: Checkout repository uses: actions/checkout@v4 with: persist-credentials: false
Citations:
persist-credentials: falseon checkout steps cli/go-gh#225gh auth setup-gitdoes not override the git user following anactions/checkoutinvocation cli/cli#10905persist-credentials: falsewith actions/checkout github/gh-aw#2641persist-credentials=trueplease actions/checkout#2312🏁 Script executed:
Repository: ChainSafe/forest
Length of output: 450
Disable persisted checkout credentials before
mise lint:go-ffi.cargo checkexecutes theforest-interop-testsbuild script and regenerates Go bindings. A build script or dependency could read the persistedGITHUB_TOKEN. Setpersist-credentials: falseon theactions/checkout@v7step; the job only needs localgit diff.🤖 Prompt for AI Agents
Source: Linters/SAST tools