Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 15, 2025

Plan

  • Add workflow_dispatch trigger with version input to build workflow
  • Investigate build failures using GitHub Actions logs
  • Identify root cause: incompatible tar transform syntax in Makefile
  • Fix tar commands to use proper sed-style syntax for --transform flag
  • Identify second issue: VERSION from branch name contains slash
  • Add VERSION sanitization to replace slashes with dashes
  • Test the fixes locally
  • Commit and push changes
  • Run code review and security scan
  • Reply to comments
  • Resolve merge conflicts with base branch

Summary

This PR addresses multiple issues with the build workflow and resolves merge conflicts with the base branch:

1. Added workflow_dispatch Trigger

Added the missing workflow_dispatch trigger with an optional version input parameter to the build workflow. This makes the existing conditional logic for version determination functional, allowing manual workflow runs to specify a custom version.

2. Fixed Incompatible tar Transform Syntax

The --transform flag in the Makefile was using incorrect syntax without proper sed-style delimiters. Updated all 6 tar commands to use 's,^pattern,replacement,' syntax which is compatible with GNU tar on Ubuntu GitHub Actions runners.

3. Resolved Merge Conflicts with Base Branch

Successfully merged with base branch fedora-package and resolved conflicts in the Makefile by combining the VERSION_SAFE approach from the base branch with the corrected tar transform syntax:

  • Kept VERSION_SAFE variable definition that sanitizes VERSION by replacing slashes with dashes in the Makefile
  • Used VERSION_SAFE in all tar/zip filenames to ensure safe filenames regardless of VERSION content
  • Applied corrected tar transform syntax with proper sed-style delimiters
  • Removed redundant VERSION sanitization from the workflow since VERSION_SAFE in the Makefile now handles it, making the Makefile the single source of truth for filename sanitization

All fixes have been validated and passed code review and security scans.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…rkflow

Co-authored-by: mapitman <4205286+mapitman@users.noreply.github.com>
Copilot AI changed the title [WIP] Update Fedora RPM packaging support and CI workflow Add workflow_dispatch trigger to build workflow Dec 15, 2025
Copilot AI requested a review from mapitman December 15, 2025 00:30
@mapitman
Copy link
Owner

@copilot address build failure

Co-authored-by: mapitman <4205286+mapitman@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 15, 2025

@copilot address build failure

Fixed the build failure. The issue was incompatible tar transform syntax in the Makefile. Changed all tar commands from --transform s/pattern/replacement/ to --transform 's,^pattern,replacement,' to use proper sed-style delimiters. Fixed in commit 41fbafb.

@mapitman
Copy link
Owner

@copilot address build failures

Co-authored-by: mapitman <4205286+mapitman@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 15, 2025

@copilot address build failures

Fixed the build failures. The issue was that VERSION was set to the branch name copilot/sub-pr-43 which contains a slash. This caused tar to try creating files like mdview-copilot/sub-pr-43-linux-amd64.tar.gz, treating mdview-copilot/ as a directory path. Added sanitization to replace slashes with dashes: VERSION="${VERSION//\//-}". Fixed in commit fa0745e.

Copilot AI changed the title Add workflow_dispatch trigger to build workflow Fix build workflow failures: add workflow_dispatch trigger and sanitize VERSION Dec 15, 2025
@mapitman
Copy link
Owner

@copilot fix merge conflicts

Copilot AI and others added 2 commits December 15, 2025 01:49
Resolved conflicts in Makefile by combining VERSION_SAFE approach
from base branch with corrected tar transform syntax.

Changes:
- Keep VERSION_SAFE variable definition for sanitizing VERSION
- Use VERSION_SAFE in all tar/zip filenames
- Use corrected tar transform syntax with proper sed-style delimiters
- Merge other changes from fedora-package branch
VERSION_SAFE in Makefile now handles slash sanitization, making
the workflow-level sanitization redundant. This approach is cleaner
as the Makefile is the single source of truth for filename sanitization.

Co-authored-by: mapitman <4205286+mapitman@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 15, 2025

@copilot fix merge conflicts

Fixed merge conflicts with base branch fedora-package. Resolved conflicts in Makefile by combining the VERSION_SAFE approach from the base branch with the corrected tar transform syntax. Also removed redundant VERSION sanitization from the workflow since VERSION_SAFE in the Makefile now handles it. Fixed in commits 6a443e4 and e352cda.

Copilot AI changed the title Fix build workflow failures: add workflow_dispatch trigger and sanitize VERSION Fix build workflow failures, add workflow_dispatch trigger, and resolve merge conflicts Dec 15, 2025
@mapitman mapitman marked this pull request as ready for review December 15, 2025 01:55
Copilot AI review requested due to automatic review settings December 15, 2025 01:55
@mapitman mapitman merged commit aa9347b into fedora-package Dec 15, 2025
3 checks passed
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes build workflow failures by correcting tar transform syntax and adding manual workflow trigger capability. The changes resolve incompatibility issues with GNU tar on Ubuntu GitHub Actions runners by using proper sed-style delimiters. The PR also merges changes from the base branch that introduced VERSION_SAFE sanitization.

Key Changes

  • Added workflow_dispatch trigger to enable manual builds with custom version input
  • Fixed tar --transform flag syntax across 6 build targets by adding proper sed-style delimiters ('s,^pattern,replacement,')
  • Integrated VERSION_SAFE variable usage for filesystem-safe filenames in tar archives

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
.github/workflows/build.yml Added workflow_dispatch trigger with optional version input parameter to support manual workflow runs
Makefile Fixed tar --transform syntax in 6 build targets (linux-amd64, linux-i386, linux-arm64, darwin-amd64, darwin-arm64, freebsd-amd64) by adding proper sed-style delimiters and quotes

The changes are well-implemented and address the root cause of the build failures. The tar transform syntax now correctly uses comma delimiters with proper quoting, which is compatible with GNU tar. The workflow_dispatch addition enables manual testing of builds with custom versions. All changes are consistent across the affected targets and align with the VERSION_SAFE sanitization approach from the merged base branch.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +45 to +55
tar czvf mdview-$(VERSION_SAFE)-linux-amd64.tar.gz --transform 's,^linux-amd64,mdview-$(VERSION_SAFE),' -C bin linux-amd64

bin/linux-i386/mdview:
env GOOS=linux GOARCH=386 go build -buildvcs=false -ldflags "-X main.appVersion=$(VERSION)" -o ./bin/linux-i386/mdview
cp mdview.1 bin/linux-i386/
tar czvf mdview-$(VERSION_SAFE)-linux-i386.tar.gz --transform s/linux-i386/mdview-$(VERSION_SAFE)/ -C bin linux-i386
tar czvf mdview-$(VERSION_SAFE)-linux-i386.tar.gz --transform 's,^linux-i386,mdview-$(VERSION_SAFE),' -C bin linux-i386

bin/linux-arm64/mdview:
env GOOS=linux GOARCH=arm64 go build -buildvcs=false -ldflags "-X main.appVersion=$(VERSION)" -o ./bin/linux-arm64/mdview
cp mdview.1 bin/linux-arm64/
tar czvf mdview-$(VERSION_SAFE)-linux-arm64.tar.gz --transform s/linux-arm64/mdview-$(VERSION_SAFE)/ -C bin linux-arm64
tar czvf mdview-$(VERSION_SAFE)-linux-arm64.tar.gz --transform 's,^linux-arm64,mdview-$(VERSION_SAFE),' -C bin linux-arm64
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of $(VERSION_SAFE) unquoted in these tar commands makes the build vulnerable to shell command injection if VERSION can be influenced (for example via a workflow version input or a branch name). VERSION_SAFE only replaces /, so characters like ;, &, backticks or ' can break the command line or the quoted --transform argument and execute arbitrary commands in the build environment. To mitigate this, constrain VERSION/VERSION_SAFE to a strict safe character set before using it in shell commands (or avoid shell interpretation entirely) and apply the same hardening to all similar packaging commands.

Copilot uses AI. Check for mistakes.
Comment on lines +64 to +74
tar czvf mdview-$(VERSION_SAFE)-darwin-amd64.tar.gz --transform 's,^darwin-amd64,mdview-$(VERSION_SAFE),' -C bin darwin-amd64

bin/darwin-arm64/mdview:
env GOOS=darwin GOARCH=arm64 go build -buildvcs=false -ldflags "-X main.appVersion=$(VERSION)" -o ./bin/darwin-arm64/mdview
cp mdview.1 bin/darwin-arm64/
tar czvf mdview-$(VERSION_SAFE)-darwin-arm64.tar.gz --transform s/darwin-arm64/mdview-$(VERSION_SAFE)/ -C bin darwin-arm64
tar czvf mdview-$(VERSION_SAFE)-darwin-arm64.tar.gz --transform 's,^darwin-arm64,mdview-$(VERSION_SAFE),' -C bin darwin-arm64

bin/freebsd-amd64/mdview:
env GOOS=freebsd GOARCH=amd64 go build -buildvcs=false -ldflags "-X main.appVersion=$(VERSION)" -o ./bin/freebsd-amd64/mdview
cp mdview.1 bin/freebsd-amd64/mdview
tar czvf mdview-$(VERSION_SAFE)-freebsd-amd64.tar.gz --transform s/freebsd-amd64/mdview-$(VERSION_SAFE)/ -C bin freebsd-amd64
tar czvf mdview-$(VERSION_SAFE)-freebsd-amd64.tar.gz --transform 's,^freebsd-amd64,mdview-$(VERSION_SAFE),' -C bin freebsd-amd64
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, $(VERSION_SAFE) is interpolated directly into these tar invocations without strong sanitization or quoting, so an attacker who can control VERSION (e.g. via workflow inputs or crafted ref names) can inject shell metacharacters and gain code execution during the build. Because VERSION_SAFE only removes /, values containing ;, &, backticks, spaces or ' can break the command line and --transform expression and run arbitrary commands. This should be fixed by restricting VERSION/VERSION_SAFE to a safe regex (e.g. alphanumerics plus a small set of delimiters) before using it in shell commands, and by reviewing other archive/packaging commands in this Makefile for the same pattern.

Copilot uses AI. Check for mistakes.
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