Skip to content

fix: authenticate demo box git pull with the job's own GITHUB_TOKEN - #898

Merged
sakibsadmanshajib merged 7 commits into
mainfrom
fix/deploy-demo-box-auth
Aug 14, 2026
Merged

sakibsadmanshajib merged 7 commits into
mainfrom
fix/deploy-demo-box-auth

Conversation

@sakibsadmanshajib

@sakibsadmanshajib sakibsadmanshajib commented Aug 13, 2026 •

Copy link
Copy Markdown
Owner

Summary

Every deploy-demo-box run since 2026-08-13 has failed at the "Pull latest main" step:

fatal: could not read Username for 'https://github.com': No such device or address
##[error]Process completed with exit code 128.

Exit 128 at the pull means the recreate never ran, so main never actually reached the box: the stack keeps serving whatever was already deployed, silently, behind a red check.

Root cause

The deploy job never checks out the repo itself; it cds into the box's own long-lived clone at /home/sakib/hive and runs git checkout main; git fetch origin; git pull --ff-only origin main. That clone's HTTPS remote authenticates from some credential external to this workflow (a stored PAT or a git credential helper on the box; this job has no shell access to the box outside its own steps to inspect which).

Evidence this is an expired or rotated credential, not a missing config line:

  • Run 31635783176 (2026-08-12 20:04 UTC): the deploy job's own log shows git fetch succeeding with a real ref listing (From https://github.com/sakibsadmanshajib/hive, multiple branch updates), i.e. the credential worked.
  • Run 31704949535 (2026-08-13 13:27 UTC, ~17 hours later): the identical git fetch origin call fails with the Username error above.
  • git log -- .github/workflows/deploy-demo-box.yml between the commits at those two runs (3549971e..7faae2d3) is empty: the workflow file is byte-identical across the gap. Nothing in this repo changed to cause the failure.

That combination (same command, same file, works then stops working with no repo change in between) is a credential that expired or was rotated out from under an unattended, non-interactive git pull, which then hits git's default terminal-prompt path and fails fast since the runner has no tty.

Fix

Stop depending on any credential stored on the box at all. The "Pull latest main" step now authenticates the fetch/pull with this job's own ephemeral GITHUB_TOKEN, via git -c http.extraheader, scoped to the single invocation and never written to disk. This is the same mechanism actions/checkout uses internally. Added contents: read to the job's permissions block (it previously only had actions: read) so the token can read repo contents. Nothing here needs future rotation: the token is minted fresh every run.

Also added:

  • GIT_TERMINAL_PROMPT=0 so any future bad credential fails in one git-native line instead of the opaque "could not read Username / No such device or address" this outage actually produced.
  • An explicit ::error:: on a failed fetch naming the real failure class (credential/permission, not a missing branch) and where to look on the box.

Alternative considered and rejected

Replacing the box's persistent clone with actions/checkout entirely (having the runner's own already-authenticated checkout do the fetch) was considered first, since it's the more architecturally clean fix. Rejected for this PR: every later step in the deploy job assumes /home/sakib/hive is a long-lived directory carrying the box's own untracked .env (every secret this stack runs on, never committed) and Docker build cache. actions/checkout's default clean: true runs git clean -ffdx, which would delete that untracked .env on first use. That is a materially bigger and riskier change than fixing the credential the existing clone already uses, and cannot be safely verified before merge (see Verification below).

Same blast radius, checked

Grepped the repo for every other place a workflow or script pulls the demo box's own clone the same way. Only deploy-demo-box.yml does this in an unattended CI context. scripts/install.sh has a similar git fetch/git checkout main --quiet pair, but it targets /opt/hive for a human-run, interactive Enterprise self-host install (uses prompt_value helpers elsewhere in the same script), not an automated redeploy loop, so it is out of scope for this fix.

Buglog entry

{"error_message": "deploy-demo-box \"Pull latest main\" step: fatal: could not read Username for 'https://github.com': No such device or address (exit 128)", "root_cause": "the demo box's persistent git clone at /home/sakib/hive authenticated its HTTPS remote from a credential external to the workflow (stored PAT or credential helper on the box); that credential worked through the 2026-08-12 20:04 UTC deploy and was gone by the next deploy 17 hours later with no workflow change in between, so it expired or was rotated out from under an unattended git pull with no fallback", "fix": "authenticate the box's own fetch/pull with the deploy job's ephemeral GITHUB_TOKEN via git -c http.extraheader, scoped per-invocation and never persisted to disk, plus GIT_TERMINAL_PROMPT=0 and an explicit ::error:: so a bad credential fails loud instead of an opaque prompt error", "tags": ["ci", "deploy-demo-box", "git", "credentials", "self-hosted-runner"]}

Verification

Verified:

  • YAML parses (python3 -c "import yaml; yaml.safe_load(...)").
  • The auth mechanism (git -c http.https://github.com/.extraheader="AUTHORIZATION: basic ...") matches actions/checkout's own internal implementation for HTTPS token auth, a well established pattern.
  • Root cause timeline above, read directly from the two runs' own logs and from git log on the workflow file across that gap; not guessed.
  • Grepped the full repo for other automated on-box pulls: none besides this one step.

Could not verify (self-hosted runner on a physical box, no local reproduction possible):

  • That the box's job actually receives a contents: read-scoped GITHUB_TOKEN with the permissions this job declares, end to end.
  • That the box's git remote is still exactly https://github.com/sakibsadmanshajib/hive.git (assumed from the existing error message and job comments; the new ::error:: step surfaces git remote -v guidance if this assumption is wrong).
  • That no other on-box state (e.g. a .netrc, a global credential.helper that might intercept before the per-invocation -c flag takes effect) interferes. git -c config takes precedence over global config for the same key, but this is asserted from git's documented config precedence, not observed on the box.

The only real proof is the next deploy-demo-box run on main after this merges. Watch the "Pull latest main" step specifically: it should show a real git fetch/pull ref update (like the 2026-08-12 20:04 run) instead of the Username error, and the deploy should proceed to recreate the stack.

Test plan

  • Merge to main, watch the triggered deploy-demo-box run (gh run watch)
  • Confirm "Pull latest main" step succeeds and shows a real fetch/pull, not the Username error
  • Confirm the box actually recreates containers on the new commit (check container CREATED timestamps in the "Dump container status" step or a follow-up docker compose ps)

Summary by CodeRabbit

  • Chores
    • Improved demo deployment reliability by securely authenticating repository updates.
    • Added validation to ensure deployments update the expected repository.
    • Added clearer error messages for authentication failures and update conflicts.
    • Deployment records the exact commit that was released for easier verification and troubleshooting.

…OKEN

Every deploy-demo-box run since 2026-08-13 has failed at the "Pull latest
main" step with `fatal: could not read Username for 'https://github.com':
No such device or address` (exit 128), so main was never actually reaching
the box: the stack kept serving whatever was already deployed.

The box's persistent clone at /home/sakib/hive authenticated its HTTPS
remote from some credential external to this workflow. That credential
worked through the 2026-08-12 20:04 UTC run (its own log shows a real
fetch, not an error) and was gone by the next deploy 17 hours later, with
the workflow file byte-identical across that gap. That is an expired or
rotated credential, not a missing config line.

Fix: stop depending on any credential stored on the box. The Pull latest
main step now authenticates with this job's own ephemeral GITHUB_TOKEN via
`git -c http.extraheader`, scoped to the single fetch/pull invocation and
never written to disk, the same mechanism actions/checkout uses
internally. Nothing here needs future rotation. GIT_TERMINAL_PROMPT=0 plus
an explicit ::error:: on a failed fetch makes a bad credential fail loud
and specific instead of the opaque prompt error this outage produced.

actions/checkout replacing the box's persistent clone entirely was
considered and rejected: every later step in this job assumes
/home/sakib/hive holds the box's own untracked .env and Docker build
cache, and actions/checkout's default `clean: true` would delete that
.env on first use.

Grepped the repo for other automated pulls on the box: none. scripts/
install.sh has a similar git fetch/checkout pair, but it targets /opt/hive
for a human-run, interactive Enterprise install, not an unattended CI
loop, so it is out of scope here.

Buglog entry (for the follow-up buglog-only PR against main):
{"error_message": "deploy-demo-box \"Pull latest main\" step: fatal: could not read Username for 'https://github.com': No such device or address (exit 128)", "root_cause": "the demo box's persistent git clone at /home/sakib/hive authenticated its HTTPS remote from a credential external to the workflow (stored PAT or credential helper on the box); that credential worked through the 2026-08-12 20:04 UTC deploy and was gone by the next deploy 17 hours later with no workflow change in between, so it expired or was rotated out from under an unattended git pull with no fallback", "fix": "authenticate the box's own fetch/pull with the deploy job's ephemeral GITHUB_TOKEN via git -c http.extraheader, scoped per-invocation and never persisted to disk, plus GIT_TERMINAL_PROMPT=0 and an explicit ::error:: so a bad credential fails loud instead of an opaque prompt error", "tags": ["ci", "deploy-demo-box", "git", "credentials", "self-hosted-runner"]}
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@cursor

cursor Bot commented Aug 13, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@coderabbitai

coderabbitai Bot commented Aug 13, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@sakibsadmanshajib, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 47 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Free

Run ID: 6d98678a-2f5a-4496-8f41-7ff4734bf095

📥 Commits

Reviewing files that changed from the base of the PR and between f41731a and 4d6ee25.

📒 Files selected for processing (1)
  • .github/workflows/deploy-demo-box.yml
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Free

Run ID: fc2952c0-6a67-4ffe-8296-ce89332fbe84

📥 Commits

Reviewing files that changed from the base of the PR and between a9b1d0f and f41731a.

📒 Files selected for processing (1)
  • .github/workflows/deploy-demo-box.yml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/deploy-demo-box.yml

📝 Walkthrough

Walkthrough

The deploy job grants read access to repository contents. Its persistent checkout validates origin and uses the job token for non-interactive authenticated git fetch and fast-forward git pull operations.

Changes

Demo deployment authentication

Layer / File(s) Summary
Authenticate latest-main checkout
.github/workflows/deploy-demo-box.yml
The deploy job adds contents: read permission. The pull step disables terminal prompts, verifies origin, authenticates fetch and fast-forward pull with GITHUB_TOKEN, reports authentication and divergence errors, and records the deployed commit.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to f4173

The change makes the demo deployment pull use the job's temporary repository token and adds clearer failure reporting; no actionable merge-blocking risk remains beyond normal checks and review.


Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login.

Comment @coderabbitai help to get the list of available commands.

CodeRabbit review on PR #898: the previous ::error:: text asserted the
fetch failure is a credential/permission problem specifically, which
would mislead debugging if a future failure is actually a missing ref,
DNS, TLS, or remote-URL issue instead. Point at the preceding git error
output as the real diagnostic source and keep the same two most likely
places to check, without asserting the cause.
Comment thread .github/workflows/deploy-demo-box.yml
@sakibsadmanshajib

Copy link
Copy Markdown
Owner Author

Adversarial review status for this PR (pipeline mode, dynamic selector).

Ran: CodeRabbit CLI (coderabbit review --agent -t all --base main). One minor finding, posted inline and fixed in 28ffb0e, thread resolved.

SKIPPED, structural capability gap: ecc:code-review, the plain adversarial pass, and security-reviewer (added for this diff's credential/token-permission shape, per the mandatory security-path rule) all require dispatching a separate subagent or invoking a Skill. This build-error-resolver subagent's toolset is Read, Write, Edit, and Bash only, with no Agent or Skill tool, so none of those three streams can be invoked from here. Flagging precisely rather than routing around it, per the gate-compliance rule: the orchestrator needs to dispatch those three streams separately before this PR can be treated as having a full pipeline review.

Comment thread .github/workflows/deploy-demo-box.yml Outdated
Comment thread .github/workflows/deploy-demo-box.yml Outdated
Comment thread .github/workflows/deploy-demo-box.yml Outdated
@sakibsadmanshajib

Copy link
Copy Markdown
Owner Author

plain adversarial stream

Read-only pass on .github/workflows/deploy-demo-box.yml at 28ffb0e5, run because the earlier pass on this PR did not land.

Checked:

  • Diagnosis coverage: whether an expired credential, a broken credential helper, and a changed remote URL are distinguishable from the evidence given, and whether the fix covers all three. Expired PAT and broken credential helper: both genuinely fixed, since http.extraheader supplies the Authorization header on the first request, so GitHub accepts it before any on-box credential path is ever consulted. Changed remote URL: not covered, see inline comment.
  • Silent success without advancing to the new commit (the priority in the follow-up ask): git pull --ff-only is structurally safe here, it advances, no-ops, or fails loud on divergence, never silently stalls. Left a suggestion for an explicit post-pull SHA log line for auditability, since nothing in this step states which commit the box ended up on.
  • Whole-job blast radius: every step in the deploy job checked for other git or credential-dependent network calls. Only this one step touches git; nothing later in the job depends on the old on-box credential.
  • Re-runs, non-fast-forward, dirty tree, force-push to main: --ff-only already fails loud on divergence, unchanged behavior from before this PR, not a regression it introduces.
  • Error annotation: the fetch-failure path names the failing command and where to check (permissions block, remote URL). The pull-failure path has no equivalent annotation (see inline LOW finding).
  • permissions.contents: read addition: correctly minimal for what git -c http.extraheader needs.
  • Timeout: the deploy job is still bounded at timeout-minutes: 15, unchanged by this diff. Clean.

Findings: 2 MEDIUM (post-pull commit auditability, remote-identity verification), 1 LOW (inconsistent error annotation on the pull-failure path). No CRITICAL, no HIGH. Nothing here blocks merge on its own.

Comment thread .github/workflows/deploy-demo-box.yml Outdated
Comment thread .github/workflows/deploy-demo-box.yml Outdated
Comment thread .github/workflows/deploy-demo-box.yml Outdated
Adversarial review on PR #898 found the extraheader token authenticates
any public github.com repo, not just this one, so a repointed origin on
the box would pull the wrong source while still reporting success. Check
the remote URL before using the token, and fail loud if it does not match
this repository.

Also add an explicit ::error:: on the pull-failure path (only the fetch
had one) and log the resulting commit SHA after a successful pull, so a
future silent-stale case is visible in the run log rather than requiring
a read of raw git output.
Comment thread .github/workflows/deploy-demo-box.yml Outdated
Comment thread .github/workflows/deploy-demo-box.yml Outdated
Comment thread .github/workflows/deploy-demo-box.yml Outdated
Comment thread .github/workflows/deploy-demo-box.yml
Comment thread .github/workflows/deploy-demo-box.yml Outdated
Comment thread .github/workflows/deploy-demo-box.yml
@sakibsadmanshajib

Copy link
Copy Markdown
Owner Author

plain adversarial stream

Read-only pass on .github/workflows/deploy-demo-box.yml at 28ffb0e5, run because the earlier pass on this PR did not land.

Checked:

  • Diagnosis coverage: whether an expired credential, a broken credential helper, and a changed remote URL are distinguishable from the evidence given, and whether the fix covers all three. Expired PAT and broken credential helper: both genuinely fixed, since http.extraheader supplies the Authorization header on the first request, so GitHub accepts it before any on-box credential path is ever consulted. Changed remote URL: not covered, see inline comment.
  • Silent success without advancing to the new commit (the priority in the follow-up ask): git pull --ff-only is structurally safe here, it advances, no-ops, or fails loud on divergence, never silently stalls. Left a suggestion for an explicit post-pull SHA log line for auditability, since nothing in this step states which commit the box ended up on.
  • Whole-job blast radius: every step in the deploy job checked for other git or credential-dependent network calls. Only this one step touches git; nothing later in the job depends on the old on-box credential.
  • Re-runs, non-fast-forward, dirty tree, force-push to main: --ff-only already fails loud on divergence, unchanged behavior from before this PR, not a regression it introduces.
  • Error annotation: the fetch-failure path names the failing command and where to check (permissions block, remote URL). The pull-failure path has no equivalent annotation (see inline LOW finding).
  • permissions.contents: read addition: correctly minimal for what git -c http.extraheader needs.
  • Timeout: the deploy job is still bounded at timeout-minutes: 15, unchanged by this diff. Clean.

Findings: 2 MEDIUM (post-pull commit auditability, remote-identity verification), 1 LOW (inconsistent error annotation on the pull-failure path). No CRITICAL, no HIGH. Nothing here blocks merge on its own.

…he remote URL

Security review on PR #898 found three HIGH severity issues in the
previous commit's fix and two MEDIUM ones worth acting on rather than
answering with a rebuttal.

HIGH, fixed:
- The Basic-auth header was passed via `git -c ...` on the command line,
  world-readable through /proc/<pid>/cmdline on a stock box for the life
  of the fetch and pull. Switched to GIT_CONFIG_COUNT/KEY_n/VALUE_n
  environment variables, readable only by the same uid or root, same
  effect, no argv exposure.
- The base64 Basic-auth blob derived from the token was never masked.
  GitHub only auto-masks the literal token, not a value derived from it.
  Added an explicit ::add-mask:: on the blob before it is used, the same
  step actions/checkout itself takes for the identical value.
- The remote-URL guard added in the previous commit printed the box's
  origin URL unredacted in its error message. If that URL ever embeds a
  credential, the guard would leak it into the run log while checking for
  exactly this class of problem. Redacted once, reused everywhere the
  value is printed.

MEDIUM, fixed:
- The previous comment claimed the token is never written to disk. False:
  a `${{ }}` expression inside a run: block materializes into a script
  under the runner's temp directory, and this runner is not ephemeral.
  The token now reaches the script only via the GH_TOKEN env var, never
  interpolated into the script text.
- Nothing reset credential.helper, so a green run did not actually prove
  the new mechanism authenticated rather than a leftover on-box
  credential. GIT_CONFIG_KEY_1/VALUE_1 empties it for this invocation.
  Also added three secret-free diagnostic lines (credential.helper state,
  redacted remote URL, presence of ~/.git-credentials) so the next outage
  does not start from the same unknowns this PR's own Verification
  section flagged as unprovable without box access.
- pull --ff-only moves the ref and writes the worktree as two separate
  steps. A run killed mid-pull (this job's own timeout, a cancel, or a
  box reboot) can leave HEAD advanced over a half-written tree that the
  next run reports as current. Added a tracked-file dirty check after the
  pull, plus a leading `rm -f .git/index.lock` for the matching stale-lock
  case (safe: the workflow-level concurrency group serializes this job).
Security review on PR #898 pointed out that contents:read, added for the
Pull latest main fix, also widens the job-scoped GH_TOKEN this later step
already used at only actions:read. Correct and unavoidable at job-level
permissions granularity, just undocumented until now. Also links issue
#902, filed for the separate, pre-existing question of who can trigger
this job via workflow_dispatch.
Comment thread .github/workflows/deploy-demo-box.yml Outdated
Comment thread .github/workflows/deploy-demo-box.yml Outdated
Comment thread .github/workflows/deploy-demo-box.yml Outdated
@sakibsadmanshajib

Copy link
Copy Markdown
Owner Author

security-reviewer stream, second pass on d23b8c53

Delta review of the fixes pushed in response to the first pass, not a re-read of the file. The fixes are themselves credential-handling changes on a workflow that runs on the owner's physical machine, so they get the same treatment the original did. Three new comments posted, all MEDIUM or below. No HIGH remains.

The three HIGH fixes, verified

1. Token out of argv. Verified, and it is complete. git -c is gone from both invocations. GIT_CONFIG_COUNT=2 with GIT_CONFIG_KEY_0 / VALUE_0 is exported at line 325, so it applies to the fetch at line 331 and the pull at line 335 alike, and to every git subprocess either of them spawns, without a second copy of the value anywhere. I also checked the construction line itself, which is where this kind of fix usually leaks back: on line 323 both printf and the pipeline's left side are bash builtins, so no process is created carrying the token in its arguments, and base64 receives it on stdin rather than argv. [ on line 315 is likewise a builtin. Nothing in this step puts a credential on a command line any more.

2. ::add-mask:: on the base64 blob. Verified, ordering is correct. The blob is computed at 323 and masked at 324, before its first use at 327. Nothing earlier in the step touches the token: the three diagnostics at 313 to 315 run before it exists. The registration echo is consumed by the runner as a workflow command rather than emitted as a log line, so registering the mask does not itself disclose it. base64 -w0 matters here and is present, since ::add-mask:: covers only the first line of its value and a wrapped blob would defeat it. The comment block at 238 to 253 now states both of the things it previously got backwards, including that the interpolated value does reach disk in the runner's materialized step script, which is the correct account.

3. redacted_url. Verified, no raw print survives. Redacted once at 301, immediately after git remote get-url. The comparison at 302 still uses the raw value, which is right, since a credential bearing URL must fail that comparison. Both print sites, the error at 305 and the diagnostic at 314, use the redacted value. I checked for a third use of $actual_url in the step and there is none. Two residuals noted inline at LOW: the character class stops at the first @, so a userinfo containing a literal @ leaks its tail, and ~/.netrc is not covered by the presence checks.

The three MEDIUM fixes

Token only via GH_TOKEN. Verified. The step now maps it in env: at 273 and the script reads $GH_TOKEN. The only remaining ${{ }} inside this step's script body is github.repository at 299, which is not a secret. The job's other token use, the agent-engine step at 375, was already env mapped. Nothing in this job interpolates a secret into a script body any more.

Helper reset plus three diagnostics. The reset itself is right: an empty credential.helper value resets the helper list, so a stored PAT or a netrc cannot silently rescue or shadow slot 0, and a failure below is unambiguously this token's failure. That was the point of the original finding and it is met. The diagnostics are where the new comment is. Taking the coordinator's two questions in order:

  • The .git-credentials check at 315 prints a fixed English string and never opens the file. Clean.
  • The helper line at 313 is not clean. --get-all prints the config value, and a credential.helper value may be an inline shell command with a credential embedded in it, which is one of the two mechanisms this PR's own root cause section suspects on this box. --name-only --get-regexp keeps the entire diagnostic (presence, plus which config file) with nothing printable left. Details inline on that line.

Ordering is right and worth recording: the diagnostics run before the GIT_CONFIG_* export, so they report the box's real credential state rather than the empty helper this step installs.

Worktree assertion and the index lock. The git status --porcelain -uno check closes the interrupted fast forward case correctly, catches a leftover half written tree from an earlier run (which is the case that produced a green deploy of the wrong bytes), and -uno keeps the box's untracked .env out of it. The reset --hard recovery guidance in the annotation is accurate about not touching untracked files. The rm -f .git/index.lock above it is the one I would not merge as written: its justification reasons from this workflow's concurrency group, but the lock is scoped to a clone that is the box's shared checkout, with interactive sessions, agents and git's own gc --auto as other writers. Clearing a live lock is worse than the message it avoids. Age gating it with find ... -mmin +15 -delete is the same one line and still clears anything this job can strand, since timeout-minutes: 15 bounds it.

Widened GH_TOKEN scope on the agent-engine step

Justified, not a finding. permissions: has no finer granularity than the job, so a job that needs contents: read for one step grants it to every step in that job. The only way to avoid it is splitting the pull into its own job, which buys almost nothing here: both jobs would land on the same self-hosted runner as the same uid, in reach of the same .env. The consequence is now documented at lines 366 to 372 rather than left implicit, which is what the first pass asked for, and the separate containment question is filed as #902 rather than argued away.

Verdict

The three HIGH findings are properly closed, and closed at the mechanism rather than papered over. What remains is one MEDIUM disclosure in a new diagnostic line (the helper value), one MEDIUM correctness issue in the new index lock removal, and LOW residuals on the redaction pattern and an unguarded empty token. None of them is a reason to keep the box on stale code. Fix the helper line before merge, since it is one flag on one line and it is the same class of leak this PR just finished closing twice; the index lock is a two word change and should ride along.

Security review, second pass on PR #898, confirmed the three HIGH argv/
mask/redaction fixes hold in the actual code, then found three more:

MEDIUM: `git config --show-origin --get-all credential.helper` prints the
helper's value, and a helper can be an inline shell snippet with a PAT
embedded, exactly the on-box credential this PR suspects. Switched to
`--name-only --get-regexp`, which reports that a helper is configured
without printing what it contains.

MEDIUM: the unconditional `rm -f .git/index.lock` could stomp a lock held
by a genuinely running process on this shared checkout (git gc --auto, a
human on the box), since the workflow-level concurrency group only
serializes this workflow's own runs against each other, not every writer
of this clone. Age-gated to `find -mmin +15 -delete`, matching this job's
own timeout-minutes.

LOW: the redaction regex stopped at the first @, which would leave part
of a credential exposed if the secret itself contained an unencoded @.
Made greedy to match the last @ instead. Also added a `.netrc` presence
check alongside the existing `.git-credentials` one, and an explicit
::error:: on an empty GH_TOKEN instead of silently building a useless
auth header from it.
Security review pointed out --name-only alone drops the config file
origin that made this diagnostic useful in the first place. --show-origin
combines safely with --name-only (origin path, still no value), so add it
back: reports whether a helper is configured AND which file configures
it, with nothing printable left.
@sakibsadmanshajib
sakibsadmanshajib merged commit 627c1ba into main Aug 14, 2026
18 checks passed
@github-actions
github-actions Bot deleted the fix/deploy-demo-box-auth branch August 14, 2026 16:17
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.

1 participant