Skip to content

Vendor SDK build context containment fixes - #1030

Draft
welteki wants to merge 1 commit into
openfaas:masterfrom
welteki:fix/handler-folder-containment
Draft

welteki wants to merge 1 commit into
openfaas:masterfrom
welteki:fix/handler-folder-containment

Conversation

@welteki

@welteki welteki commented Sep 8, 2026

Copy link
Copy Markdown
Member

WIP: go.mod temporarily replaces go-sdk with a local checkout containing openfaas/go-sdk#40. Use a released SDK version and regenerate vendor before merging.

Description

Vendor the SDK fixes for function names, handler folders and extra-copy paths
used by faas-cli build and faas-cli publish.

Motivation and Context

Fix three build context containment issues: function names could cause deletion
of directories outside the build root, template handler_folder values could
write files outside the function's build context, and configuration.copy could
read files from sibling directories whose names merely start with the project
directory name. These paths are now rejected.

  • I have raised an issue to propose this change (required)

How Has This Been Tested?

Tested multiple E2E cases across build/publish --shrinkwrap: reproduced the
failures on master, verified rejection and outside-file preservation, and checked
valid handler folders and extra-copy paths.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Update the vendored go-sdk to reject function names that could delete
directories outside the build root and handler folders that could write
files outside the function's build context.

Use path-aware containment for extra-copy paths to reject sibling
directories whose names merely start with the project directory name.

These fixes apply to both faas-cli build and faas-cli publish.

Signed-off-by: Han Verstraete (OpenFaaS Ltd) <han@openfaas.com>
@reviewfn

reviewfn Bot commented Sep 8, 2026

Copy link
Copy Markdown

AI Pull Request Overview

Disclaimer: This review was generated by automated AI and may contain errors. Do not trust its outputs without human verification.

Summary

  • The PR vendors build-context containment fixes from github.com/openfaas/go-sdk.
  • The scoped non-vendor change leaves go.mod pointing to a sibling checkout with replace github.com/openfaas/go-sdk => ../go-sdk.
  • That local replacement makes normal consumers and CI dependent on filesystem state outside the repository.
  • The PR should not merge until the SDK dependency is updated to a released or otherwise fetchable module version and vendor metadata is regenerated from that version.

Approval rating (1-10)

4/10. The intended security fixes are valuable, but the committed local module replacement is a merge blocker for reproducible builds.

Summary per file

Summary per file
File path Summary
go.mod Adds local replacement for the SDK dependency.

Overall Assessment

The PR appears aimed at closing important build-context containment gaps, but the final state is not ready to merge because it commits a local replace directive. The repository should depend on a released SDK version, or a stable pseudo-version, with vendor metadata generated from that dependency rather than a developer-local sibling directory.

Detailed Review

Detailed Review

Findings

High: Local SDK replacement makes builds non-reproducible

go.mod:63 adds:

replace github.com/openfaas/go-sdk => ../go-sdk

This makes the module graph depend on a sibling directory that is not part of this repository. Any workflow that resolves modules outside the committed vendor tree, or regenerates vendor metadata, now requires /home/ubuntu/workdir/go-sdk-style local state to exist with exactly the expected contents. That breaks reproducibility for contributors and CI, and can cause builds to use a different SDK checkout than the vendored files committed in this PR.

Replace this with a released github.com/openfaas/go-sdk version, or a stable pseudo-version that contains the containment fixes, and regenerate vendor metadata from that module version before merging.

AI agent details.

Agent processing time: 1m4.307s
Environment preparation time: 3.633s
Total time from webhook: 1m14.447s

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.

1 participant