diff --git a/.agents/skills/agent-conduct/SKILL.md b/.agents/skills/agent-conduct/SKILL.md index aa93444c0..d5118c587 100644 --- a/.agents/skills/agent-conduct/SKILL.md +++ b/.agents/skills/agent-conduct/SKILL.md @@ -77,6 +77,7 @@ Claims about a pull request being reviewed, clean, or mergeable are owned by the - **Reference every pull request as a clickable link.** When you mention a PR on a surface that renders Markdown (chat, a summary, a report), render it as a Markdown link to the PR (`[#N](https://github.com/OWNER/REPO/pull/N)`), never a bare `#N`. The same applies to issues and commits. **The form follows the surface.** Some surfaces link neither a Markdown link nor a bare URL, an interactive prompt's question and option text among them, and pasting a full URL into one of those does not rescue it, since the reader gets a string to copy, which is the outcome this rule exists to prevent. There the reference is a bare `#N`, and the clickable link goes in the message that comes **before** the prompt rather than merely alongside it, because the prompt blocks on an answer and a message emitted after it is read once that answer is already given, which is the one moment the link is no longer any use. The test is whether the reader can click it where it is read, not whether it was written in the syntax that works elsewhere. +- **Put a command the user must run in the question's own text.** Where a question asks the user to run something, the question carries the exact command, complete and ready to paste. One example is a command the agent cannot run itself because the command prompts for a password. A message before the prompt may repeat the command, but the question never points at that message. A question pointing at it can leave the reader to find the command among a long run of earlier output. In the clickable-link bullet above, only the rendered link moves out of the prompt. A command needs no rendering, since the prompt shows it as plain text the reader can copy. - **Ask every question through the interface's prompt, never in prose.** When you need the user to decide, answer, or approve anything, put it to them through the interface's own prompt mechanism, whether or not work is blocked on it, and an offer to do more work ("want me to file that?") is a question like any other. A question written into a message is lost in the report around it however short it is, and one closing a long report sits where the reader is least likely to reach it. Where no prompt mechanism is available, the questions go as a numbered list opening the message rather than closing it, so the user can reply per number. This bullet settles how a question is put once it is asked, and whether a question is asked now or recorded for later is the parking bullet's to settle, the one opening "A question filed as an issue is parked rather than asked". - **Raise work blocked on the user as a direct interactive prompt.** When progress needs a decision, an authorization, or an answer only the user can give, ask for it through the interface's own prompt mechanism, at the point the work stops. Never leave it as prose in a summary: a handoff buried in a paragraph is a handoff that did not happen, because a summary reads as a report of finished work and the one line still waiting on the user is the easiest in it to skim past. The blocked item is the message, not a closing remark on a message about something else. **The options offered are the actions themselves**, and the one that unblocks the work names the action it authorizes ("squash and merge it"), so selecting it is the go-ahead rather than a note to act on later. Offering only ways to wait is the same failure in interactive clothing, since a prompt whose every choice is inaction reports the block rather than clearing it, and where the agent may not perform the authorized action itself, the option says who does it. Where no interactive prompt is available, the numbered list the bullet above names is the fallback. - **Lead every choice with a recommendation and its reason.** Wherever the user is asked to choose, in a prompt or in a numbered list, the option the agent recommends comes first and is marked as the recommendation, and every option states the reason for it. A user who takes the recommendation then reads one line, and one who does not sees what the other options trade away. A question with no defensible recommendation says so rather than inventing one. Where an open question has no options and the agent does have an answer, that answer and its reason go in the question's text, never as an invented option. diff --git a/.agents/skills/session-handoff/SKILL.md b/.agents/skills/session-handoff/SKILL.md index 4264f53bf..530928be7 100644 --- a/.agents/skills/session-handoff/SKILL.md +++ b/.agents/skills/session-handoff/SKILL.md @@ -70,8 +70,9 @@ without trusting the parts of it that have gone stale. session. A defect in WSL interop, the `WSLInterop` binfmt handler, `/mnt/` paths, or `wslpath` reproduces only inside WSL. So does running a Windows `.exe` from Linux, although WSL runs on Windows. Behavior that differs under Windows PowerShell 5.1 or `winget`, or in Windows - path and ACL semantics, reproduces only on Windows. Each of these defects still goes to the backlog where a Linux - host outside WSL verifies its fix, as a path-translation unit test can. An issue in + path and ACL semantics, reproduces only on Windows. Each of these defects still goes to the + backlog when a Linux host outside WSL can both fix it and verify the fix. A path-translation + fix made and unit-tested on a Linux host outside WSL is one such case. An issue in systemd user scopes, apt, a bash script, or a `pwsh` script run in a Linux container gets no lane by its topic. The conditions decide it like any other issue. Where only part of an issue needs a platform, the session finding it files each such part as its own issue for its platform's lane. @@ -273,6 +274,7 @@ falls to the next session that has one. The rules below settle which case applie - **Reference every pull request as a clickable link.** When you mention a PR on a surface that renders Markdown (chat, a summary, a report), render it as a Markdown link to the PR (`[#N](https://github.com/OWNER/REPO/pull/N)`), never a bare `#N`. The same applies to issues and commits. **The form follows the surface.** Some surfaces link neither a Markdown link nor a bare URL, an interactive prompt's question and option text among them, and pasting a full URL into one of those does not rescue it, since the reader gets a string to copy, which is the outcome this rule exists to prevent. There the reference is a bare `#N`, and the clickable link goes in the message that comes **before** the prompt rather than merely alongside it, because the prompt blocks on an answer and a message emitted after it is read once that answer is already given, which is the one moment the link is no longer any use. The test is whether the reader can click it where it is read, not whether it was written in the syntax that works elsewhere. +- **Put a command the user must run in the question's own text.** Where a question asks the user to run something, the question carries the exact command, complete and ready to paste. One example is a command the agent cannot run itself because the command prompts for a password. A message before the prompt may repeat the command, but the question never points at that message. A question pointing at it can leave the reader to find the command among a long run of earlier output. In the clickable-link bullet above, only the rendered link moves out of the prompt. A command needs no rendering, since the prompt shows it as plain text the reader can copy. - **Ask every question through the interface's prompt, never in prose.** When you need the user to decide, answer, or approve anything, put it to them through the interface's own prompt mechanism, whether or not work is blocked on it, and an offer to do more work ("want me to file that?") is a question like any other. A question written into a message is lost in the report around it however short it is, and one closing a long report sits where the reader is least likely to reach it. Where no prompt mechanism is available, the questions go as a numbered list opening the message rather than closing it, so the user can reply per number. This bullet settles how a question is put once it is asked, and whether a question is asked now or recorded for later is the parking bullet's to settle, the one opening "A question filed as an issue is parked rather than asked". - **Raise work blocked on the user as a direct interactive prompt.** When progress needs a decision, an authorization, or an answer only the user can give, ask for it through the interface's own prompt mechanism, at the point the work stops. Never leave it as prose in a summary: a handoff buried in a paragraph is a handoff that did not happen, because a summary reads as a report of finished work and the one line still waiting on the user is the easiest in it to skim past. The blocked item is the message, not a closing remark on a message about something else. **The options offered are the actions themselves**, and the one that unblocks the work names the action it authorizes ("squash and merge it"), so selecting it is the go-ahead rather than a note to act on later. Offering only ways to wait is the same failure in interactive clothing, since a prompt whose every choice is inaction reports the block rather than clearing it, and where the agent may not perform the authorized action itself, the option says who does it. Where no interactive prompt is available, the numbered list the bullet above names is the fallback. - **Lead every choice with a recommendation and its reason.** Wherever the user is asked to choose, in a prompt or in a numbered list, the option the agent recommends comes first and is marked as the recommendation, and every option states the reason for it. A user who takes the recommendation then reads one line, and one who does not sees what the other options trade away. A question with no defensible recommendation says so rather than inventing one. Where an open question has no options and the agent does have an answer, that answer and its reason go in the question's text, never as an invented option. diff --git a/.claude-plugin/fleet-skills/.source-digests/agent-conduct b/.claude-plugin/fleet-skills/.source-digests/agent-conduct index 5e7d9b244..cded7e77e 100644 --- a/.claude-plugin/fleet-skills/.source-digests/agent-conduct +++ b/.claude-plugin/fleet-skills/.source-digests/agent-conduct @@ -1 +1 @@ -104c9334429bf721 +af896a6f3797483f diff --git a/.claude-plugin/fleet-skills/.source-digests/session-handoff b/.claude-plugin/fleet-skills/.source-digests/session-handoff index 341ebb874..4d33b2a9a 100644 --- a/.claude-plugin/fleet-skills/.source-digests/session-handoff +++ b/.claude-plugin/fleet-skills/.source-digests/session-handoff @@ -1 +1 @@ -1d8c4702f7b8a31c +9438d88bc6983156 diff --git a/.claude-plugin/fleet-skills/skills/agent-conduct/SKILL.md b/.claude-plugin/fleet-skills/skills/agent-conduct/SKILL.md index aa93444c0..d5118c587 100644 --- a/.claude-plugin/fleet-skills/skills/agent-conduct/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/agent-conduct/SKILL.md @@ -77,6 +77,7 @@ Claims about a pull request being reviewed, clean, or mergeable are owned by the - **Reference every pull request as a clickable link.** When you mention a PR on a surface that renders Markdown (chat, a summary, a report), render it as a Markdown link to the PR (`[#N](https://github.com/OWNER/REPO/pull/N)`), never a bare `#N`. The same applies to issues and commits. **The form follows the surface.** Some surfaces link neither a Markdown link nor a bare URL, an interactive prompt's question and option text among them, and pasting a full URL into one of those does not rescue it, since the reader gets a string to copy, which is the outcome this rule exists to prevent. There the reference is a bare `#N`, and the clickable link goes in the message that comes **before** the prompt rather than merely alongside it, because the prompt blocks on an answer and a message emitted after it is read once that answer is already given, which is the one moment the link is no longer any use. The test is whether the reader can click it where it is read, not whether it was written in the syntax that works elsewhere. +- **Put a command the user must run in the question's own text.** Where a question asks the user to run something, the question carries the exact command, complete and ready to paste. One example is a command the agent cannot run itself because the command prompts for a password. A message before the prompt may repeat the command, but the question never points at that message. A question pointing at it can leave the reader to find the command among a long run of earlier output. In the clickable-link bullet above, only the rendered link moves out of the prompt. A command needs no rendering, since the prompt shows it as plain text the reader can copy. - **Ask every question through the interface's prompt, never in prose.** When you need the user to decide, answer, or approve anything, put it to them through the interface's own prompt mechanism, whether or not work is blocked on it, and an offer to do more work ("want me to file that?") is a question like any other. A question written into a message is lost in the report around it however short it is, and one closing a long report sits where the reader is least likely to reach it. Where no prompt mechanism is available, the questions go as a numbered list opening the message rather than closing it, so the user can reply per number. This bullet settles how a question is put once it is asked, and whether a question is asked now or recorded for later is the parking bullet's to settle, the one opening "A question filed as an issue is parked rather than asked". - **Raise work blocked on the user as a direct interactive prompt.** When progress needs a decision, an authorization, or an answer only the user can give, ask for it through the interface's own prompt mechanism, at the point the work stops. Never leave it as prose in a summary: a handoff buried in a paragraph is a handoff that did not happen, because a summary reads as a report of finished work and the one line still waiting on the user is the easiest in it to skim past. The blocked item is the message, not a closing remark on a message about something else. **The options offered are the actions themselves**, and the one that unblocks the work names the action it authorizes ("squash and merge it"), so selecting it is the go-ahead rather than a note to act on later. Offering only ways to wait is the same failure in interactive clothing, since a prompt whose every choice is inaction reports the block rather than clearing it, and where the agent may not perform the authorized action itself, the option says who does it. Where no interactive prompt is available, the numbered list the bullet above names is the fallback. - **Lead every choice with a recommendation and its reason.** Wherever the user is asked to choose, in a prompt or in a numbered list, the option the agent recommends comes first and is marked as the recommendation, and every option states the reason for it. A user who takes the recommendation then reads one line, and one who does not sees what the other options trade away. A question with no defensible recommendation says so rather than inventing one. Where an open question has no options and the agent does have an answer, that answer and its reason go in the question's text, never as an invented option. diff --git a/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md b/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md index 4264f53bf..530928be7 100644 --- a/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md @@ -70,8 +70,9 @@ without trusting the parts of it that have gone stale. session. A defect in WSL interop, the `WSLInterop` binfmt handler, `/mnt/` paths, or `wslpath` reproduces only inside WSL. So does running a Windows `.exe` from Linux, although WSL runs on Windows. Behavior that differs under Windows PowerShell 5.1 or `winget`, or in Windows - path and ACL semantics, reproduces only on Windows. Each of these defects still goes to the backlog where a Linux - host outside WSL verifies its fix, as a path-translation unit test can. An issue in + path and ACL semantics, reproduces only on Windows. Each of these defects still goes to the + backlog when a Linux host outside WSL can both fix it and verify the fix. A path-translation + fix made and unit-tested on a Linux host outside WSL is one such case. An issue in systemd user scopes, apt, a bash script, or a `pwsh` script run in a Linux container gets no lane by its topic. The conditions decide it like any other issue. Where only part of an issue needs a platform, the session finding it files each such part as its own issue for its platform's lane. @@ -273,6 +274,7 @@ falls to the next session that has one. The rules below settle which case applie - **Reference every pull request as a clickable link.** When you mention a PR on a surface that renders Markdown (chat, a summary, a report), render it as a Markdown link to the PR (`[#N](https://github.com/OWNER/REPO/pull/N)`), never a bare `#N`. The same applies to issues and commits. **The form follows the surface.** Some surfaces link neither a Markdown link nor a bare URL, an interactive prompt's question and option text among them, and pasting a full URL into one of those does not rescue it, since the reader gets a string to copy, which is the outcome this rule exists to prevent. There the reference is a bare `#N`, and the clickable link goes in the message that comes **before** the prompt rather than merely alongside it, because the prompt blocks on an answer and a message emitted after it is read once that answer is already given, which is the one moment the link is no longer any use. The test is whether the reader can click it where it is read, not whether it was written in the syntax that works elsewhere. +- **Put a command the user must run in the question's own text.** Where a question asks the user to run something, the question carries the exact command, complete and ready to paste. One example is a command the agent cannot run itself because the command prompts for a password. A message before the prompt may repeat the command, but the question never points at that message. A question pointing at it can leave the reader to find the command among a long run of earlier output. In the clickable-link bullet above, only the rendered link moves out of the prompt. A command needs no rendering, since the prompt shows it as plain text the reader can copy. - **Ask every question through the interface's prompt, never in prose.** When you need the user to decide, answer, or approve anything, put it to them through the interface's own prompt mechanism, whether or not work is blocked on it, and an offer to do more work ("want me to file that?") is a question like any other. A question written into a message is lost in the report around it however short it is, and one closing a long report sits where the reader is least likely to reach it. Where no prompt mechanism is available, the questions go as a numbered list opening the message rather than closing it, so the user can reply per number. This bullet settles how a question is put once it is asked, and whether a question is asked now or recorded for later is the parking bullet's to settle, the one opening "A question filed as an issue is parked rather than asked". - **Raise work blocked on the user as a direct interactive prompt.** When progress needs a decision, an authorization, or an answer only the user can give, ask for it through the interface's own prompt mechanism, at the point the work stops. Never leave it as prose in a summary: a handoff buried in a paragraph is a handoff that did not happen, because a summary reads as a report of finished work and the one line still waiting on the user is the easiest in it to skim past. The blocked item is the message, not a closing remark on a message about something else. **The options offered are the actions themselves**, and the one that unblocks the work names the action it authorizes ("squash and merge it"), so selecting it is the go-ahead rather than a note to act on later. Offering only ways to wait is the same failure in interactive clothing, since a prompt whose every choice is inaction reports the block rather than clearing it, and where the agent may not perform the authorized action itself, the option says who does it. Where no interactive prompt is available, the numbered list the bullet above names is the fallback. - **Lead every choice with a recommendation and its reason.** Wherever the user is asked to choose, in a prompt or in a numbered list, the option the agent recommends comes first and is marked as the recommendation, and every option states the reason for it. A user who takes the recommendation then reads one line, and one who does not sees what the other options trade away. A question with no defensible recommendation says so rather than inventing one. Where an open question has no options and the agent does have an answer, that answer and its reason go in the question's text, never as an invented option. diff --git a/.github/skills/agent-conduct/SKILL.md b/.github/skills/agent-conduct/SKILL.md index aa93444c0..d5118c587 100644 --- a/.github/skills/agent-conduct/SKILL.md +++ b/.github/skills/agent-conduct/SKILL.md @@ -77,6 +77,7 @@ Claims about a pull request being reviewed, clean, or mergeable are owned by the - **Reference every pull request as a clickable link.** When you mention a PR on a surface that renders Markdown (chat, a summary, a report), render it as a Markdown link to the PR (`[#N](https://github.com/OWNER/REPO/pull/N)`), never a bare `#N`. The same applies to issues and commits. **The form follows the surface.** Some surfaces link neither a Markdown link nor a bare URL, an interactive prompt's question and option text among them, and pasting a full URL into one of those does not rescue it, since the reader gets a string to copy, which is the outcome this rule exists to prevent. There the reference is a bare `#N`, and the clickable link goes in the message that comes **before** the prompt rather than merely alongside it, because the prompt blocks on an answer and a message emitted after it is read once that answer is already given, which is the one moment the link is no longer any use. The test is whether the reader can click it where it is read, not whether it was written in the syntax that works elsewhere. +- **Put a command the user must run in the question's own text.** Where a question asks the user to run something, the question carries the exact command, complete and ready to paste. One example is a command the agent cannot run itself because the command prompts for a password. A message before the prompt may repeat the command, but the question never points at that message. A question pointing at it can leave the reader to find the command among a long run of earlier output. In the clickable-link bullet above, only the rendered link moves out of the prompt. A command needs no rendering, since the prompt shows it as plain text the reader can copy. - **Ask every question through the interface's prompt, never in prose.** When you need the user to decide, answer, or approve anything, put it to them through the interface's own prompt mechanism, whether or not work is blocked on it, and an offer to do more work ("want me to file that?") is a question like any other. A question written into a message is lost in the report around it however short it is, and one closing a long report sits where the reader is least likely to reach it. Where no prompt mechanism is available, the questions go as a numbered list opening the message rather than closing it, so the user can reply per number. This bullet settles how a question is put once it is asked, and whether a question is asked now or recorded for later is the parking bullet's to settle, the one opening "A question filed as an issue is parked rather than asked". - **Raise work blocked on the user as a direct interactive prompt.** When progress needs a decision, an authorization, or an answer only the user can give, ask for it through the interface's own prompt mechanism, at the point the work stops. Never leave it as prose in a summary: a handoff buried in a paragraph is a handoff that did not happen, because a summary reads as a report of finished work and the one line still waiting on the user is the easiest in it to skim past. The blocked item is the message, not a closing remark on a message about something else. **The options offered are the actions themselves**, and the one that unblocks the work names the action it authorizes ("squash and merge it"), so selecting it is the go-ahead rather than a note to act on later. Offering only ways to wait is the same failure in interactive clothing, since a prompt whose every choice is inaction reports the block rather than clearing it, and where the agent may not perform the authorized action itself, the option says who does it. Where no interactive prompt is available, the numbered list the bullet above names is the fallback. - **Lead every choice with a recommendation and its reason.** Wherever the user is asked to choose, in a prompt or in a numbered list, the option the agent recommends comes first and is marked as the recommendation, and every option states the reason for it. A user who takes the recommendation then reads one line, and one who does not sees what the other options trade away. A question with no defensible recommendation says so rather than inventing one. Where an open question has no options and the agent does have an answer, that answer and its reason go in the question's text, never as an invented option. diff --git a/.github/skills/session-handoff/SKILL.md b/.github/skills/session-handoff/SKILL.md index 4264f53bf..530928be7 100644 --- a/.github/skills/session-handoff/SKILL.md +++ b/.github/skills/session-handoff/SKILL.md @@ -70,8 +70,9 @@ without trusting the parts of it that have gone stale. session. A defect in WSL interop, the `WSLInterop` binfmt handler, `/mnt/` paths, or `wslpath` reproduces only inside WSL. So does running a Windows `.exe` from Linux, although WSL runs on Windows. Behavior that differs under Windows PowerShell 5.1 or `winget`, or in Windows - path and ACL semantics, reproduces only on Windows. Each of these defects still goes to the backlog where a Linux - host outside WSL verifies its fix, as a path-translation unit test can. An issue in + path and ACL semantics, reproduces only on Windows. Each of these defects still goes to the + backlog when a Linux host outside WSL can both fix it and verify the fix. A path-translation + fix made and unit-tested on a Linux host outside WSL is one such case. An issue in systemd user scopes, apt, a bash script, or a `pwsh` script run in a Linux container gets no lane by its topic. The conditions decide it like any other issue. Where only part of an issue needs a platform, the session finding it files each such part as its own issue for its platform's lane. @@ -273,6 +274,7 @@ falls to the next session that has one. The rules below settle which case applie - **Reference every pull request as a clickable link.** When you mention a PR on a surface that renders Markdown (chat, a summary, a report), render it as a Markdown link to the PR (`[#N](https://github.com/OWNER/REPO/pull/N)`), never a bare `#N`. The same applies to issues and commits. **The form follows the surface.** Some surfaces link neither a Markdown link nor a bare URL, an interactive prompt's question and option text among them, and pasting a full URL into one of those does not rescue it, since the reader gets a string to copy, which is the outcome this rule exists to prevent. There the reference is a bare `#N`, and the clickable link goes in the message that comes **before** the prompt rather than merely alongside it, because the prompt blocks on an answer and a message emitted after it is read once that answer is already given, which is the one moment the link is no longer any use. The test is whether the reader can click it where it is read, not whether it was written in the syntax that works elsewhere. +- **Put a command the user must run in the question's own text.** Where a question asks the user to run something, the question carries the exact command, complete and ready to paste. One example is a command the agent cannot run itself because the command prompts for a password. A message before the prompt may repeat the command, but the question never points at that message. A question pointing at it can leave the reader to find the command among a long run of earlier output. In the clickable-link bullet above, only the rendered link moves out of the prompt. A command needs no rendering, since the prompt shows it as plain text the reader can copy. - **Ask every question through the interface's prompt, never in prose.** When you need the user to decide, answer, or approve anything, put it to them through the interface's own prompt mechanism, whether or not work is blocked on it, and an offer to do more work ("want me to file that?") is a question like any other. A question written into a message is lost in the report around it however short it is, and one closing a long report sits where the reader is least likely to reach it. Where no prompt mechanism is available, the questions go as a numbered list opening the message rather than closing it, so the user can reply per number. This bullet settles how a question is put once it is asked, and whether a question is asked now or recorded for later is the parking bullet's to settle, the one opening "A question filed as an issue is parked rather than asked". - **Raise work blocked on the user as a direct interactive prompt.** When progress needs a decision, an authorization, or an answer only the user can give, ask for it through the interface's own prompt mechanism, at the point the work stops. Never leave it as prose in a summary: a handoff buried in a paragraph is a handoff that did not happen, because a summary reads as a report of finished work and the one line still waiting on the user is the easiest in it to skim past. The blocked item is the message, not a closing remark on a message about something else. **The options offered are the actions themselves**, and the one that unblocks the work names the action it authorizes ("squash and merge it"), so selecting it is the go-ahead rather than a note to act on later. Offering only ways to wait is the same failure in interactive clothing, since a prompt whose every choice is inaction reports the block rather than clearing it, and where the agent may not perform the authorized action itself, the option says who does it. Where no interactive prompt is available, the numbered list the bullet above names is the fallback. - **Lead every choice with a recommendation and its reason.** Wherever the user is asked to choose, in a prompt or in a numbered list, the option the agent recommends comes first and is marked as the recommendation, and every option states the reason for it. A user who takes the recommendation then reads one line, and one who does not sees what the other options trade away. A question with no defensible recommendation says so rather than inventing one. Where an open question has no options and the agent does have an answer, that answer and its reason go in the question's text, never as an invented option. diff --git a/GOVERNANCE.md b/GOVERNANCE.md index df57d5fa0..249ac6f95 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -251,6 +251,7 @@ The provider-specific mechanics this contract needs to actually drive GitHub Cop ## Communicating with the User - **Reference every pull request as a clickable link.** When you mention a PR on a surface that renders Markdown (chat, a summary, a report), render it as a Markdown link to the PR (`[#N](https://github.com/OWNER/REPO/pull/N)`), never a bare `#N`. The same applies to issues and commits. **The form follows the surface.** Some surfaces link neither a Markdown link nor a bare URL, an interactive prompt's question and option text among them, and pasting a full URL into one of those does not rescue it, since the reader gets a string to copy, which is the outcome this rule exists to prevent. There the reference is a bare `#N`, and the clickable link goes in the message that comes **before** the prompt rather than merely alongside it, because the prompt blocks on an answer and a message emitted after it is read once that answer is already given, which is the one moment the link is no longer any use. The test is whether the reader can click it where it is read, not whether it was written in the syntax that works elsewhere. +- **Put a command the user must run in the question's own text.** Where a question asks the user to run something, the question carries the exact command, complete and ready to paste. One example is a command the agent cannot run itself because the command prompts for a password. A message before the prompt may repeat the command, but the question never points at that message. A question pointing at it can leave the reader to find the command among a long run of earlier output. In the clickable-link bullet above, only the rendered link moves out of the prompt. A command needs no rendering, since the prompt shows it as plain text the reader can copy. - **Ask every question through the interface's prompt, never in prose.** When you need the user to decide, answer, or approve anything, put it to them through the interface's own prompt mechanism, whether or not work is blocked on it, and an offer to do more work ("want me to file that?") is a question like any other. A question written into a message is lost in the report around it however short it is, and one closing a long report sits where the reader is least likely to reach it. Where no prompt mechanism is available, the questions go as a numbered list opening the message rather than closing it, so the user can reply per number. This bullet settles how a question is put once it is asked, and whether a question is asked now or recorded for later is the parking bullet's to settle, the one opening "A question filed as an issue is parked rather than asked". - **Raise work blocked on the user as a direct interactive prompt.** When progress needs a decision, an authorization, or an answer only the user can give, ask for it through the interface's own prompt mechanism, at the point the work stops. Never leave it as prose in a summary: a handoff buried in a paragraph is a handoff that did not happen, because a summary reads as a report of finished work and the one line still waiting on the user is the easiest in it to skim past. The blocked item is the message, not a closing remark on a message about something else. **The options offered are the actions themselves**, and the one that unblocks the work names the action it authorizes ("squash and merge it"), so selecting it is the go-ahead rather than a note to act on later. Offering only ways to wait is the same failure in interactive clothing, since a prompt whose every choice is inaction reports the block rather than clearing it, and where the agent may not perform the authorized action itself, the option says who does it. Where no interactive prompt is available, the numbered list the bullet above names is the fallback. - **Lead every choice with a recommendation and its reason.** Wherever the user is asked to choose, in a prompt or in a numbered list, the option the agent recommends comes first and is marked as the recommendation, and every option states the reason for it. A user who takes the recommendation then reads one line, and one who does not sees what the other options trade away. A question with no defensible recommendation says so rather than inventing one. Where an open question has no options and the agent does have an answer, that answer and its reason go in the question's text, never as an invented option. diff --git a/docs/reusable-workflows.md b/docs/reusable-workflows.md index 6ba3fdf4e..63dab08be 100644 --- a/docs/reusable-workflows.md +++ b/docs/reusable-workflows.md @@ -507,7 +507,7 @@ The `get-version` example passes no `ref`, so the task versions the triggering c ## Adopting the Release Chain -A downstream repo replaces its carried release orchestrator and per-target leaf tasks with a caller stub in its own `publish-release.yml` reaching the hub tasks by pin. `test-pull-request.yml`'s smoke job calls `build-release-task.yml` the same way, with `smoke: true` and the paths-filter's `enable_*` outputs. A dispatch bypass on a caller that maps filter outputs to `enable_*` inputs must also force them on. A dispatch run has no pull request diff, so the outputs can leave every target disabled. The full shape below is the catalog snippet [`publish-release.yml`][release-caller-snippet] byte for byte, pinned to a release carrying the current task interface, and Dependabot bumps it from there ([Pinning][pinning]). The task declares no job-level `permissions:` of its own, because a called job's block is validated against the caller's grant before its `if:` runs and would fail a caller that does not grant it at startup. The caller therefore grants only what its enabled paths write with: `contents: write` and `actions: write` when it sets `github: true` on a non-smoke run (the release upload, the artifact cleanup, and the [`WORKFLOW.md`][workflow] D4.7 publisher dispatch and self-cancel), and nothing beyond `contents: read` on a build-only or smoke run, where a Dependabot pull request holds a read-only token. No call to this task ever needs `id-token: write`, because neither package push happens inside it, for the reason the next paragraph gives. +A downstream repo replaces its carried release orchestrator and per-target leaf tasks with a caller stub in its own `publish-release.yml` reaching the hub tasks by pin. `test-pull-request.yml`'s smoke job calls `build-release-task.yml` the same way, with `smoke: true` and its own fixed `enable_*` set, the same one its `publish-release.yml` passes. The `changes` paths filter in `test-pull-request.yml` decides only whether the smoke job runs. A multi-target caller that wires filter outputs into `enable_*` inputs and adds a dispatch bypass must also force them on. A dispatch run has no pull request diff, so the outputs can leave every target disabled. The full shape below is the catalog snippet [`publish-release.yml`][release-caller-snippet] byte for byte, pinned to a release carrying the current task interface, and Dependabot bumps it from there ([Pinning][pinning]). The task declares no job-level `permissions:` of its own, because a called job's block is validated against the caller's grant before its `if:` runs and would fail a caller that does not grant it at startup. The caller therefore grants only what its enabled paths write with: `contents: write` and `actions: write` when it sets `github: true` on a non-smoke run (the release upload, the artifact cleanup, and the [`WORKFLOW.md`][workflow] D4.7 publisher dispatch and self-cancel), and nothing beyond `contents: read` on a build-only or smoke run, where a Dependabot pull request holds a read-only token. No call to this task ever needs `id-token: write`, because neither package push happens inside it, for the reason the next paragraph gives. Neither package push runs inside the hub task, and that is a constraint rather than a preference. NuGet.org and PyPI trusted publishing both validate the OIDC token's `job_workflow_ref` claim against the repository that owns the package, and that claim names the workflow file the job actually ran from. A job running from a hub task therefore carries `ptr727/ProjectTemplate/.github/workflows/build-release-task.yml@`, and the token exchange is rejected, NuGet.org answering `HTTP 401` with `does not start with //.github/workflows/` and PyPI rejecting the same shape under its own code. A caller hook does not avoid it, since a composite action runs inside the hub's job and leaves the claim unchanged. The hub task instead builds the package and uploads it as `nuget-build-` or `pypi-build-`, and the caller stub's own `publish-nuget` or `publish-pypi` job downloads that artifact and pushes, so the claim names the publishing repository. A smoke build never reaches either push, which is why a pull request cannot catch this and the first real release is where it surfaces. diff --git a/host-setup/agent-safety/README.md b/host-setup/agent-safety/README.md index b02ab6164..50d58b7d3 100644 --- a/host-setup/agent-safety/README.md +++ b/host-setup/agent-safety/README.md @@ -49,10 +49,12 @@ text says, because the harm it covers was never in the text. allowed only when named in a grant read from the environment the session was launched with -- never a channel the agent itself can set (an inline `VAR=x cmd` prefix or an `export` inside the same call must not satisfy this). **When no `origin` resolves at all** (a non-git directory, or a - checkout whose remote can't be read), this requirement has nothing to compare the target against - and does not fire -- requirements 1 and 2 still apply regardless, and this is the same - precision-over-recall stance every requirement but 4 takes. A write is recognized by its command - and subcommand as gh's own parser finds them, past the flags before each. So a repo flag before + checkout whose remote can't be read), no owner exists to compare against. This requirement's + owner comparison and its run-time-word check then do not fire, while requirements 1 and 2 still + apply. This is the same precision-over-recall stance every requirement but 4 takes. A write whose + target the hook does not read, described below, fails closed with no origin too, since that + target is unknown either way. A write is recognized by its command and subcommand as gh's own + parser finds them, past the flags before each. So a repo flag before the command or between `pr` and its subcommand still names the target. The target is also the `R` in a short-option cluster such as `-dR`, and a `GH_REPO` value that reaches the gh word, the one in the environment the session was launched with included. A repository named by a diff --git a/host-setup/agent-safety/claude/README.md b/host-setup/agent-safety/claude/README.md index d11c12033..c2cf0b0f7 100644 --- a/host-setup/agent-safety/claude/README.md +++ b/host-setup/agent-safety/claude/README.md @@ -155,7 +155,7 @@ Every other key in the file is left as it stands, `permissions.allow` and the re - **Per-machine.** `~/.claude/` does not travel, so run the installer on each box. This is the rollout that [#365][issue-365] tracks. - **Precision over recall for the write footguns.** The hook denies the specific dangerous write shapes with high confidence rather than gating every write, so it is written to avoid blocking legitimate work -- a known false-positive shape is a parser defect to fix, not an accepted cost. The exception is a write whose target the hook cannot compare, which it denies. That is a write whose repository owner is a run-time word. It is also one whose target the hook does not read, as the root spec's requirement 3 lists. A shape it does not catch still falls under the behavioral rules. - **The branch-bypass rule fails closed.** Unlike the write-footgun rules, a push to `main`/`master`/`develop` is denied even when its rules cannot be determined (the API is unreachable, or the checkout's origin cannot be resolved to query them), because the harm there is a silent success under the maintainer's admin bypass. The rule reads each branch's live rules, so it adapts to every repo (a code-style `develop` denies, a config-style `develop` allows) with no per-repo configuration, and hands the exact command to the maintainer to run when a bypass is genuinely intended. -- **Opaque targets are unseen.** The hook cannot see the repository behind a GraphQL node id, which is exactly why rule 2 blocks a *literal* id at all, since a captured `$variable` is trusted. Likewise, the cross-origin check only runs when an `origin` can be resolved and the write names an explicit target, such as `-R`, `repos//`, a `GH_REPO` value, or a pull request URL. A write from a non-git directory, or one whose target is only a node id, is evaluated by rules 1 and 2 alone. The hook reads an inherited `GH_REPO` from its own environment, the one the session was launched with. A value a Bash tool call gets some other way, such as from a shell profile it sources, is not seen. An inherited value is read beside any `export` in the command, since the hook cannot tell whether that export runs. To write elsewhere, override it with a prefix, as in `GH_REPO=/ gh ...`, or with `-R`. Where a command-text `export` may not run where it sits, as in a subshell or a branch, every value the command sets is read. A `gh api` endpoint's run-time segment past a literal owner is compared by that owner alone. Such a segment may still hold a `..` segment that reaches another owner. +- **Opaque targets are unseen.** The hook cannot see the repository behind a GraphQL node id. That is why rule 2 blocks a *literal* id at all, since a captured `$variable` is trusted. Likewise, the cross-origin check only runs when an `origin` can be resolved and the write names an explicit target. Such a target is `-R`, `repos//`, a `GH_REPO` value, or a pull request URL. No owner comparison reads a write whose target is only a node id. A write from a non-git directory skips only the owner comparison and the run-time-word check. A target the hook does not read fails closed outside a checkout too. Examples are a gh write an `eval` runs and a `gh api` write to a `repositories/` endpoint or through a `..` segment. The hook reads an inherited `GH_REPO` from its own environment, the one the session was launched with. A value a Bash tool call gets some other way, such as from a shell profile it sources, is not seen. An inherited value is read beside any `export` in the command, since the hook cannot tell whether that export runs. To write elsewhere, override it with a prefix, as in `GH_REPO=/ gh ...`, or with `-R`. Where a command-text `export` may not run where it sits, as in a subshell or a branch, every value the command sets is read. A `gh api` endpoint's run-time segment past a literal owner is compared by that owner alone. Such a segment may still hold a `..` segment that reaches another owner. - **A write or a wait inside a script file is unseen, so never put either in a script to run it.** The hook reads the command the agent runs, which for `bash deploy.sh` is that one string. A `git push` or a `gh` mutation inside the script therefore reaches the server unparsed. An unbounded wait inside that same script is equally unseen, and worse in one way. A denied write fails loudly. A wait the guard never saw runs until something ends it, which is one of the leaks the SessionEnd sweep exists to find afterwards. This is the widest hole in the kit, and an agent opens it by accident. Collecting fifteen repetitive pushes into a loop looks like tidiness, and so does moving a poll into a script to keep a tool call short. Neither looks like disabling a guard. Issue each write and each wait as its own command. A script that only reads, computes, or prepares local commits is fine, because the boundary is the write and the wait rather than the script. - **Not a credential control.** A fine-grained PAT limited to owned repositories is a separate, stronger structural guard (a hard `403` on any non-owned repo) and is left to per-machine credential setup, out of this kit. - **Rule 6's ref-checkout and `--ff-only` exemptions are deliberate, not a gap.** The #1073 incident's own literal commands (an unforced `git checkout main`, then `git pull --ff-only`) are exactly this shape, so this rule does not deny the incident's own commands specifically. Denying them would also break the documented base-clone cleanup step in the `repo-worktree` skill, which needs precisely those two. The concurrent-access hazard those commands still carry (switching HEAD, or fast-forwarding, in a checkout another task might be relying on) is not decidable from the command text alone, so it stays the prose rule's job. A non-force flag alongside the ref, such as `--detach`, is exempt too, not only a strictly zero-flags invocation -- git's own overwrite-refusal on a dirty tracked file is unaffected by it, verified live. diff --git a/host-setup/agent-safety/claude/gh-write-guard.py b/host-setup/agent-safety/claude/gh-write-guard.py index 415a3ce2f..769a7d27d 100755 --- a/host-setup/agent-safety/claude/gh-write-guard.py +++ b/host-setup/agent-safety/claude/gh-write-guard.py @@ -7406,7 +7406,8 @@ def classify( # `_gh_write_targets` reads argv position within each real `gh` invocation, so a compound command carrying one target per invocation still has every one read (the write after `&&` is not skipped). # A --repo/repos// mention living inside an unrelated --body/--title/-f value, or in a non-gh command entirely, is not read as a target. targets = _gh_write_targets(cmd, environ) - # This only runs when origin resolves, meaning a git checkout, since with no project context there is nothing to compare an explicit target against, so the check is skipped and rules 1 and 2 still apply. + # The owner comparison and the run-time-word check run only when origin resolves, since with no origin there is no owner to compare against. + # The unread-target check after this block runs either way, since a target the guard does not read is unknown with or without an origin. # A node-id target is invisible here regardless, which is what rule 2 guards. if origin: granted = _granted_targets(environ) @@ -7431,12 +7432,12 @@ def classify( "it writes to. Failing closed. Write the repository out, as in --repo owner/repo, " "GH_REPO=owner/repo, or repos/owner/repo." + _handoff(cmd) ) - unread = _gh_unread_target(cmd, payloads) - if unread: - return "deny", ( - f"This gh write {unread[0]}, so the guard cannot tell which repository it writes " - f"to. Failing closed. {unread[1]}" + _handoff(cmd) - ) + unread = _gh_unread_target(cmd, payloads) + if unread: + return "deny", ( + f"This gh write {unread[0]}, so the guard cannot tell which repository it writes " + f"to. Failing closed. {unread[1]}" + _handoff(cmd) + ) # 5. Hand-rolled reply/resolve for a review thread, bypassing scripts/pr_review.py's one-call helper. dec, reason = _check_reply_resolve_helper(cmd, environ) @@ -8424,6 +8425,44 @@ def classify( ] +_NO_ORIGIN_CASES = [ + ("eval 'gh pr comment 5 -b x'", "deny", "no origin: a gh write an eval runs"), + ( + "gh api -X POST repositories/12345/issues -f title=t", + "deny", + "no origin: an api write naming its repository by id", + ), + ( + "gh api -X POST repos/example/repo/../../stranger/example/issues -f title=t", + "deny", + "no origin: a dot-segment api write", + ), + ( + "declare -x GH_REPO=stranger/example; gh pr comment 5 -b x", + "deny", + "no origin: a write taking a GH_REPO a declare -x sets", + ), + ( + "gh api -X POST repos/stranger/repo/issues", + "allow", + "no origin: a literal api target under another owner", + ), + ("gh pr comment 5 -b x", "allow", "no origin: a write naming no target"), + ( + "gh pr comment 5 --repo stranger/example -b x", + "allow", + "no origin: a literal repo flag under another owner", + ), + ( + 'gh pr comment 5 --repo "$R" -b x', + "allow", + "no origin: a run-time repo flag, which only the origin branch reads", + ), + ("gh api repositories/12345", "allow", "no origin: an api read by repository id"), + ("eval 'gh pr view 5'", "allow", "no origin: a gh read an eval runs"), +] + + # Rule-5 cases, covering the hand-rolled reply/resolve denial and its cross-owner grant escape. # Each carries the environment the grant is read from, matching the _SCOPE_CASES convention below. _REPLY_RESOLVE_CASES = [ @@ -15087,6 +15126,33 @@ def _push_repo_selftest(): return ok +def _no_origin_selftest(): + """Run `_NO_ORIGIN_CASES` with the origin read pinned to resolve nothing. + + That is the read outside any checkout. An unread target fails closed there, while a literal + or run-time target is allowed, since there is no owner to compare it against. The pin + replaces `_origin_owner_repo` rather than passing a directory, so no case depends on where + the self-test runs or on a git spawn. + """ + saved = globals()["_origin_owner_repo"] + globals()["_origin_owner_repo"] = lambda _cwd: None + ok = True + try: + for cmd, want, label in _NO_ORIGIN_CASES: + got, _ = classify( + cmd, + current_branch="feature/x", + rules_lookup=lambda br: set(), + environ={}, + primary_checkout_lookup=lambda d: False, + ) + ok = ok and got == want + print(f" {'ok ' if got == want else 'FAIL'} [{got:5}] want={want:5} {label}") + finally: + globals()["_origin_owner_repo"] = saved + return ok + + def _selftest(): # A deterministic offline run, pinning origin to ptr727/PlexCleaner, the incident repo, so the cross-origin case resolves without touching a real checkout. # The gh-write cases inject empty rules and a feature current-branch so no case reaches the live branch-rules query. @@ -15613,6 +15679,8 @@ def counting_lex(text, posix=True): ok = False if not _push_repo_selftest(): ok = False + if not _no_origin_selftest(): + ok = False if not _alias_push_selftest(origin): ok = False _PINNED_ALIASES.clear() diff --git a/tests/test_docker_task_workflow.py b/tests/test_docker_task_workflow.py index 5a39e18af..c4d05aa25 100755 --- a/tests/test_docker_task_workflow.py +++ b/tests/test_docker_task_workflow.py @@ -5,6 +5,18 @@ that pushes, the caller's hook step included, and the hook step must receive the inputs it builds from. The job and step sets are read from the workflow, so a reordered, renamed, or added job is judged by the same rule rather than by a hard-coded index. + +The reader is an indentation reader on the standard library alone, not a YAML or shell parser. +Its remaining limits are listed here, and all but the last two have a `KnownLimitsCase` case below, so a change that alters one is noticed. +A push inside a heredoc body, after a quoted separator, or in a trailing shell comment is read as a push. +A `type=registry` cache source is read as a push. +A command folded across lines without a backslash is not joined. +A flow-style `with` mapping spanning lines is not read. +A block-scalar login value is not read as the secret. +An inverted credential check is accepted, and the secret assignments and `-z` tests are matched as text anywhere in the step. +A push by `docker compose push`, `imagetools create`, `--output type=image,push=true`, or a buildx-level option before `build` is missed. +`with: push: false` is read as a push, and only lowercase `--push=false` and `--push=0` are read as no push. +Two further limits have no case: a bare `-` sequence item ends the step list, and a `run` body less than two columns deeper than its key is not read. """ from __future__ import annotations @@ -68,6 +80,9 @@ def split_flow(body: str) -> list[str]: def normalize_item(item: list[str]) -> list[str]: """Re-indent one step to the block layout the readers expect and expand a one-line `with` mapping.""" + first = re.fullmatch(r"(\s*)- +((?!name:)[A-Za-z0-9_-]+:.*)", item[0]) + if first: + item = [f"{first.group(1)}-", f"{first.group(1)} {first.group(2)}", *item[1:]] shift = 6 - (len(item[0]) - len(item[0].lstrip())) lines = [ (" " * shift + line if shift >= 0 else line[min(-shift, len(line) - len(line.lstrip())) :]) @@ -130,15 +145,24 @@ def with_values(item: list[str]) -> dict[str, str]: """The keys and values directly under the step's `with:`, ignoring comments.""" values: dict[str, str] = {} in_with = False + child = "" for line in item: if re.fullmatch(r" with:\s*(#.*)?", line): in_with = True elif in_with: - pair = re.match(r" [\"']?([A-Za-z0-9_-]+)[\"']?:\s*(.*?)\s*$", line) + if not child and line.strip() and not line.lstrip().startswith("#"): + child = line[: len(line) - len(line.lstrip())] + if len(child) <= 8: + break + pair = ( + re.match(rf"{child}[\"']?([A-Za-z0-9_-]+)[\"']?:\s*(.*?)\s*$", line) + if child + else None + ) if pair: value = re.sub(r"\s+#.*$", "", pair.group(2)).strip("\"'") values[pair.group(1)] = re.sub(r"\$\{\{\s*(.*?)\s*\}\}", r"${{ \1 }}", value) - elif line.strip() and not line.startswith(" "): + elif line.strip() and not line.startswith(child): break return values @@ -180,9 +204,10 @@ def is_login(step: Step) -> bool: ) +DOCKER = r"\bdocker(?:\s+--?[A-Za-z][\w-]*(?:=\S+|\s+[^-\s]\S*)?)*?" PUSH_COMMAND = re.compile( - r"\bdocker\s+(?:(?:image|manifest|buildx)\s+)?push\b" - r"|\bdocker\s+(?:buildx\s+)?(?:build|bake)\b.*(?:--push\b|type=registry\b)" + rf"{DOCKER}\s+(?:(?:image|manifest|buildx)\s+)?push\b" + rf"|{DOCKER}\s+(?:buildx\s+)?(?:build|bake)\b.*(?:--push\b(?!=(?:false|0)\b)|type=registry\b)" ) @@ -193,9 +218,10 @@ def run_script(step: Step) -> str: match = re.fullmatch(r" run:\s*(.*)", line) if match: body = [match.group(1)] - body.extend( - rest for rest in lines[i + 1 :] if rest.startswith(" ") or not rest.strip() - ) + for rest in lines[i + 1 :]: + if rest.strip() and not rest.startswith(" "): + break + body.append(rest) return re.sub(r"\\\n\s*", " ", "\n".join(body)) return "" @@ -527,5 +553,147 @@ def test_login_value_comment_and_default_registry_are_clean(self) -> None: self.assertEqual([], violations(mutated)) +def before_login(step: str) -> str: + """The workflow with the step text written before the login step of the build-docker job.""" + return insert_before_login(DockerTaskWorkflowCase.workflow, "build-docker", step + "\n") + + +class ReaderFixCase(unittest.TestCase): + """Shapes the reader once missed or misjudged and now reads correctly.""" + + def test_first_key_run_or_uses_push_is_caught(self) -> None: + for step in ( + " - run: docker push example/image:tag\n name: Early push step\n", + " - uses: ./.github/actions/docker-build-example\n name: Early hook step\n", + ): + with self.subTest(step=step): + found = violations(before_login(step)) + self.assertIn("build-docker: no login before the first push step", found) + + def test_docker_global_options_before_push_are_caught(self) -> None: + for command in ( + "docker --config /tmp/example push example/image:tag", + "docker -H tcp://example:2375 image push example/image:tag", + "docker --debug buildx build --push .", + ): + with self.subTest(command=command): + step = f" - name: Early push step\n run: {command}\n" + found = violations(before_login(step)) + self.assertIn("build-docker: no login before the first push step", found) + + def test_with_child_indented_deeper_than_ten_is_read(self) -> None: + step = " - name: Early push step\n uses: example/push@v1\n with:\n push: true\n" + found = violations(before_login(step)) + self.assertIn("build-docker: no login before the first push step", found) + + def test_push_false_is_not_a_push(self) -> None: + for command in ("docker buildx build --push=false .", "docker buildx build --push=0 ."): + with self.subTest(command=command): + step = f" - name: Local build step\n run: {command}\n" + self.assertEqual([], violations(before_login(step))) + + def test_env_value_after_run_is_not_part_of_the_script(self) -> None: + step = ( + " - name: Quiet step\n run: true\n" + " env:\n NOTE: docker push later\n" + ) + self.assertEqual([], violations(before_login(step))) + + +class KnownLimitsCase(unittest.TestCase): + """Each case asserts the reader's CURRENT behavior for a shape it does not judge correctly.""" + + def test_limit_heredoc_body_push_is_flagged(self) -> None: + step = " - name: Doc step\n run: |\n cat < None: + step = ' - name: Echo step\n run: echo "a; docker push example/image"\n' + self.assertIn( + "build-docker: no login before the first push step", violations(before_login(step)) + ) + + def test_limit_trailing_shell_comment_push_is_flagged(self) -> None: + step = " - name: Comment step\n run: true # docker push example/image\n" + self.assertIn( + "build-docker: no login before the first push step", violations(before_login(step)) + ) + + def test_limit_registry_cache_source_is_flagged(self) -> None: + step = " - name: Cache step\n run: docker buildx build --cache-from type=registry,ref=example/cache .\n" + self.assertIn( + "build-docker: no login before the first push step", violations(before_login(step)) + ) + + def test_limit_folded_run_push_is_missed(self) -> None: + step = " - name: Folded step\n run: >\n docker\n push example/image\n" + self.assertEqual([], violations(before_login(step))) + + def test_limit_plain_multiline_run_push_is_missed(self) -> None: + step = " - name: Plain step\n run: docker\n push example/image\n" + self.assertEqual([], violations(before_login(step))) + + def test_limit_multiline_flow_with_push_is_missed(self) -> None: + step = " - name: Flow step\n uses: example/push@v1\n with: {\n push: true}\n" + self.assertEqual([], violations(before_login(step))) + + def test_limit_other_push_forms_are_missed(self) -> None: + for command in ( + "docker compose push", + "docker buildx imagetools create -t example/image:tag example/image:src", + "docker buildx build --output type=image,name=x,push=true .", + "docker buildx --builder b build --push .", + ): + with self.subTest(command=command): + step = f" - name: Early step\n run: {command}\n" + self.assertEqual([], violations(before_login(step))) + + def test_limit_uppercase_push_false_is_read_as_a_push(self) -> None: + step = " - name: Local step\n run: docker buildx build --push=FALSE .\n" + self.assertIn( + "build-docker: no login before the first push step", violations(before_login(step)) + ) + + def test_limit_push_false_input_is_read_as_a_push(self) -> None: + step = " - name: Local step\n uses: example/build@v1\n with:\n push: false\n" + self.assertIn( + "build-docker: no login before the first push step", violations(before_login(step)) + ) + + def test_limit_secret_assignment_held_in_another_value_is_accepted(self) -> None: + workflow = DockerTaskWorkflowCase.workflow + mutated = workflow + for name in CREDENTIAL_NAMES: + line = f" {name}: ${{{{ secrets.{name} }}}}" + mutated = mutated.replace(line, f' NOTE_{name}: "{line.strip()}"') + self.assertNotEqual(workflow, mutated) + self.assertEqual([], violations(mutated)) + + def test_limit_block_scalar_login_value_is_not_read(self) -> None: + password = " password: ${{ secrets.DOCKER_HUB_ACCESS_TOKEN }}" + workflow = DockerTaskWorkflowCase.workflow + mutated = workflow.replace( + password, " password: >-\n ${{ secrets.DOCKER_HUB_ACCESS_TOKEN }}" + ) + self.assertNotEqual(workflow, mutated) + self.assertIn("build-docker: no login before the first push step", violations(mutated)) + + def test_limit_inverted_credential_check_is_accepted(self) -> None: + joined = '-z "$DOCKER_HUB_USERNAME" || -z "$DOCKER_HUB_ACCESS_TOKEN"' + workflow = DockerTaskWorkflowCase.workflow + mutated = workflow.replace(f"[[ {joined} ]]", f"! [[ {joined} ]]") + self.assertNotEqual(workflow, mutated) + self.assertEqual([], violations(mutated)) + + def test_limit_check_tests_inside_an_echo_are_accepted(self) -> None: + joined = '-z "$DOCKER_HUB_USERNAME" || -z "$DOCKER_HUB_ACCESS_TOKEN"' + workflow = DockerTaskWorkflowCase.workflow + mutated = workflow.replace(f"if [[ {joined} ]]; then", f"echo '{joined}'; if false; then") + self.assertNotEqual(workflow, mutated) + self.assertEqual([], violations(mutated)) + + if __name__ == "__main__": unittest.main()