chore: remove org-specific data from public repo, add CLAUDE.md rule#455
Merged
Conversation
…E.md rule
Kapsis is a public, general-purpose tool — internal hostnames, employer
names, and personal identifiers don't belong in tracked files. Removed all
remaining Taboola-specific data found in the repo (the personal aviad-*.yaml
configs are already gitignored and untouched — this is only about tracked,
public files):
- configs/network-allowlist.yaml: removed the hardcoded
git.taboolasyndication.com / artifactory*.taboolasyndication.com /
ci.taboolasyndication.com entries, replaced with commented-out generic
examples. Added configs/network-allowlist-personal.yaml (gitignored via
the existing configs/*-personal.yaml pattern) with the real Taboola
domains restored, so nothing is lost — just moved out of the public repo.
- scripts/lib/git-remote-utils.sh: detect_git_provider() had
git.taboolasyndication.com hardcoded as a special case for self-hosted
Bitbucket Server detection (needed because that hostname doesn't contain
"bitbucket"). Replaced with a KAPSIS_BITBUCKET_SERVER_HOSTS env var
(comma-separated hostnames) — same capability, no hardcoded org.
- scripts/lib/security.sh: reworded a comment that named "a Taboola
corporate Mac" as the source of an example UID, to generic phrasing.
- docs/CONFIG-REFERENCE.md, configs/build-profiles/java-dev.yaml: removed
"Taboola Java development" / "(Taboola default)" wording, several
git.taboolasyndication.com example hostnames, and personal
username/service-name examples ("aviad.s", "bitbucket-deeperdive-bot",
"taboola-bitbucket") in keychain config examples — replaced with
git.example.com and a placeholder name (jon.d — not ${USER}, which isn't
reliably evaluated in every config context this repo has).
- tests/test-git-remote-utils.sh, test-secret-store-injection.sh,
test-committer-identity.sh, test-git-credential-helper.sh: same cleanup
in test fixtures/assertions, plus a new test for the
KAPSIS_BITBUCKET_SERVER_HOSTS mechanism.
- CLAUDE.md: added a "Personal/Organization Configuration Separation"
section (plus a "Things to Avoid" entry pointing to it) so this doesn't
need to be re-litigated — documents where personal/org config belongs
(configs/*-personal.yaml, gitignored agent-sandbox.yaml), the preference
for pluggable mechanisms over hardcoding one org's specifics, and the
placeholder conventions to use in docs/tests instead.
Verified: all 4 modified test files pass (95/95 total), shellcheck clean,
network-allowlist.yaml and the new personal file both valid YAML, and
detect_git_provider's new env-var-based Bitbucket Server detection tested
directly (works with the env var set, correctly falls through to "unknown"
without it, doesn't affect generic bitbucket.* hostname detection).
aviadshiber
changed the title
Verified several claims against the actual current repo state and found real drift, plus compressed content that duplicates code comments (a staleness risk in itself — two copies of the same fact require remembering to update both): Fixed genuine inaccuracies: - Hardcoded version "(v2.34.2)" — actual latest is v2.38.3. Removed the specific version entirely (it goes stale within days of any release, since releases are cut on every merge to main); point at VERSION/GitHub Releases instead. - "111 test files" / "18 guides" — actual counts are 122 and 19. Replaced with commands to check the current count rather than a number that will drift again. - scripts/lib/ file listing was missing 2 files that already exist (overlay-sandbox.sh, worktree-guard.sh) and duplicated the Key Files table below almost entirely — collapsed to a breadcrumb pointing at that table + `ls scripts/lib/`. - Dashboard Sync Rule said the AgentStatus TypeScript interface lives in dashboard/server/src/types.ts — it doesn't; that file just re-exports dashboard/shared/src/index.ts, which is where it's actually defined and where edits need to go. Fixed. - Config Resolution Order was missing a real resolution step (<project>/agent-sandbox.yaml) that scripts/launch-agent.sh actually checks — verified against the source and added it. Compressed to breadcrumbs (verified accurate but duplicated exact thresholds/timing already documented in the implementing script's own header comments — Mount Failure Detection, Hung Agent Detection, Commit Failure Detection): kept the one-paragraph summary and actionable bits (recovery commands, config keys), replaced the multi-paragraph blow-by-blow mechanism descriptions with a pointer to the source file, since that's the copy that can't drift out of sync with itself. Left alone (verified accurate, not duplicated, genuinely useful process guidance): Release Artifact Rule, Dashboard Sync Rule's four-surface checklist, Push Fallback, Commit Artifact Filtering, Agent Partial Completion, exit-code table. 554 -> 488 lines.
Owner
Author
Added: CLAUDE.md stale-data cleanupPer follow-up request, also cleaned up CLAUDE.md (554 → 488 lines). Verified claims against the actual repo rather than just trimming for length: Real inaccuracies found and fixed:
Compressed to breadcrumbs (verified accurate, but duplicating exact thresholds/timing already in the implementing script's own header comments — Mount Failure Detection, Hung Agent Detection, Commit Failure Detection sections): kept the summary + actionable bits (recovery commands, config keys), replaced multi-paragraph internals with a pointer to the source file, since that copy can't drift out of sync with itself. Left alone (verified accurate, not duplicated, genuinely useful process guidance): Release Artifact Rule, Dashboard Sync Rule's four-surface checklist, Push Fallback, Commit Artifact Filtering, Agent Partial Completion, exit-code table. |
4 tasks
aviadshiber
added a commit
that referenced
this pull request
Jul 13, 2026
#456) * feat(git): support explicit KAPSIS_GIT_PROVIDER / KAPSIS_GIT_PR_URL_TEMPLATE overrides Replaces the KAPSIS_BITBUCKET_SERVER_HOSTS hostname-list hack (PR #455) with a general provider override plus a URL-template escape hatch. Public host auto-detection (github.com/gitlab.com/bitbucket.org) is unchanged and remains the zero-config default; the override only matters for self-hosted or otherwise undetectable instances. Adds azure-devops as a known provider. * feat(git): wire git.provider / git.pr_url_template config keys through launch-agent.sh Mirrors the existing git.attribution.commit/pr parsing pattern exactly: parsed via yq, exported both as a container -e flag and a host-side shell export (post-container-git.sh is sourced in the same process). Empty by default — zero-config auto-detection for public hosts is unaffected. * docs: document git.provider / git.pr_url_template config keys * fix(git): escape pr_url in status.json and reject non-http(s) PR URLs Ensemble review findings on git.pr_url_template (explicit git-provider config): 1. CRITICAL: pr_url was the only status.sh field embedded in status.json without running through _status_json_escape, unlike message/error/ branch/worktree/push_fallback/gist. Previously harmless because pr_url was only ever built from hardcoded formats + repo_path/branch (which can't contain a `"`). git.pr_url_template makes the whole value attacker-controllable (e.g. from a repo-embedded .kapsis/config.yaml), so an unescaped `"` in a template could inject arbitrary sibling JSON keys - corrupting fields like exit_code/error_type/push_status that drive automation decisions. 2. generate_pr_url() echoed whatever the template substitution (or built-in provider formats) produced with no validation. Since the result becomes a clickable "Create PR" link surfaced in the dashboard, a malicious template (e.g. containing javascript:/data:) could produce a dangerous link. Added a final `^https?://` check as a safety net. Both fixes are needed and non-redundant: a template like `https://x/"},"exit_code":"0` passes the scheme check but still needs _status_json_escape to avoid injecting sibling JSON keys. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(git): normalize Azure DevOps SSH remotes for PR URLs; add test coverage Fix 3 (already folded into the previous commit's git-remote-utils.sh diff, documented here for the tests that pin it): azure-devops's generate_pr_url case used the raw remote_url, which breaks for SSH-style Azure DevOps remotes (git@ssh.dev.azure.com:v3/org/project/repo or ssh://git@ssh.dev.azure.com/v3/org/project/repo) - Azure's SSH path shape (v3/{org}/{project}/{repo}, no _git segment) doesn't match its https PR-creation shape (https://dev.azure.com/{org}/{project}/_git/{repo}). Added _generate_azure_devops_pr_url() to translate SSH remotes to the correct https form, requiring exactly 3 path segments so a malformed remote degrades to empty (via the Fix 2 scheme check) rather than a broken link. Test coverage added (Fix 4): - test_generate_pr_url_azure_devops_ssh_remote: SSH remote -> correct https PR URL. - test_detect_git_provider_azure_devops_no_override_is_unknown: pins that dev.azure.com genuinely needs the explicit KAPSIS_GIT_PROVIDER override (no auto-detect pattern exists for it). - test_generate_pr_url_provider_and_template_both_set_template_wins: pins the documented priority order when both env vars are set. - test_generate_pr_url_empty_template_falls_through_to_provider: confirms an explicitly empty KAPSIS_GIT_PR_URL_TEMPLATE is treated as unset by the `[[ -n ... ]]` guard, not as "set to nothing". - test_config_git_provider_parsing_with_values_set: yq extraction of git.provider/git.pr_url_template from a fixture with real values set, not just the empty-default fallback the existing test covered. - test_config_git_provider_env_export_wiring: exercises launch-agent.sh's actual parse_config() (not just git-remote-utils.sh in isolation) to confirm GIT_PROVIDER/GIT_PR_URL_TEMPLATE are set from a temp config. Scoped to parse_config() only - the env-export half (GIT_PROVIDER -> KAPSIS_GIT_PROVIDER/ENV_VARS, launch-agent.sh:2295-2301) lives in generate_env_vars(), which has many interdependent prerequisites (WORKTREE_PATH, AGENT_COMMAND inference, GIT_ATTRIBUTION_*) that make it impractical to invoke in isolation without a much larger test harness. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): guard readonly redeclaration; satisfy dashboard-sync check - scripts/lib/git-remote-utils.sh: _KAPSIS_VALID_GIT_PROVIDERS was declared readonly unconditionally, which aborts with "readonly variable" when the file is sourced twice in the same shell (happens in tests/test-post-container-git.sh). Guard with a presence check. - dashboard/server/src/types.ts: document that the pr_url escaping fix in scripts/lib/status.sh (PR #456) is not a schema change, satisfying dashboard-sync.yml's file-presence check per its established convention (see prior PR #435/#409/#430 sync comments in this file). * fix(git): recognize org@ HTTPS Azure DevOps remotes; validate path shape Azure DevOps' actual default Clone-button HTTPS URL form is https://{org}@dev.azure.com/{org}/{project}/_git/{repo} (with a userinfo org@ prefix), which the HTTPS branch of _generate_azure_devops_pr_url() did not recognize — the entire Azure DevOps feature silently produced nothing for this, the most common input shape. Broaden the HTTPS match to tolerate an optional {something}@ userinfo prefix, then validate the remaining path against the same org/project/_git/repo shape the SSH branch already enforces, returning empty for malformed input instead of a plausible-but-wrong URL. Also fixes a stale test assertion message referencing the removed KAPSIS_BITBUCKET_SERVER_HOSTS env var, and adds regression coverage for: - the org@ HTTPS normalization and its malformed-path rejection - the generate_pr_url scheme-validation guard (javascript: / scheme-less templates) - the readonly-redeclaration guard (double-sourcing the library) - malformed Azure DevOps SSH remote rejection Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(status): add regression coverage for pr_url JSON-injection escaping status_complete() escapes pr_url via _status_json_escape() before embedding it in status.json, but no test proved this — a revert of that escaping would pass the existing suite silently. Add a test that feeds a pr_url containing a double-quote and backslash and asserts the resulting file is valid JSON with the quote escaped, not raw. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Aviad Shiber <aviad.s@taboola.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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
2 participants
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.
Summary
Kapsis is a public, general-purpose tool. Found and removed remaining Taboola-specific data in tracked files (your personal
configs/aviad-*.yamlfiles are already gitignored and were never touched — this is only about what's actually public).configs/network-allowlist.yaml: removed hardcodedgit.taboolasyndication.com/artifactory*.taboolasyndication.com/ci.taboolasyndication.com, replaced with commented-out generic examples. Addedconfigs/network-allowlist-personal.yaml(gitignored via the existingconfigs/*-personal.yamlpattern) with the real domains restored, so nothing is lost.scripts/lib/git-remote-utils.sh:detect_git_provider()hadgit.taboolasyndication.comhardcoded as a special case (needed because that hostname doesn't contain "bitbucket"). Replaced with aKAPSIS_BITBUCKET_SERVER_HOSTSenv var — same capability, no hardcoded org.scripts/lib/security.sh: reworded a comment naming "a Taboola corporate Mac" to generic phrasing.docs/CONFIG-REFERENCE.md,configs/build-profiles/java-dev.yaml: removed "Taboola Java development" wording and severalgit.taboolasyndication.com/ personal-username / product-codename examples in doc/config examples.KAPSIS_BITBUCKET_SERVER_HOSTSmechanism.CLAUDE.md: added a "Personal/Organization Configuration Separation" section so this is a durable rule, not something to re-discover — where personal/org config belongs, preference for pluggable mechanisms over hardcoding, and placeholder conventions (git.example.com,jon.d, etc.) for docs/tests going forward.Verification
All 4 modified test files pass (95/95 total), shellcheck clean, both YAML files valid. Directly tested
detect_git_provider's new env-var-based detection: works withKAPSIS_BITBUCKET_SERVER_HOSTSset, correctly falls through tounknownwithout it, doesn't affect genericbitbucket.*hostname detection.No file overlap with #454 (different subsystems entirely) — independent, mergeable in any order.