Hacktoberfest 2026: những issue maintainer đã đánh dấu cho tháng Mười, đang mở và phù hợp người mới. Xem issue Hacktoberfest

fix(journal): git_sha records a clean revision for a dirty working tree, contradicting its own contract

Đã đóng
  • Có gì không ổn?
#149 1 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

Chưa có ai nhận issue này.

Đánh giá

Độ khó
3/5
Thời gian dự kiến
1-2 ngày
Mức phù hợp với người mới
55/100
Loại issue
Lỗi
Độ rõ ràng
Khá rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
go
Lĩnh vực
devops

Hướng nghiên cứu

Bắt đầu với internal/onebox/service.go:159 và lần theo các caller của nó tại service.go:139 và cmd/ob/commands.go:680. Xem xét cách internal/compose/payload.go:33 thu thập các tệp trong working tree và cách internal/engine/audit.go:37 hiển thị các giá trị GIT. Được xem là hoàn tất khi các thay đổi payload trong working tree và các thay đổi payload chưa được theo dõi không thể được báo cáo dưới dạng một HEAD SHA sạch đơn thuần, trong khi các thư mục không phải git vẫn giữ nguyên hành vi hiện tại; xác nhận maintainer muốn dùng cách biểu diễn nào trong số các cách được đề xuất.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

bug

Summary

gitShortSHA documents that it omits a dirty revision. It does not. Every operation records the bare HEAD SHA regardless of working-tree state, so ob audit attests a commit for a release whose payload may not match that commit.

The contract, and the code

internal/onebox/service.go:159:

// gitShortSHA is the working tree's revision, recorded on every operation so a
// journal entry can be traced to the code that produced it. An unavailable or
// dirty revision is simply absent rather than guessed at.
func gitShortSHA(ctx context.Context, dir string) string {
	out, err := exec.CommandContext(ctx, "git", "-C", dir, "rev-parse", "--short=7", "HEAD").Output()
	if err != nil {
		return ""
	}
	return strings.TrimSpace(string(out))
}

The comment promises three behaviors. Two hold: unavailable git returns "", and nothing is guessed. The third — that a dirty revision is absent — is not implemented anywhere. There is no working-tree check in this function or its callers (service.go:139, cmd/ob/commands.go:680).

git rev-parse --short=7 HEAD succeeds on a dirty tree. Verified on a scratch repository:

$ echo TAMPERED >> a.txt
$ git status --porcelain
 M a.txt
$ git rev-parse --short=7 HEAD
20da80a
$ echo $?
0

So a dirty tree records a clean SHA. Either the comment or the code is wrong; both cannot stand.

Why it matters

This is not cosmetic, because the payload is copied from the working tree, not from git. StagePayloadContext (internal/compose/payload.go:33) walks bind sources and env files and copies whatever is on disk — including uncommitted modifications and untracked files. The recorded SHA and the shipped bytes are independently determined and can disagree.

The consequence is operator-visible. Engine.Audit (internal/engine/audit.go:37) prints a GIT column per release:

RELEASE                    ACTION   OPERATOR   GIT       OUTCOME   STARTED
20260902-054001-7e6ad74…   deploy   …          7e6ad74   ok        …

An operator reading that row concludes the release contains 7e6ad74. If someone deployed with edits in the tree, it does not, and nothing in the journal, plan, or audit output says so. That is the failure mode docs/product.md is written against — evidence that reads as proof of something it never checked. The product statement promises "evidence-backed execution"; a provenance field that silently rounds "HEAD plus edits" to "HEAD" is the one place that should not round.

Worth being precise about the threat model: product.md already concedes that "a compromised host can lie about its own evidence." This is different and, I think, worse — an uncompromised host recording a misleading fact by construction, with no adversary involved.

Precedent in this codebase

Onebox already treats dirty-state as material, for its own binary:

  • internal/buildinfo/info.go:25 carries Dirty.
  • cmd/ob/version.go:72 renders revision+dirty.
  • cmd/ob/doctor.go:218 raises a warning: "the runner was built from a dirty checkout".
  • internal/engine/status.go:83 appends +dirty to the runner line.

So the standard is established and applied to the runner, while the payload — the thing actually being shipped to production — is exempt. That asymmetry is the argument; nothing here is imported from outside the project's own values.

Proposal

Detect the working-tree state where the SHA is captured, and stop at recording it — no refusal, no policy. Three shapes, in my order of preference:

  1. Suffix, matching version.go. Record 7e6ad74+dirty. Consistent with how the runner already reports itself, needs no schema change, and survives into the audit table's existing GIT column.
  2. Separate field. Add payload_dirty next to git_sha in the journal record and plan artifact, and let each surface render it. Cleaner for machine consumers, at the cost of a field in the record schema.
  3. Honor the comment literally — omit the SHA when dirty. I would not: "based on 7e6ad74 plus edits" is strictly more useful to an operator than "-", and a bare dash is indistinguishable from "not a git repository".

Untracked files should count as dirty. git status --porcelain reports them, and they are copied into the release by the payload walk, so excluding them would reintroduce the same gap in a smaller form.

A follow-on, if 1 or 2 lands: ob doctor could warn on a dirty payload the way it already warns on a dirty runner. That would be a policy addition rather than a correctness fix, so it belongs behind this, not inside it.

Notes

  • Reproduced against 8500ab7.
  • Cost is one git status --porcelain per operation on the config directory, alongside the rev-parse already being run.
  • Non-git directories are unaffected: rev-parse fails, "" is recorded, and no claim is made — that path is already correct.
Ngôn ngữ chính
Go
Star
4
Fork
0
Merge trung bình
2 giờ 46 phút
Pull request đã merge (30 ngày)
38

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của labstack/onebox

Tất cả issue của labstack/onebox

Issue tương tự

Thêm issue về Go

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.