feat: reject binary files in AddFile by k1LoW · Pull Request #107 · k1LoW/mo

k1LoW · 2026-03-11T01:34:56Z

Summary

Add text-only file check to AddFile using Git's NUL byte detection heuristic (first 8KB)
Binary files are rejected with ErrBinaryFile; non-regular files (FIFO, device, socket) are also rejected
Non-existent files are allowed through in internal flows (glob watch, backup restore) but HTTP API endpoints (POST /_/api/files, POST /_/api/files/open) still validate file existence via os.Stat before calling AddFile
All entry points (CLI, HTTP API, glob patterns, backup restore) go through the same AddFile check
Server startup fails with an error when all files (including restored session files) are skipped and no patterns or uploaded files exist

Add a text-only check using the same heuristic as Git (NUL byte detection in the first 8KB). Binary files are rejected with ErrBinaryFile. Non-existent files are still allowed through.

- Use io.EOF instead of string comparison for EOF check - Use bytes.IndexByte instead of bytes.ContainsRune for NUL detection - Check for duplicate files before binary check to avoid unnecessary I/O

- Use errors.Is(err, io.EOF) instead of direct comparison - Check AddFile error return values in tests

Copilot

Pull request overview

This PR adds binary-file detection to the Go server’s AddFile path so non-text files can be rejected early (using Git’s “NUL byte in first 8KB” heuristic), and updates callers/tests to handle the new error-returning API.

Changes:

Introduce ErrBinaryFile, an isBinaryFile helper, and change State.AddFile to return (*FileEntry, error).
Update CLI/server entry points (glob expansion, HTTP endpoints, startup) to handle AddFile errors and skip/log when appropriate.
Update server tests to use temp files and add coverage for binary rejection + non-existent path behavior.

Reviewed changes

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

File	Description
`internal/server/server.go`	Adds binary detection + error-returning `AddFile`; updates pattern/glob + HTTP handlers to handle errors.
`cmd/root.go`	Updates server startup file loading to handle `AddFile` errors (skip + warn).
`internal/server/server_test.go`	Updates existing backup tests to use temp files; adds a test for binary rejection and non-existent file behavior.

You can also share your feedback on Copilot code review. Take the survey.

internal/server/server.go

Reorder checks so non-EOF errors (e.g. EISDIR for directories) are returned even when n==0, instead of silently passing the binary check.

Copilot

Pull request overview

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

You can also share your feedback on Copilot code review. Take the survey.

cmd/root.go

internal/server/server.go

- Fix import sort order to pass gofmt - Error when all CLI-specified files are skipped - Add handler-level tests for POST /_/api/files with binary/text files

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

You can also share your feedback on Copilot code review. Take the survey.

cmd/root.go

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

You can also share your feedback on Copilot code review. Take the survey.

internal/server/server.go

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

You can also share your feedback on Copilot code review. Take the survey.

internal/server/server.go

Stat the path before opening to ensure it is a regular file. This prevents blocking reads on FIFOs, device files, and sockets.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

You can also share your feedback on Copilot code review. Take the survey.

cmd/root.go

Copilot

Pull request overview

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

You can also share your feedback on Copilot code review. Take the survey.

cmd/root.go

internal/server/server.go

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

You can also share your feedback on Copilot code review. Take the survey.

internal/server/server_test.go

Copilot

Pull request overview

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

You can also share your feedback on Copilot code review. Take the survey.

internal/server/server.go

internal/server/server_test.go

Copilot

Pull request overview

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

You can also share your feedback on Copilot code review. Take the survey.

internal/server/server.go

cmd/root.go

github-actions · 2026-03-11T07:28:56Z

Code Metrics Report

	main (b91e1b2)	#107 (71cfe54)	+/-
Coverage	51.5%	52.1%	+0.6%
Code to Test Ratio	1:0.5	1:0.5	-0.1
Test Execution Time	37s	36s	-1s

Details

  |                     | main (b91e1b2) | #107 (71cfe54) |  +/-  |
  |---------------------|----------------|----------------|-------|
+ | Coverage            |          51.5% |          52.1% | +0.6% |
  |   Files             |             36 |             36 |     0 |
  |   Lines             |           2819 |           2886 |   +67 |
+ |   Covered           |           1454 |           1506 |   +52 |
- | Code to Test Ratio  |          1:0.5 |          1:0.5 |  -0.1 |
  |   Code              |           4363 |           4427 |   +64 |
+ |   Test              |           2589 |           2590 |    +1 |
+ | Test Execution Time |            37s |            36s |   -1s |

Code coverage of files in pull request scope (50.2% → 51.2%)

Files	Coverage	+/-	Status
cmd/root.go	22.9%	-0.3%	modified
internal/server/server.go	72.4%	+1.0%	modified

Reported by octocov

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

You can also share your feedback on Copilot code review. Take the survey.

k1LoW added 3 commits March 11, 2026 10:34

feat: reject binary files in AddFile

f24bb62

Add a text-only check using the same heuristic as Git (NUL byte detection in the first 8KB). Binary files are rejected with ErrBinaryFile. Non-existent files are still allowed through.

refactor: improve isBinaryFile correctness and AddFile efficiency

41d7345

- Use io.EOF instead of string comparison for EOF check - Use bytes.IndexByte instead of bytes.ContainsRune for NUL detection - Check for duplicate files before binary check to avoid unnecessary I/O

fix: address lint errors in binary file check

ea0555e

- Use errors.Is(err, io.EOF) instead of direct comparison - Check AddFile error return values in tests

This comment has been minimized.

Sign in to view

k1LoW requested a review from Copilot March 11, 2026 01:46

Copilot started reviewing on behalf of k1LoW March 11, 2026 01:47 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

internal/server/server.go Outdated Show resolved Hide resolved

internal/server/server.go Show resolved Hide resolved

fix: check read error before n==0 in isBinaryFile

8256d49

Reorder checks so non-EOF errors (e.g. EISDIR for directories) are returned even when n==0, instead of silently passing the binary check.

k1LoW requested a review from Copilot March 11, 2026 02:05

Copilot started reviewing on behalf of k1LoW March 11, 2026 02:06 View session

This comment has been minimized.

Sign in to view

Copilot AI reviewed Mar 11, 2026

View reviewed changes

cmd/root.go Show resolved Hide resolved

internal/server/server.go Show resolved Hide resolved

internal/server/server.go Outdated Show resolved Hide resolved

fix: address PR review comments

9268674

- Fix import sort order to pass gofmt - Error when all CLI-specified files are skipped - Add handler-level tests for POST /_/api/files with binary/text files

This comment has been minimized.

Sign in to view

k1LoW requested a review from Copilot March 11, 2026 05:35

Copilot started reviewing on behalf of k1LoW March 11, 2026 05:35 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

cmd/root.go Outdated Show resolved Hide resolved

fix: use generic error message when all files are skipped

b4b34df

This comment has been minimized.

Sign in to view

k1LoW requested a review from Copilot March 11, 2026 06:22

Copilot started reviewing on behalf of k1LoW March 11, 2026 06:23 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

internal/server/server.go Outdated Show resolved Hide resolved

perf: use stack-allocated buffer in isBinaryFile

75636f1

k1LoW requested a review from Copilot March 11, 2026 06:32

Copilot started reviewing on behalf of k1LoW March 11, 2026 06:32 View session

This comment has been minimized.

Sign in to view

k1LoW self-assigned this Mar 11, 2026

Copilot AI reviewed Mar 11, 2026

View reviewed changes

internal/server/server.go Show resolved Hide resolved

fix: reject non-regular files in isBinaryFile

8d8e642

Stat the path before opening to ensure it is a regular file. This prevents blocking reads on FIFOs, device files, and sockets.

k1LoW requested a review from Copilot March 11, 2026 06:39

This comment has been minimized.

Sign in to view

Copilot AI reviewed Mar 11, 2026

View reviewed changes

cmd/root.go Outdated Show resolved Hide resolved

fix: only fail when all files skipped and no patterns or uploads exist

1d266ac

k1LoW requested a review from Copilot March 11, 2026 06:54

Copilot started reviewing on behalf of k1LoW March 11, 2026 06:54 View session

This comment has been minimized.

Sign in to view

Copilot AI reviewed Mar 11, 2026

View reviewed changes

cmd/root.go Outdated Show resolved Hide resolved

internal/server/server.go Show resolved Hide resolved

fix: track successful pattern registrations for skip guard

84fa06e

k1LoW requested a review from Copilot March 11, 2026 07:01

Copilot started reviewing on behalf of k1LoW March 11, 2026 07:02 View session

This comment has been minimized.

Sign in to view

Copilot AI reviewed Mar 11, 2026

View reviewed changes

internal/server/server_test.go Show resolved Hide resolved

test: add test for rejecting non-regular files in AddFile

4b0e52a

k1LoW requested a review from Copilot March 11, 2026 07:07

Copilot started reviewing on behalf of k1LoW March 11, 2026 07:07 View session

This comment has been minimized.

Sign in to view

Copilot AI reviewed Mar 11, 2026

View reviewed changes

internal/server/server.go Outdated Show resolved Hide resolved

internal/server/server_test.go Outdated Show resolved Hide resolved

refactor: use RLock for duplicate check and assert nil entry in test

052e68b

k1LoW requested a review from Copilot March 11, 2026 07:15

Copilot started reviewing on behalf of k1LoW March 11, 2026 07:15 View session

This comment has been minimized.

Sign in to view

Copilot AI reviewed Mar 11, 2026

View reviewed changes

internal/server/server.go Show resolved Hide resolved

cmd/root.go Show resolved Hide resolved

fix: distinguish not-found from other errors in handleOpenFile

5dc7146

k1LoW requested a review from Copilot March 11, 2026 07:27

Copilot started reviewing on behalf of k1LoW March 11, 2026 07:28 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

k1LoW merged commit bb29e6c into main Mar 11, 2026
7 checks passed

k1LoW deleted the text-only branch March 11, 2026 08:11

github-actions bot mentioned this pull request Mar 11, 2026

Release for v0.18.2 #106

Merged

Conversation

k1LoW commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Mar 11, 2026

Code Metrics Report

Code coverage of files in pull request scope (50.2% → 51.2%)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

k1LoW commented Mar 11, 2026 •

edited

Loading