🐛 fix unexpected run of release workflow

[zawakin]

I have discovered a bug located within .github/workflows/_release.yml which is the primary cause of continuous integration (CI) errors. The problem can be solved; therefore, I have constructed a PR to address the issue.

The Issue

Access the following link to view the exact errors: Langhain Release Workflow

The instances of these errors take place for each PR that updates pyproject.toml, excluding those specifically associated with bumping PRs.

See below for the specific error message:

Error: Error 422: Validation Failed: {"resource":"Release","code":"already_exists","field":"tag_name"}

An image of the error can be viewed here:

The _release.yml document contains the following if-condition:

    if: |
        ${{ github.event.pull_request.merged == true }}
        && ${{ contains(github.event.pull_request.labels.*.name, 'release') }}

The Root Cause

The above job constantly runs as the if-condition is always identified as true.

The Logic

The if-condition can be defined as if: ${{ b1 }} && ${{ b2 }}, where b1 and b2 are boolean values. However, in terms of condition evaluation with GitHub Actions, ${{ false }} is identified as a string value, thereby rendering it as truthy as per the official documentation.

I have run some tests regarding this behavior within my forked repository. You can consult my debug PR for reference.

Here is the result of the tests:

If-Condition	Outcome
if: true && ${{ false }}	Execution
if: ${{ false }}	Skipped
if: true && false	Skipped
if: false	Skipped
if: ${{ true && false }}	Skipped

In view of the first and second results, we can infer that ${{ false }} can only be interpreted as true for conditions composed of some expressions.
It is consistent that the condition of if: ${{ inputs.working-directory == 'libs/langchain' }} works.

It is surprised to be skipped for the second case but it seems the spec of GitHub Actions 😓

Anyway, the PR would fix these errors, I believe 👍

Could you review this? @hwchase17 or @shoelsch , who is the author of PR.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
langchain	⬜️ Ignored (Inspect)	Visit Preview		Aug 19, 2023 5:31pm

[baskaryan]

cc @obi1kenobi

[zawakin]

(A little background leading to the discovery: I personally like your project and was referencing the release workflow in my personal development. I noticed that there were times when the if-condition for the PyPI release workflow was not functioning properly.)

[zawakin]

Let me add one more point. Both of the following code would work,

github.event.pull_request.merged == true
&& contains(github.event.pull_request.labels.*.name, 'release')

as in this PR,

${{ github.event.pull_request.merged == true && contains(github.event.pull_request.labels.*.name, 'release') }}

[obi1kenobi]

Thanks for the report and for the fix! 🚀

[obi1kenobi]

We have completed triage and remediation for this issue.

TL;DR: This was not exploited. No actions are required for users of LangChain.

The misconfiguration identified here allowed the release workflow to be triggered by an unmerged PR, which could have led to the creation of an unintended langchain or langchain-experimental release on PyPI.

Having reviewed all runs of the release workflows, we have determined that no inappropriate releases took place. We have already merged the fix as well as some additional hardening of our GitHub Actions that will make similar issues impossible in the future.

What went well:

• GitHub prevents first-time contributors from triggering workflows without explicit opt-in from maintainers. This means that any attempts to take advantage of the misconfiguration would have had to either pass code-review or come from an account that has previously contributed to LangChain and has been approved for running workflows. It was thus not possible for a new account to trigger the release workflow without the maintainer explicitly clicking to approve the run first.

What we're improving for the future:

• We're adding a SECURITY.md to the repo and configuring GitHub to ask users to send security-related information to security@langchain.dev: Add SECURITY.md file to the repo. #9551
• We're adding a security.txt file at https://langchain.com/.well-known/security.txt directing security reports to security@langchain.dev.
• If despite these steps, a security-related issue is opened on public GitHub, we will internally escalate it to security@langchain.dev to ensure it is promptly acted upon.
• We are changing the release workflows on all our repos so they can only be triggered on our master branch, which makes it impossible for unmerged PRs to get released: Release on push to master instead of on closed PRs targeting it. #9544
• We're also making releases of LangChain packages require being manually triggered, to give us an additional opportunity to prevent any unintended changes from being published: Require manually triggering release workflows. #9552
• We're adopting PyPI Trusted Publishing to make it even harder to abuse release workflows: Use PyPI Trusted Publishing to publish langchain packages. #9467
• We are further reviewing and hardening our GitHub Actions against other possible kinds of attempted exploitation. More PRs to come.

[zawakin]

@obi1kenobi Thank you! I was really impressed with how quickly you've put together the security.md and the other actions. Wishing you continued improvement and success!