Internal Guard - #74
Closed
aaronsmulktis wants to merge 8 commits into
Closed
Conversation
- theme design vars - set default layout - set default theme - use design vars in todo app - update README - add theme & layout to appserver - retain appearance for legacy usage - add theme & layout to registry and session - 44 tests passing
[Improvement] Todo layout
aaronsmulktis
requested review from
marksibrahim
and
a lite review from Copilot
August 19, 2026 01:34
meta-cla
Bot
added
the
CLA Signed
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a multi-layer “internal content guard” to prevent internal-only branches/commits/strings from being pushed or merged into the public repository, combining local git hooks, a CI workflow, and an importable GitHub ruleset, plus developer setup/docs.
Changes:
- Adds per-clone setup script to configure
core.hooksPathand a custom merge driver for divergent internal/public paths. - Introduces local hooks (
pre-push,commit-msg) and a merge driver (keep-internal) to block/internalize content and reduce merge conflicts. - Adds a PR workflow and a server-side ruleset JSON to detect/block internal refs and internal-only artifacts.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/setup-dev.sh | New setup script to install/check hooksPath and merge driver configuration. |
| CONTRIBUTING.md | Documents required dev setup step and reserved branch namespace. |
| .github/workflows/guard-internal.yml | PR-time checks to reject internal branch names, internal-only paths, and Internal-Only commits. |
| .github/rulesets/block-internal-refs.json | Importable ruleset to reject pushes of internal branch refs server-side. |
| .githooks/README.md | Documents the hook system, patterns, and limitations. |
| .githooks/pre-push | Implements local push guard with ref-name, lineage, and content-based checks. |
| .githooks/keep-internal | Merge driver to keep internal version of divergent files and emit drift notices. |
| .githooks/commit-msg | Stamps commits made on internal branches with an Internal-Only trailer. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+76
to
+80
| if ! git remote get-url internal >/dev/null 2>&1 && [ -z "$( | ||
| git for-each-ref --count=1 --format='%(refname)' \ | ||
| refs/heads/int refs/remotes/internal/int 2>/dev/null)" ]; then | ||
| exit 0 | ||
| fi |
Comment on lines
+120
to
+122
| internal_refs=$(git for-each-ref --format='%(refname)' \ | ||
| refs/heads/int refs/remotes/internal/int 2>/dev/null | tr '\n' ' ') | ||
|
|
|
|
||
| # Everything below here is a push to a PUBLIC remote. | ||
|
|
||
| tmpdir=$(mktemp -d) || die "could not create temp dir" |
Comment on lines
+35
to
+38
| case "$HEAD_REF" in | ||
| int|int/*) | ||
| echo "::error::Head branch '$HEAD_REF' is an internal branch and cannot be merged to a public branch." | ||
| exit 1 |
Comment on lines
+25
to
+33
| "conditions": { | ||
| "ref_name": { | ||
| "include": [ | ||
| "refs/heads/int", | ||
| "refs/heads/int/**" | ||
| ], | ||
| "exclude": [] | ||
| } | ||
| }, |
Comment on lines
+190
to
+197
| case "$local_ref" in | ||
| refs/heads/int|refs/heads/int/*) | ||
| banner "$local_ref" "$remote_ref" | ||
| printf ' internal branch pushed to a public remote\n' >&2 | ||
| failed=1 | ||
| continue | ||
| ;; | ||
| esac |
Comment on lines
+20
to
+23
| case "$branch" in | ||
| int|int/*) ;; | ||
| *) exit 0 ;; | ||
| esac |
Keeps internal-only work on int/* branches and out of the public remotes, without prefixing filenames or maintaining two clones. - .githooks/pre-push: when the destination is a public remote, refuses int/* refs, commits reachable from int/* that are not yet public, and content matching an internal pattern list. Inert in a clone that has neither an internal remote nor any int/* ref. - .githooks/keep-internal: merge driver for files that deliberately differ at the same path between public and int/*. Keeps the internal copy and reports drift when public also changed that path. - .githooks/commit-msg: stamps int/* commits with an Internal-Only trailer so they remain identifiable after a cherry-pick, which defeats the lineage check by producing a fresh SHA. - scripts/setup-dev.sh: per-clone setup plus a --check mode. Required rather than automatic, because git does not let a repository install its own hooks at clone time. - .github/workflows/guard-internal.yml: pull-request equivalents of the branch name, artifact, and trailer checks. - .github/rulesets/block-internal-refs.json: importable ruleset rejecting int/* refs server-side. The only layer here that prevents rather than detects; the hook and the workflow are both bypassable.
The guard rejects int/* pushes and int/* pull requests, but nothing told an outside contributor that the namespace was reserved or why, so the failure would read as an unexplained CI error. Explains the reservation in terms of what it buys -- a file at the same path on both sides, so cluster configs stop being renamed -- and states plainly that normal contributions never touch any of it. Also adds internal/ to the workflow's internal-only path list, alongside .gitattributes and .githooks/patterns.txt.
Merging a public-safe branch into int/* before pushing it to a public remote makes its commits reachable from int/* , so check 2 puts them in the taint set and refuses the push that would have made them public. The old message sent people the wrong way -- 'internal work belongs on int/*' -- when the commits were public-safe all along, and the only reading left was --no-verify. The check is correct; the recovery is an ordering rule. Push the source branch public first and the commits subtract out of the taint set. Hook now says so at the point of failure, and .githooks/README.md states the rule up front, since the order is easier to remember than the recovery. Hit this while setting up int/main, which is how it surfaced.
setup-dev.sh keyed 'clone shape' on the internal remote alone, so a clone holding int/* branches without that remote was told 'public-only clone -- nothing internal can leak from here'. That is the reassuring message in the one shape where every configured remote is public and internal commits are sitting right there. .githooks/pre-push already arms on either condition -- remote OR int/* refs -- precisely because removing the remote should not disarm it. Match that here, and name the mismatched shape as its own warning rather than folding it into either of the other two. The merge-driver check moves onto the same combined condition, since a clone with int/* branches needs the driver whether or not the remote is wired up. Found by running --check from int/main in a clone with no internal remote.
int/* was the only guarded namespace, so a branch named internal/my-work got nothing: no ref block, no Internal-Only stamp, no taint, no ruleset. That is the likeliest wrong guess now that the repo has an internal/ directory, and an unguarded near-miss is worse than a second guarded spelling. Adds internal and internal/* to all four layers, plus the two ref queries in pre-push and the one in setup-dev.sh. int/* stays canonical -- the trunk is still int/main and patterns.txt is still read from there. internal/* is blocked, not promoted. int-* and internal-* are deliberately left alone. They are not namespaces, and git's ref matching only breaks at slashes, so widening to them would have to be a prefix match -- which would catch aaronsmulktis/internal-guard, the branch carrying this very change. Verified refs/heads/internal matches internal/work but not internal-guard or internal2. The namespace list now appears in five files. pre-push holds the canonical comment and the others point at it.
aaronsmulktis
force-pushed
the
aaronsmulktis/internal-guard
branch
from
August 24, 2026 23:01
aaaa4be to
10c5a63
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three-layer guard against pushing internals into the public:
int/*branches at push time (manual config in Github)int/*branches, internal commits & internal strings (except w/--no-verify)Defining internals:
int,int/<name>,internal,internal/<name>. Notint-xorinternal-x..gitattributes,.githooks/patterns.txtor just anything ininternal/.githooks/patterns.txt. Hook scans patch text and commit messages not already publicConfiguration
internalgit remote./scripts/setup-dev.shinstalls./scripts/setup-dev.sh --checkstatus, exits if broken/missing guardAdmins only:
.github/rulesets/block-internal-refs.jsonunder Settings -> Rules -> Rulesets on public repoNotes:
pre-pushexits immediately if clone has nointernalremote and noint/*branches. Anyone who clonesfacebookresearch/OpenAppsnever sees the hook fire, is never warned, is never asked for setup.Definitions
-X ours/theirsmerge strategy but configured at the file-level.