Skip to content

Test for Robert Fiko's idea on how to run SAFE for external contributions - #644

Closed
elbrujohalcon wants to merge 1 commit into
mainfrom
external-safe-check
Closed

elbrujohalcon wants to merge 1 commit into
mainfrom
external-safe-check

Conversation

@elbrujohalcon

Copy link
Copy Markdown
Member

Description

@robertfiko recommended this change to allow SAFE to be run when receiving external contributions (i.e., the issue we had on #635).

To be 100% clear: I'm not sure what the consequences of this change can be, I'm just applying the changes recommended by @robertfiko.

Closes #<issue>.

@robertfiko robertfiko mentioned this pull request Aug 6, 2026
1 task done
@robertfiko

Copy link
Copy Markdown

@elbrujohalcon I've opened a PR against this PR's branch.

To be 100% clear: I'm not sure what the consequences of this change can be, I'm just applying the changes recommended by @robertfiko.

Regarding to your concern: pull_request_target runs workflows with the target repository's permissions and in their context, so if it executes code from an untrusted fork, a malicious PR could harm the repository.
It's much safer if workflows require maintainer approval (just like my PR is also waiting for approval now)

image

@elbrujohalcon

Copy link
Copy Markdown
Member Author

Hummm… I assumed this one would work, even if #645 may not. Any idea of what else to change, @robertfiko ?

@elbrujohalcon

Copy link
Copy Markdown
Member Author

Just in case, this is the CI output:

Run rebar3 safe analyse
===> Skipping certifi v2.15.0 as an app of the same name has already been fetched
===> Skipping erlware_commons v1.4.0 as an app of the same name has already been fetched
===> Verifying dependencies...
===> Fetching katana_code v2.4.3
===> Analyzing applications...
===> Compiling katana_code
===> Analyzing applications...
===> Compiling elvis_core
* Downloading locked SAFE 1.5.0
* Downloading SAFE 1.5.0
* SAFE binary ready
* Using config from .safe/config.json
* running SAFE analysis
SAFE project dir: /home/runner/.safe/data/elvis_core-1d1b61f
SAFE CLI v1.5.0 © Erlang Solutions Ltd.

Error: License string is empty in the SAFE_LICENSE environment variable. Please provide a valid license.
SAFE analysis failed.
===> SAFE analysis failed.
Check the output above for details.
Error: Process completed with exit code 1.

@robertfiko

Copy link
Copy Markdown

Yeah unfortunatelly this does not work
image

@robertfiko

Copy link
Copy Markdown

Even with this setting, what we can do is you can give the license to regular maintainers, to set it up in their fork.
Were thinking about an online license check, that would make things easier for open source maintainers.

@elbrujohalcon

Copy link
Copy Markdown
Member Author

I think we're fine as we are now. Most contributions come from this same repo, anyway… As soon as anybody become regular maintainers, they're granted access to this repo, anyway.

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.

2 participants