Place the managed cron block first

.config/dot/merge-hooks.d/cron/README.md

@@ -9,3 +9,19 @@ are:
 
 The executable hook implementation lives at
 `~/.local/lib/dotfiles/merge-hooks.d/cron.sh`.
+
+The managed block is placed at the top of the account crontab. Its `SHELL`,
+`BASH_ENV`, `PATH`, and any other variable assignments apply to all later jobs,
+including unmanaged jobs. A later unmanaged assignment overrides that value
+for jobs below it. Other accounts, system crontabs, and `/etc/cron.d` files
+have independent environments.
+
+On the first update from the earlier layout, an unchanged block at the bottom
+is moved to the top. An unmanaged assignment that previously preceded the
+managed block (for example `MAILTO` or `CRON_TZ`) no longer applies to managed
+jobs; it still applies to subsequent unmanaged jobs. Put a value intended for
+managed jobs in a managed fragment. Ambiguous, duplicate or incomplete managed
+markers cause the update to leave the crontab untouched for manual repair.
+Before replacing a crontab the hook re-reads it and stops on a concurrent
+change; crontab does not provide an atomic compare-and-swap, so a writer can
+still race after that check.

.config/shell/README.md

@@ -65,8 +65,10 @@ every shell regardless of mode.
 
 Cron jobs are fill-only children too. The cron merge hook writes `SHELL` (an
 absolute bash) and `BASH_ENV` (this loader) into the managed crontab block
-ahead of its `PATH`, so jobs see the same `env.d` values as agent and script
-shells instead of cron's bare `/bin/sh` environment. The crontab's `PATH`
+ahead of its `PATH` at the top of the crontab, so managed and subsequent
+unmanaged jobs see the same `env.d` values as agent and script shells instead
+of cron's bare `/bin/sh` environment. Later unmanaged assignments may override
+these values for subsequent jobs. The crontab's `PATH`
 arrives as an inherited value. When bash or the loader is missing, the hook
 leaves cron on its default shell.
 

.local/lib/dotfiles/doctor.d/lib/cron.sh

@@ -188,6 +188,9 @@ _dr_check_cron() {
 
   if [[ -z $marker || -z $mode ]]; then
     _dr_skip "managed cron block unchecked" "the cron merge hook could not load"
+  elif ((listed != 2)) && ! _cron_block_valid "$marker" "$crontab_out"; then
+    _dr_hint_row warn "managed cron block is malformed" \
+      "ambiguous markers leave the crontab unchanged" "inspect and repair its markers manually"
   elif [[ -z $expected ]]; then
     if [[ $crontab_out != *"$marker begin"* ]]; then
       _dr_ok "no tracked cron entries apply to this host"
@@ -204,7 +207,7 @@ _dr_check_cron() {
     # would send the reader round in circles; crontab's message says why.
     _dr_hint_row warn "crontab is not usable by this account" "$_CRON_LIST_ERR" \
       "ask an administrator to allow crontab for this account (cron.allow or cron.deny, PAM, its setuid/setgid bit), then run 'dot update'"
-  elif [[ $crontab_out == *"$expected"* ]]; then
+  elif [[ $crontab_out == "$expected" || $crontab_out == "$expected"$'\n'* ]]; then
     _dr_ok "managed cron block is current"
   elif [[ $crontab_out == *"$marker begin"* ]]; then
     _dr_hint_row warn "managed cron block is stale" "it differs from the tracked entries" \

.local/lib/dotfiles/merge-hooks.d/cron.sh

@@ -291,36 +291,45 @@ merge() {
     return 1
   fi
 
-  # No active entries — strip any existing managed block and return.
+  # The block stripper is substring-based. Validate even an unchanged or
+  # strip-only block before it can discard ambiguous user-owned entries.
+  if ! _cron_block_valid "$cron_marker" "$current"; then
+    dot_hook_warn "  warning: cron block markers are malformed; crontab left unchanged"
+    return 1
+  fi
+
+  local filtered new_crontab remove=0
+  # No active entries — strip any existing managed block.
   if [[ -z "$managed_block" ]]; then
-    if [[ "$current" == *"$cron_marker begin"* ]]; then
-      local stripped
-      stripped="$(dot_managed_block_strip "$cron_marker" "$current")"
-      if [[ -n "$stripped" ]]; then
-        echo "$stripped" | "$cron_command" -
-      else
-        "$cron_command" -r 2>/dev/null || true
-      fi
+    [[ "$current" == *"$cron_marker begin"* ]] || return 0
+    new_crontab="$(dot_managed_block_strip "$cron_marker" "$current")"
+    [[ -n "$new_crontab" ]] || remove=1
+  else
+    # Position is part of the contract: a legacy block at the bottom must move.
+    if [[ "$current" == "$managed_block" || "$current" == "$managed_block"$'\n'* ]]; then
+      return 0
+    fi
+    filtered="$(dot_managed_block_strip "$cron_marker" "$current")"
+    # Unmanaged assignments remain in place and can override these defaults.
+    if [[ -n "$filtered" ]]; then
+      new_crontab="$managed_block"$'\n\n'"$filtered"
+    else
+      new_crontab="$managed_block"
     fi
-    return 0
   fi
 
-  # Already installed with same content — nothing to do.
-  if [[ "$current" == *"$managed_block"* ]]; then
-    return 0
+  # A second read narrows the window in which a concurrent edit could be lost.
+  # Crontab has no compare-and-swap, so another writer can still race afterward.
+  local latest latest_status=0
+  _cron_list "$cron_command" || latest_status=$?
+  latest=$REPLY
+  if ((latest_status != listed)) || [[ "$latest" != "$current" ]]; then
+    dot_hook_warn "  warning: crontab changed or could not be re-read; crontab left unchanged"
+    return 1
   fi
-
-  # Strip any existing managed block.
-  local filtered
-  filtered="$(dot_managed_block_strip "$cron_marker" "$current")"
-
-  # Append the new managed block.
-  local new_crontab
-  if [[ -n "$filtered" ]]; then
-    new_crontab="$filtered"$'\n\n'"$managed_block"
+  if ((remove)); then
+    "$cron_command" -r
   else
-    new_crontab="$managed_block"
+    printf '%s\n' "$new_crontab" | "$cron_command" -
   fi
-
-  echo "$new_crontab" | "$cron_command" -
 }

.local/lib/dotfiles/merge-hooks.d/lib/cron-list.sh

@@ -24,6 +24,28 @@
 # untranslated. Any other message or exit status (126 and 127 when crontab
 # cannot run) is a failure, the safe side for both callers.
 _CRON_LIST_ERR=
+
+# The generic block stripper matches substrings. Refuse ambiguous input before
+# using it so an incomplete or lookalike marker cannot consume a user's jobs.
+# Accept zero blocks or exactly one complete pair on lines by themselves.
+_cron_block_valid() {
+  local marker="$1" input="$2" line state=0
+  while IFS= read -r line || [[ -n $line ]]; do
+    case "$line" in
+      "$marker begin")
+        ((state == 0)) || return 1
+        state=1
+        ;;
+      "$marker end")
+        ((state == 1)) || return 1
+        state=2
+        ;;
+      *"$marker begin"* | *"$marker end"*) return 1 ;;
+    esac
+  done <<<"$input"
+  ((state != 1))
+}
+
 _cron_list() {
   local sep=$'\x1f''dot-crontab-list'$'\x1f' captured err status line
   # crontab's stdout goes straight to the outer capture (fd 3), its stderr

.local/lib/dotfiles/tests/core/cron.sh

@@ -180,6 +180,36 @@ EOF
     "$("${DOT_TEST_ENV_CLEAN[@]}" HOME="$TEST_HOME" PATH="$cron_job_path:$DOT_TEST_SYSTEM_PATH" \
       SHELL="$cron_shell" BASH_ENV="$cron_bash_env" GH_TOKEN=ghp_job \
       "$cron_shell" -c 'printf "token=[%s]\n" "${GH_TOKEN-}"' 2>&1)"
+  # A top block makes its environment apply to unmanaged jobs too. A later
+  # explicit assignment must remain below the managed default and before its
+  # own job, so the account owner can still override that default.
+  printf '%s\n' '0 1 * * * unmanaged-before-override' \
+    'SHELL=/bin/sh' 'BASH_ENV=/operator/env' 'PATH=/operator/bin' \
+    '0 2 * * * unmanaged-after-override' | crontab -
+  _run_cron_merge 2>/dev/null
+  result=$(crontab -l 2>/dev/null)
+  _assert_eq "unmanaged jobs inherit managed shell, loader and PATH" \
+    'SHELL BASH_ENV PATH' \
+    "$(printf '%s\n' "$result" | awk '
+      /unmanaged-before-override/ { exit }
+      /^SHELL=/ { shell=1 }
+      /^BASH_ENV=/ { env=1 }
+      /^PATH=/ { path=1 }
+      END { if (shell && env && path) print "SHELL BASH_ENV PATH" }
+    ')"
+  _assert_eq "later unmanaged PATH overrides the managed default" \
+    '/operator/bin' \
+    "$(printf '%s\n' "$result" | awk '
+      /^PATH=/ { value=substr($0,6) }
+      /unmanaged-after-override/ { print value; exit }
+    ')"
+  _assert_eq "later unmanaged shell and loader override managed defaults" \
+    '/bin/sh /operator/env' \
+    "$(printf '%s\n' "$result" | awk '
+      /^SHELL=/ { shell=substr($0,7) }
+      /^BASH_ENV=/ { env=substr($0,10) }
+      /unmanaged-after-override/ { print shell, env; exit }
+    ')"
   rm -rf "$TEST_HOME/.config/shell" "$TEST_HOME/.config/gh"
   unset cron_shell cron_bash_env cron_job_path
 
@@ -207,6 +237,95 @@ EOF
   result=$(crontab -l 2>/dev/null)
   _assert_contains "preserves other jobs" "other-job" "$result"
   _assert_contains "installs managed entry" "dot update --cron" "$result"
+  case "$result" in
+    '# dot-managed-cron begin'*) _pass "managed block precedes unmanaged jobs" ;;
+    *) _fail "managed block precedes unmanaged jobs" ;;
+  esac
+
+  # A byte-identical block in the old appended position still needs migration.
+  _legacy_block=$(printf '%s\n' "$result" | awk '
+    /^# dot-managed-cron begin$/ { in_block=1 }
+    in_block { print }
+    /^# dot-managed-cron end$/ { exit }
+  ')
+  printf '%s\n\n%s\n' 'MAILTO=custom@example.invalid' "$_legacy_block" | crontab -
+  _run_cron_merge 2>/dev/null
+  result=$(crontab -l 2>/dev/null)
+  case "$result" in
+    '# dot-managed-cron begin'*) _pass "unchanged legacy block moves to top" ;;
+    *) _fail "unchanged legacy block moves to top" ;;
+  esac
+  _assert_contains "later unmanaged environment remains present" 'MAILTO=custom@example.invalid' "$result"
+  _before_repeat=$result
+  _run_cron_merge 2>/dev/null
+  _assert_eq "top placement is idempotent" "$_before_repeat" "$(crontab -l 2>/dev/null)"
+
+  # A substring-based block stripper must never consume user entries when a
+  # marker is incomplete, duplicated or merely embedded in a command.
+  for _bad_crontab in \
+    $'# dot-managed-cron begin\n0 6 * * * keep-unclosed' \
+    $'# dot-managed-cron end\n0 6 * * * keep-orphan' \
+    $'0 6 * * * echo "# dot-managed-cron begin"\n0 7 * * * keep-embedded' \
+    $'# dot-managed-cron begin\none\n# dot-managed-cron end\n# dot-managed-cron begin\ntwo\n# dot-managed-cron end\n0 7 * * * keep-duplicate'; do
+    printf '%s\n' "$_bad_crontab" | crontab -
+    _bad_status=0
+    _run_cron_merge >/dev/null 2>&1 || _bad_status=$?
+    _assert_eq "malformed marker refuses merge" 1 "$_bad_status"
+    _assert_eq "malformed marker preserves the entire crontab" "$_bad_crontab" "$(crontab -l 2>/dev/null)"
+  done
+  _bad_crontab="$_legacy_block"$'\n# dot-managed-cron begin\n0 8 * * * keep-malformed-tail'
+  printf '%s\n' "$_bad_crontab" | crontab -
+  _bad_status=0
+  _run_cron_merge >/dev/null 2>&1 || _bad_status=$?
+  _assert_eq "valid prefix with malformed tail refuses idempotence" 1 "$_bad_status"
+  _assert_eq "malformed tail remains byte-for-byte" "$_bad_crontab" "$(crontab -l 2>/dev/null)"
+  # The strip-only path is subject to the same protection.
+  printf '%s\n' '# filter: hosts=nonexistent-host-xyz' '0 1 * * * ignored' \
+    >"$TEST_HOME/.config/dot/merge-hooks.d/cron/cron.d/10-base.cron"
+  _bad_crontab=$'# dot-managed-cron begin\n0 6 * * * keep-on-empty'
+  printf '%s\n' "$_bad_crontab" | crontab -
+  _bad_status=0
+  _run_cron_merge >/dev/null 2>&1 || _bad_status=$?
+  _assert_eq "malformed marker refuses empty-source strip" 1 "$_bad_status"
+  _assert_eq "empty-source strip preserves malformed crontab" "$_bad_crontab" "$(crontab -l 2>/dev/null)"
+
+  # Simulate another editor changing the crontab between the first listing
+  # and the final pre-install listing; the merge must not overwrite it.
+  printf '%s\n' '*/30 * * * * dot update --cron' >"$TEST_HOME/.config/dot/merge-hooks.d/cron/cron.d/10-base.cron"
+  _race_bin=$(_mock_bin)
+  _race_state=$(_tmpfile)
+  _race_count=$(_tmpfile)
+  printf '%s\n' '0 1 * * * original' >"$_race_state"
+  : >"$_race_count"
+  cat >"$_race_bin/crontab" <<'SH'
+#!/usr/bin/env bash
+case ${1:-} in
+  -l)
+    if [[ -s $CRON_RACE_COUNT ]]; then
+      printf '%s\n' '0 2 * * * concurrent-edit' >"$CRON_RACE_STATE"
+    else
+      printf '%s\n' seen >"$CRON_RACE_COUNT"
+    fi
+    cat "$CRON_RACE_STATE" ;;
+  -) cat >"$CRON_RACE_STATE" ;;
+  -r) : >"$CRON_RACE_STATE" ;;
+esac
+SH
+  chmod +x "$_race_bin/crontab"
+  _race_status=0
+  DOT_TEST_CRONTAB="$_race_bin/crontab" CRON_RACE_COUNT="$_race_count" \
+    CRON_RACE_STATE="$_race_state" _run_cron_merge >/dev/null 2>&1 || _race_status=$?
+  _assert_eq "concurrent crontab edit refuses install" 1 "$_race_status"
+  _assert_eq "concurrent crontab edit survives" '0 2 * * * concurrent-edit' "$(cat "$_race_state")"
+  printf '%s\n' '# filter: hosts=nonexistent-host-xyz' '0 1 * * * ignored' \
+    >"$TEST_HOME/.config/dot/merge-hooks.d/cron/cron.d/10-base.cron"
+  printf '%s\n' "$_legacy_block" >"$_race_state"
+  : >"$_race_count"
+  _race_status=0
+  DOT_TEST_CRONTAB="$_race_bin/crontab" CRON_RACE_COUNT="$_race_count" \
+    CRON_RACE_STATE="$_race_state" _run_cron_merge >/dev/null 2>&1 || _race_status=$?
+  _assert_eq "concurrent edit refuses strip-only removal" 1 "$_race_status"
+  _assert_eq "concurrent edit survives strip-only removal" '0 2 * * * concurrent-edit' "$(cat "$_race_state")"
 
   # cron.local entries are merged with cron entries
   : >"$MOCK_CRONTAB"

.local/lib/dotfiles/tests/core/doctor.sh

@@ -355,6 +355,22 @@ SH
     _doctor_records _dr_check_cron)
   _assert_contains "doctor cron: a fresh install is current" \
     $'ok\tmanaged cron block is current' "$result"
+  doctor_installed_block=$(cat "$doctor_cron_file")
+  printf '%s\n\n%s\n' '0 1 * * * unmanaged-job' "$doctor_installed_block" >"$doctor_cron_file"
+  result=$(HOME="$TEST_HOME" PATH="$doctor_bin:$PATH" \
+    DOT_TEST_CRONTAB="$doctor_bin/crontab-file" \
+    DOT_TEST_CRONTAB_FILE="$doctor_cron_file" \
+    _doctor_records _dr_check_cron)
+  _assert_contains "doctor cron: unchanged block below unmanaged entries is stale" \
+    $'warn\tmanaged cron block is stale' "$result"
+  printf '%s\n\n%s\n' "$doctor_installed_block" "$doctor_installed_block" >"$doctor_cron_file"
+  result=$(HOME="$TEST_HOME" PATH="$doctor_bin:$PATH" \
+    DOT_TEST_CRONTAB="$doctor_bin/crontab-file" \
+    DOT_TEST_CRONTAB_FILE="$doctor_cron_file" \
+    _doctor_records _dr_check_cron)
+  _assert_contains "doctor cron: duplicate blocks are malformed" \
+    $'warn\tmanaged cron block is malformed' "$result"
+  printf '%s\n' "$doctor_installed_block" >"$doctor_cron_file"
   # The listing keeps crontab's stdout and stderr apart from one run. Each
   # case prints the status, then what was listed (REPLY) or the message.
   cat >"$doctor_bin/crontab-noisy" <<'SH'