Skip to content

fix(sync-workspace): allow pre-wsId flat branch past init guard (closes #1961 more) #1997

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
john-the-dev wants to merge 3 commits into from
Closed

fix(sync-workspace): allow pre-wsId flat branch past init guard (closes #1961 more) #1997

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ced2cc2
fix(sync-workspace): allow pre-wsId flat branch past init guard (clos…
john-the-dev Jul 7, 2026
6fb1bf8
Merge branch 'main' into fix/sync-workspace-prewsid-init-guard
john-the-dev Jul 7, 2026
b5b0870
fix(sync-workspace): require a real whitelist on the pre-wsId branch …
john-the-dev Jul 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions scripts/sync-workspace.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,12 +637,18 @@ _snapshot_per_host_config() {
# v0.8 migration report (2026-06-06): plain `bash scripts/sync-workspace.sh`
# silent-committed an uninitialized state.
#
# Sentinels we accept as proof of a real init (either suffices):
# Sentinels we accept as proof of a real init (any one suffices):
# 1. `.git/info/exclude` contains the generator marker from
# `_compose_exclude_content()` — proves we wrote the whitelist.
# 2. `.sutando-vault/ws-id` — proves _init_impl reached its ws-id step (PR #1459).
# 3. HEAD is on a flat `host/<hostname>` branch (no wsId segment) — pre-wsId init
# that _migrate_flat_branch will upgrade; ws-id is written later in the same run.
# GUARDED: this fast path ALSO requires a real whitelist to exist first (either
# the `.git/info/exclude` marker OR the pre-#1459 in-tree `.gitignore` deny-all),
# so a half-initialized repo — HEAD parked on `host/<hostname>` but with NO
# carrier-set filter — cannot ride the branch name past the guard into `git add -A`.
#
# If neither is present, refuse with a clear error pointing to --init.
# If none is present, refuse with a clear error pointing to --init.
# Override: `SUTANDO_SYNC_SKIP_INIT_GUARD=1` for an operator who knows what
# they're doing (e.g. resurrecting a pre-marker init from before this fix).
_assert_sync_initialized() {
Expand All @@ -651,12 +657,33 @@ _assert_sync_initialized() {

local _exclude="$WORKSPACE_DIR/.git/info/exclude"
local _wsid="$WORKSPACE_DIR/.sutando-vault/ws-id"
local _has_exclude_marker=0
if [ -f "$_exclude" ] && grep -q "Generated by sync-workspace.sh" "$_exclude" 2>/dev/null; then
_has_exclude_marker=1
return 0
fi
if [ -f "$_wsid" ]; then
return 0
fi
# Pre-wsId flat branch: HEAD is `host/<hostname>` with no wsId segment.
# _migrate_flat_branch will carry history into the wsId branch and _host_ws_segment
# will write .sutando-vault/ws-id before push-only runs — so this is safe to allow.
#
# BUT only when a real whitelist already exists — otherwise a half-initialized
# repo (HEAD parked on `host/<hostname>` but no carrier-set filter) would ride the
# branch name straight into `git add -A` and commit the WHOLE workspace tree.
# The exclude marker is checked above (returns 0 when present); here we also accept
# the pre-#1459 in-tree `.gitignore` deny-all, which was that era's whitelist and is
# exactly what a genuine pre-wsId workspace carries (see _migrate_flat_branch).
local _cur_branch
_cur_branch="$(git -C "$WORKSPACE_DIR" symbolic-ref --short HEAD 2>/dev/null || true)"
if [ "$_cur_branch" = "host/$(_host)" ]; then

@qingyun-wu qingyun-wu Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Automated review: this new branch-name fast path weakens the init guard enough to reintroduce the "commit the whole workspace" failure mode it was added to prevent. An unrelated git repo (or any half-initialized workspace) can already have HEAD on host/<hostname> without ever having the generated .git/info/exclude or a ws-id; in that case _push_only_impl now proceeds to git add -A with no carrier-set filter. I think this should only bypass the guard if the flat branch is accompanied by some sync-owned marker, or after _migrate_flat_branch has proved it is a real pre-wsId vault repo.

local _legacy_gitignore="$WORKSPACE_DIR/.gitignore"
if [ "$_has_exclude_marker" = "1" ] || \
{ [ -f "$_legacy_gitignore" ] && grep -qE '^\*[[:space:]]*$' "$_legacy_gitignore" 2>/dev/null; }; then
return 0
fi
fi
die "${_caller}: $WORKSPACE_DIR has .git but sync was never initialized (no whitelist marker in .git/info/exclude and no .sutando-vault/ws-id). Refusing to push — git add -A here would commit the WHOLE workspace tree with NO carrier-set filter. Run: bash scripts/sync-workspace.sh --init (or set SUTANDO_SYNC_SKIP_INIT_GUARD=1 to bypass at your own risk)"
}

Expand Down
49 changes: 49 additions & 0 deletions tests/sync-workspace-uninit-guard.test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,32 @@ TMP_E2E="$(mktemp -d -t sutando-sync-uninit-e2e.XXXXXX)"
# Re-implement the helper inline. Mirror of scripts/sync-workspace.sh.
# Kept in lockstep with the script — if these diverge, structural tests
# 5+6 still catch missing marker/sentinel grep in the script.
_host() { echo "${SUTANDO_HOST_OVERRIDE:-testhost}"; }
_assert_sync_initialized() {
local _caller="${1:-sync}"
[ "${SUTANDO_SYNC_SKIP_INIT_GUARD:-0}" = "1" ] && return 0
local _exclude="$WORKSPACE_DIR/.git/info/exclude"
local _wsid="$WORKSPACE_DIR/.sutando-vault/ws-id"
local _has_exclude_marker=0
if [ -f "$_exclude" ] && grep -q "Generated by sync-workspace.sh" "$_exclude" 2>/dev/null; then
_has_exclude_marker=1
return 0
fi
if [ -f "$_wsid" ]; then
return 0
fi
# Pre-wsId flat branch fast path — but only when a real whitelist exists,
# so a half-init repo (branch set, no filter) can't ride the branch name
# into git add -A. Mirror of scripts/sync-workspace.sh.
local _cur_branch
_cur_branch="$(git -C "$WORKSPACE_DIR" symbolic-ref --short HEAD 2>/dev/null || true)"
if [ "$_cur_branch" = "host/$(_host)" ]; then
local _legacy_gitignore="$WORKSPACE_DIR/.gitignore"
if [ "$_has_exclude_marker" = "1" ] || \
{ [ -f "$_legacy_gitignore" ] && grep -qE '^\*[[:space:]]*$' "$_legacy_gitignore" 2>/dev/null; }; then
return 0
fi
fi
echo "ERR: ${_caller}: $WORKSPACE_DIR has .git but sync was never initialized"
return 1
}
Expand Down Expand Up @@ -117,6 +132,40 @@ if [ "$?" != "0" ]; then
echo " FAIL: T11 — SUTANDO_SYNC_SKIP_INIT_GUARD=1 did NOT bypass guard"; fail=1
fi

# Test 12: pre-wsId flat branch WITH a whitelist (.gitignore deny-all) → passes.
# Genuine pre-#1459 layout: HEAD on host/<host>, in-tree .gitignore carries the
# `*` deny-all whitelist. _migrate_flat_branch upgrades it; ws-id written later.
mkdir -p "$TMP_E2E/flat-ok"
(
cd "$TMP_E2E/flat-ok"
git init -q
git symbolic-ref HEAD "refs/heads/host/$(SUTANDO_HOST_OVERRIDE=flathost; echo flathost)"
printf '*\n!notes/\n' > .gitignore
)
WORKSPACE_DIR="$TMP_E2E/flat-ok"
if ! SUTANDO_HOST_OVERRIDE=flathost _assert_sync_initialized "test" >/dev/null 2>&1; then
echo " FAIL: T12 — pre-wsId flat branch with .gitignore whitelist rejected"; fail=1
fi

# Test 13 (reviewer qingyun-wu, PR #1997): half-initialized repo — HEAD parked on
# host/<host> but NO whitelist (no .git/info/exclude marker, no in-tree .gitignore).
# The branch-name fast path MUST NOT let this ride past into `git add -A`.
mkdir -p "$TMP_E2E/flat-halfinit"
(
cd "$TMP_E2E/flat-halfinit"
git init -q
git symbolic-ref HEAD "refs/heads/host/flathost"
# deliberately no .gitignore, no .git/info/exclude marker, no ws-id
)
WORKSPACE_DIR="$TMP_E2E/flat-halfinit"
out13="$(SUTANDO_HOST_OVERRIDE=flathost _assert_sync_initialized "test" 2>&1)"
rc13=$?
if [ "$rc13" = "0" ]; then
echo " FAIL: T13 — half-init flat branch (no whitelist) slipped past the guard"; fail=1
fi
echo "$out13" | grep -q "never initialized" \
|| { echo " FAIL: T13 — half-init error message missing 'never initialized'"; fail=1; }

# Cleanup
rm -rf "$TMP_E2E"

Expand Down
Loading