Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions .github/workflows/go-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ on:
- "go.work"
- "mise.toml"
- "f3-sidecar/**"
- "interop-tests/src/tests/**"
- "interop-tests/**"
- "build.rs"
- "src/f3/go_ffi.rs"
push:
branches:
- main
Expand All @@ -27,7 +29,9 @@ on:
- "go.work"
- "mise.toml"
- "f3-sidecar/**"
- "interop-tests/src/tests/**"
- "interop-tests/**"
- "build.rs"
- "src/f3/go_ffi.rs"

jobs:
# Merge groups don't support path filters, so detect the relevant changes
Expand All @@ -48,13 +52,23 @@ jobs:
- 'go.work'
- 'mise.toml'
- 'f3-sidecar/**'
- 'interop-tests/src/tests/**'
- 'interop-tests/**'
- 'build.rs'
- 'src/f3/go_ffi.rs'
lint-go:
name: Go lint checks
needs: changes
if: ${{ needs.changes.outputs.changesFound == 'true' || github.event_name == 'workflow_dispatch' }}
runs-on: ubuntu-slim
runs-on: ubuntu-24.04-arm
steps:
- uses: actions/checkout@v7
- uses: jdx/mise-action@v4
- name: Apt Dependencies
uses: nick-fields/retry@v4
with:
timeout_minutes: 5
max_attempts: 3
command: |
sudo apt-get install -y libclang-dev # required by rust2go's bindgen
- run: mise lint:go-ffi

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.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- workflow ---'
sed -n '1,110p' .github/workflows/go-lint.yml

printf '%s\n' '--- relevant mise tasks and scripts ---'
rg -n -C 5 'lint:go-ffi|go-ffi|cargo|go generate|git diff' mise.toml .mise.toml mise.yml .mise.yml .github scripts Makefile 2>/dev/null || true

printf '%s\n' '--- checkout configuration across workflows ---'
rg -n -C 4 'actions/checkout|persist-credentials' .github/workflows 2>/dev/null || true

Repository: ChainSafe/forest

Length of output: 50372


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- FFI package manifest and build scripts ---'
rg -n -C 8 'forest-interop-tests|build\s*=|FOREST_REGENERATE_GO_FFI|go_ffi|ffi_gen' Cargo.toml Cargo.lock interop-tests f3-sidecar 2>/dev/null | head -n 240

printf '%s\n' '--- exact checkout settings in the reviewed workflow ---'
nl -ba .github/workflows/go-lint.yml | sed -n '45,70p'

printf '%s\n' '--- workflow token and repository context usage ---'
rg -n -C 3 'GITHUB_TOKEN|github.token|secrets\.[A-Za-z0-9_]+|permissions:' .github/workflows/go-lint.yml .github/actions .github 2>/dev/null | head -n 160

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:


🏁 Script executed:

#!/bin/bash
set -eu

python3 - <<'PY'
from pathlib import Path
import re

workflow = Path(".github/workflows/go-lint.yml").read_text()
mise = Path("mise.toml").read_text()
build = Path("interop-tests/build.rs").read_text()
manifest = Path("interop-tests/Cargo.toml").read_text()

checks = {
    "lint workflow has checkout before FFI task": bool(re.search(
        r"actions/checkout@v7[\s\S]*?mise lint:go-ffi", workflow)),
    "FFI task runs cargo check": "FOREST_REGENERATE_GO_FFI=1 cargo check -p forest-interop-tests" in mise,
    "FFI task checks generated file with git diff": "git diff --exit-code -- interop-tests/src/tests/go_app/ffi_gen.go" in mise,
    "interop-tests has a Cargo build script": 'build = "build.rs"' in manifest,
    "build script enables regeneration from environment": 'FOREST_REGENERATE_GO_FFI' in build,
    "build script writes generated Go file": 'src/tests/go_app/ffi_gen.go' in build,
}
for name, result in checks.items():
    print(f"{name}: {result}")
if not all(checks.values()):
    raise SystemExit("invariant failed")
PY

Repository: ChainSafe/forest

Length of output: 450


Disable persisted checkout credentials before mise lint:go-ffi.

cargo check executes the forest-interop-tests build script and regenerates Go bindings. A build script or dependency could read the persisted GITHUB_TOKEN. Set persist-credentials: false on the actions/checkout@v7 step; the job only needs local git diff.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/go-lint.yml at line 62, Update the actions/checkout@v7
step in the workflow to set persist-credentials to false before the mise
lint:go-ffi step, while preserving the job’s ability to inspect local git diff
output.

Source: Linters/SAST tools

- run: mise lint:golang
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ lldb target/debugging/forest
- `RUST_LOG` - Logging configuration (e.g., `debug`, `forest=trace`)
- `FULLNODE_API_INFO` - RPC endpoint and authentication token
- `FOREST_F3_SIDECAR_FFI_BUILD_OPT_OUT` - Disable F3 sidecar build (for debugging profile)
- `FOREST_REGENERATE_GO_FFI` - when set to `1` forces the regeneration of all Go FFI bindings based on their current Rust definitions, MUST USE when changing any `go_ffi.rs`

## Build Profiles

Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@

### Changed

- [#7467](https://github.com/ChainSafe/forest/issues/7467): CI now verifies that the committed Go FFI bindings `interop-tests/src/tests/go_app/ffi_gen.go` and `f3-sidecar/ffi_gen.go` stay in sync with their `go_ffi.rs` sources through the `FOREST_REGENERATE_GO_FFI` environment variable, included `mise run lint:go-ffi`.

### Removed

### Fixed
Expand Down
28 changes: 17 additions & 11 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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

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.

🗄️ 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.yml

Repository: 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
    )),
})
PY

Repository: 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="")
PY

Repository: ChainSafe/forest

Length of output: 16134


Make lint:go-ffi immune to FOREST_F3_SIDECAR_FFI_BUILD_OPT_OUT. The task sets FOREST_REGENERATE_GO_FFI=1 but preserves an inherited opt-out. The root build.rs then skips regeneration, and git diff can pass with stale f3-sidecar/ffi_gen.go. Unset or reject the opt-out, or make regeneration override it.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@build.rs` around lines 43 - 50, Update the root build.rs regeneration path
guarded by FOREST_REGENERATE_GO_FFI so it overrides or rejects
FOREST_F3_SIDECAR_FFI_BUILD_OPT_OUT, ensuring ffi_gen.go is regenerated whenever
regeneration is requested and preventing stale generated output from passing
lint:go-ffi.


builder.build();
}

rpc_regression_tests_gen();
Expand Down
6 changes: 6 additions & 0 deletions docs/docs/users/reference/env_variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,12 @@ the binary.
By default, the Go f3-sidecar is built and linked into Forest binary unless environment
variable `FOREST_F3_SIDECAR_FFI_BUILD_OPT_OUT=1` is set.

### `FOREST_REGENERATE_GO_FFI`

Forces the regeneration of all Go FFI bindings in the project when set `FOREST_REGENERATE_GO_FFI=1`.

Used in the CI checks to ensure the current tracked Go FFI bindings are in sync with their respective Rust definitions

### `FOREST_DB_DEV_MODE`

By default, Forest will create a database of its current version or try to
Expand Down
2 changes: 2 additions & 0 deletions f3-sidecar/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

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.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Clarify the fallback binding description.

The phrase current/already generated ones is ambiguous. State that the build uses the existing generated bindings when regeneration is disabled.

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

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@f3-sidecar/README.md` at line 68, Update the Go F3-sidecar FFI binding
generation guidance to state that when FOREST_REGENERATE_GO_FFI is disabled, the
build uses the existing generated bindings.

34 changes: 24 additions & 10 deletions interop-tests/build.rs
Comment thread
EclesioMeloJunior marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,32 @@

fn main() {
println!("cargo::rerun-if-changed=src/tests/go_app");
println!("cargo::rerun-if-changed=src/tests/go_ffi.rs");
println!("cargo::rerun-if-env-changed=FOREST_REGENERATE_GO_FFI");

unsafe {
std::env::set_var("GOWORK", "off");
std::env::set_var("GOFLAGS", "-tags=netgo");
}
rust2go::Builder::default()
.with_go_src("./src/tests/go_app")
// the generated Go file has been committed to the git repository,
// uncomment to regenerate the code locally
// .with_regen_arg(rust2go::RegenArgs {
// src: "./src/tests/go_ffi.rs".into(),
// dst: "./src/tests/go_app/ffi_gen.go".into(),
// ..Default::default()
// })
.build();

let mut builder = rust2go::Builder::default().with_go_src("./src/tests/go_app");

// the generated Go file has been committed to the git repository
// set the var to regenerate the file, CI sets this var to verify freshness.
if is_env_truthy("FOREST_REGENERATE_GO_FFI") {
builder = builder.with_regen_arg(rust2go::RegenArgs {
src: "./src/tests/go_ffi.rs".into(),
dst: "./src/tests/go_app/ffi_gen.go".into(),
..Default::default()
})
}

builder.build();
}

fn is_env_truthy(env: &str) -> bool {
std::env::var(env)
.ok()
.map(|var| matches!(var.to_lowercase().as_str(), "1" | "true" | "yes" | "_yes_"))
.unwrap_or_default()
}
10 changes: 10 additions & 0 deletions mise.toml
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,16 @@ run = '''
golangci-lint run ./f3-sidecar ./interop-tests/src/tests/go_app
'''

[tasks."lint:go-ffi"]
description = "Verify the committed Go FFI bindings are in sync with their go_ffi.rs sources."
run = '''
# Checking the (empty) lib target is enough to run `build.rs`
FOREST_REGENERATE_GO_FFI=1 cargo check -p forest-interop-tests --profile quick
FOREST_REGENERATE_GO_FFI=1 cargo check -p forest-filecoin --profile quick
git diff --exit-code -- interop-tests/src/tests/go_app/ffi_gen.go f3-sidecar/ffi_gen.go \
|| (echo "a committed ffi_gen.go is stale — run 'mise lint:go-ffi' and commit the result"; false)
'''

[tasks."lint:ruby"]
description = "Lint Ruby code using rubocop."
tools.ruby = "latest"
Expand Down
6 changes: 2 additions & 4 deletions src/f3/go_ffi.rs
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)]
Expand All @@ -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

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.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Document the changed public FFI contract.

The public GoF3Node::run method has no method-level rustdoc. Document that bootstrap_epoch and finality are Filecoin epochs encoded as i64 at the Go FFI boundary. Document the meaning of the returned bool.

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

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
bootstrap_epoch: i64,
finality: i64,
/// 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(
bootstrap_epoch: i64,
finality: i64,
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/f3/go_ffi.rs` around lines 17 - 18, Add method-level rustdoc to the
public GoF3Node::run method documenting that bootstrap_epoch and finality
represent Filecoin epochs encoded as i64 across the Go FFI boundary, and clearly
state what the returned bool indicates.

Source: Coding guidelines

f3_root: String,
) -> bool;

Expand Down
Loading