Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
505b6f799c |
@@ -34,12 +34,18 @@ EOF
|
||||
# get_remote_host and get_gitea_token are provided by detect-platform.sh
|
||||
|
||||
get_state_from_status_json() {
|
||||
python3 - <<'PY'
|
||||
# Capture piped JSON BEFORE invoking `python3 - <<PY`. The heredoc binds
|
||||
# stdin to the Python program text — so json.load(sys.stdin) inside would
|
||||
# try to re-read stdin after `-` already consumed it for the program,
|
||||
# yielding EOF and returning "unknown" every time. Pass payload via env.
|
||||
local payload
|
||||
payload=$(cat)
|
||||
CI_QUEUE_STATUS_JSON="$payload" python3 - <<'PY'
|
||||
import json
|
||||
import sys
|
||||
import os
|
||||
|
||||
try:
|
||||
payload = json.load(sys.stdin)
|
||||
payload = json.loads(os.environ.get("CI_QUEUE_STATUS_JSON", ""))
|
||||
except Exception:
|
||||
print("unknown")
|
||||
raise SystemExit(0)
|
||||
@@ -83,12 +89,15 @@ PY
|
||||
}
|
||||
|
||||
print_pending_contexts() {
|
||||
python3 - <<'PY'
|
||||
# Same stdin hazard as get_state_from_status_json above — pass payload via env.
|
||||
local payload
|
||||
payload=$(cat)
|
||||
CI_QUEUE_STATUS_JSON="$payload" python3 - <<'PY'
|
||||
import json
|
||||
import sys
|
||||
import os
|
||||
|
||||
try:
|
||||
payload = json.load(sys.stdin)
|
||||
payload = json.loads(os.environ.get("CI_QUEUE_STATUS_JSON", ""))
|
||||
except Exception:
|
||||
print("[ci-queue-wait] unable to decode status payload")
|
||||
raise SystemExit(0)
|
||||
|
||||
@@ -254,32 +254,15 @@ from urllib.parse import urlparse
|
||||
|
||||
|
||||
def _origin_and_path(url):
|
||||
# Normalize a URL to (scheme-class, host, distinguishing-port) + comment path.
|
||||
#
|
||||
# #991: http and https collapse into ONE scheme class ("web"). A Gitea whose
|
||||
# ROOT_URL is configured http:// returns http:// object URLs even when every
|
||||
# client reaches it over https://, so a scheme-strict comparison rejects the
|
||||
# provider's own correct answer about a write that landed — a deterministic
|
||||
# false negative on every comment posted against such a deployment. The
|
||||
# scheme is also not what this check defends: the forgeries it exists to
|
||||
# catch (look-alike host, decoy path prefix, wrong owner/repo/number) all
|
||||
# vary the HOST or the PATH, both of which stay strict below. Any OTHER
|
||||
# scheme (file:, ftp:, javascript:) remains distinguishing and is rejected.
|
||||
#
|
||||
# Port: an implicit port and its own scheme's default compare equal, so
|
||||
# http://h == https://h. An EXPLICIT non-default port still distinguishes,
|
||||
# because a different port is a different service on the same host.
|
||||
# Normalize a URL to (scheme, host, effective-port) + comment path. The port
|
||||
# defaults to the scheme's default (80 http / 443 otherwise) so an implicit
|
||||
# port and its explicit default form compare equal.
|
||||
parsed = urlparse(url or "")
|
||||
scheme = (parsed.scheme or "").lower()
|
||||
host = (parsed.hostname or "").lower()
|
||||
if scheme in ("http", "https"):
|
||||
scheme_class = "web"
|
||||
default_port = 80 if scheme == "http" else 443
|
||||
port = None if parsed.port in (None, default_port) else parsed.port
|
||||
else:
|
||||
scheme_class = scheme
|
||||
port = parsed.port
|
||||
return (scheme_class, host, port), parsed.path.rstrip("/")
|
||||
default_port = 80 if scheme == "http" else 443
|
||||
port = parsed.port if parsed.port is not None else default_port
|
||||
return (scheme, host, port), parsed.path.rstrip("/")
|
||||
|
||||
|
||||
try:
|
||||
|
||||
@@ -243,35 +243,15 @@ from urllib.parse import urlparse
|
||||
|
||||
|
||||
def _origin_and_path(url):
|
||||
# Normalize a URL to (scheme-class, host, distinguishing-port) + comment path.
|
||||
#
|
||||
# #991: http and https collapse into ONE scheme class ("web"). A Gitea whose
|
||||
# ROOT_URL is configured http:// returns http:// object URLs even when every
|
||||
# client reaches it over https://, so a scheme-strict comparison rejects the
|
||||
# provider's own correct answer about a comment that landed — a deterministic
|
||||
# false negative on EVERY review comment posted against such a deployment.
|
||||
# That matters more here than anywhere else: on a host where no seat can
|
||||
# create a review OBJECT, the comment-form review record this path produces
|
||||
# is the only gate-16 evidence available, and this check refuses all of it.
|
||||
# The scheme is also not what the check defends: the forgeries it exists to
|
||||
# catch (look-alike host, decoy path prefix, wrong owner/repo/kind/number)
|
||||
# all vary the HOST or the PATH, both of which stay strict below. Any OTHER
|
||||
# scheme (file:, ftp:, javascript:) remains distinguishing and is rejected.
|
||||
#
|
||||
# Port: an implicit port and its own scheme's default compare equal, so
|
||||
# http://h == https://h. An EXPLICIT non-default port still distinguishes,
|
||||
# because a different port is a different service on the same host.
|
||||
# Normalize a URL to (scheme, host, effective-port) + comment path. The port
|
||||
# defaults to the scheme's default (80 http / 443 otherwise) so an implicit
|
||||
# port and its explicit default form compare equal.
|
||||
parsed = urlparse(url or "")
|
||||
scheme = (parsed.scheme or "").lower()
|
||||
host = (parsed.hostname or "").lower()
|
||||
if scheme in ("http", "https"):
|
||||
scheme_class = "web"
|
||||
default_port = 80 if scheme == "http" else 443
|
||||
port = None if parsed.port in (None, default_port) else parsed.port
|
||||
else:
|
||||
scheme_class = scheme
|
||||
port = parsed.port
|
||||
return (scheme_class, host, port), parsed.path.rstrip("/")
|
||||
default_port = 80 if scheme == "http" else 443
|
||||
port = parsed.port if parsed.port is not None else default_port
|
||||
return (scheme, host, port), parsed.path.rstrip("/")
|
||||
|
||||
|
||||
try:
|
||||
|
||||
@@ -0,0 +1,205 @@
|
||||
#!/usr/bin/env bash
|
||||
# Regression suite for #1019: ci-queue-wait.sh's status parser must return a
|
||||
# state DERIVED FROM ITS INPUT.
|
||||
#
|
||||
# The defect this pins: both python sites piped a payload into `python3 - <<'PY'`.
|
||||
# The heredoc binds stdin to the Python program text, so json.load(sys.stdin) saw
|
||||
# EOF, the bare `except` fired, and the function returned "unknown" for every input
|
||||
# — success, pending and failure alike. "unknown" then reaches an `exit 0` arm, so
|
||||
# the gate-6 queue guard passed unconditionally on both the gitea and github paths.
|
||||
#
|
||||
# Every assertion below is on the RETURNED STATE STRING. A test that asserted only
|
||||
# `rc=0` would have passed against the broken build, which is why this bug survived.
|
||||
#
|
||||
# The functions are extracted from the shipped wrapper rather than reimplemented, so
|
||||
# this suite measures the code that actually runs.
|
||||
|
||||
set -uo pipefail
|
||||
|
||||
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||
# Overridable so a mutation test can point the suite at a deliberately-broken copy
|
||||
# without touching the tracked file. The earlier version of this suite required
|
||||
# `git stash` + `git checkout` to do that, which left a repo-global stash entry
|
||||
# that any sibling worktree could have popped onto an unrelated branch.
|
||||
WRAPPER="${CI_QUEUE_WAIT_WRAPPER:-$SCRIPT_DIR/ci-queue-wait.sh}"
|
||||
|
||||
PASS=0
|
||||
FAIL=0
|
||||
|
||||
pass() { printf ' PASS %s\n' "$1"; PASS=$((PASS + 1)); }
|
||||
fail() { printf ' FAIL %s\n' "$1"; FAIL=$((FAIL + 1)); }
|
||||
|
||||
if [[ ! -f "$WRAPPER" ]]; then
|
||||
printf 'FATAL: wrapper not found at %s\n' "$WRAPPER" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
TMPDIR_T="$(mktemp -d)"
|
||||
trap 'rm -rf "$TMPDIR_T"' EXIT
|
||||
|
||||
extract_fn() {
|
||||
# $1 = function name, $2 = source file, $3 = destination
|
||||
sed -n "/^$1()/,/^}/p" "$2" > "$3"
|
||||
[[ -s "$3" ]] || { printf 'FATAL: could not extract %s from %s\n' "$1" "$2" >&2; exit 1; }
|
||||
}
|
||||
|
||||
extract_fn get_state_from_status_json "$WRAPPER" "$TMPDIR_T/state.sh"
|
||||
extract_fn print_pending_contexts "$WRAPPER" "$TMPDIR_T/contexts.sh"
|
||||
|
||||
state_of() {
|
||||
# shellcheck disable=SC1091
|
||||
( source "$TMPDIR_T/state.sh"; printf '%s' "$1" | get_state_from_status_json )
|
||||
}
|
||||
|
||||
expect_state() {
|
||||
local label="$1" payload="$2" want="$3" got
|
||||
got="$(state_of "$payload")"
|
||||
if [[ "$got" == "$want" ]]; then
|
||||
pass "$label -> $want"
|
||||
else
|
||||
fail "$label -> got '$got', want '$want'"
|
||||
fi
|
||||
}
|
||||
|
||||
echo "=== parser returns a state derived from its input (#1019) ==="
|
||||
|
||||
expect_state "success payload" \
|
||||
'{"state":"success","statuses":[{"status":"success","context":"ci/build"}]}' \
|
||||
'terminal-success'
|
||||
|
||||
expect_state "pending payload" \
|
||||
'{"state":"pending","statuses":[{"status":"pending","context":"ci/build"}]}' \
|
||||
'pending'
|
||||
|
||||
expect_state "failure payload" \
|
||||
'{"state":"failure","statuses":[{"status":"failure","context":"ci/build"}]}' \
|
||||
'terminal-failure'
|
||||
|
||||
expect_state "mixed success+pending is pending" \
|
||||
'{"state":"pending","statuses":[{"status":"success"},{"status":"pending"}]}' \
|
||||
'pending'
|
||||
|
||||
expect_state "running counts as pending" \
|
||||
'{"state":"pending","statuses":[{"status":"running"}]}' \
|
||||
'pending'
|
||||
|
||||
expect_state "error counts as failure" \
|
||||
'{"state":"failure","statuses":[{"status":"error"}]}' \
|
||||
'terminal-failure'
|
||||
|
||||
expect_state "empty status set is no-status" \
|
||||
'{"state":"","statuses":[]}' \
|
||||
'no-status'
|
||||
|
||||
# Control. This is the ONE input for which "unknown" is correct. Without it, a
|
||||
# regression that hardcoded "unknown" again would still fail the cases above but
|
||||
# the suite would give no signal that "unknown" remains reachable when it should be.
|
||||
expect_state "undecodable payload stays unknown" \
|
||||
'not json at all' \
|
||||
'unknown'
|
||||
|
||||
expect_state "unrecognised status vocabulary is unknown" \
|
||||
'{"state":"weird","statuses":[{"status":"weird"}]}' \
|
||||
'unknown'
|
||||
|
||||
echo "=== pending contexts are reported to the operator ==="
|
||||
|
||||
contexts_of() {
|
||||
# shellcheck disable=SC1091
|
||||
( source "$TMPDIR_T/contexts.sh"; printf '%s' "$1" | print_pending_contexts )
|
||||
}
|
||||
|
||||
out="$(contexts_of '{"statuses":[{"status":"pending","context":"ci/alpha"},{"status":"pending","context":"ci/beta"}]}')"
|
||||
if grep -q 'ci/alpha' <<<"$out" && grep -q 'ci/beta' <<<"$out"; then
|
||||
pass "both pending contexts emitted"
|
||||
else
|
||||
fail "pending contexts not emitted; got: $out"
|
||||
fi
|
||||
|
||||
out="$(contexts_of '{"statuses":[{"status":"success","context":"ci/alpha"}]}')"
|
||||
# Assert the POSITIVE message, not merely the absence of the context name. Absence
|
||||
# alone is satisfied by total silence — and the pre-fix build was silent, so an
|
||||
# absence-only assertion passed against the very defect this suite exists to catch.
|
||||
if grep -q 'ci/alpha' <<<"$out"; then
|
||||
fail "a non-pending context was emitted; got: $out"
|
||||
elif grep -q 'no pending contexts' <<<"$out"; then
|
||||
pass "non-pending context suppressed, and reported as 'no pending contexts'"
|
||||
else
|
||||
fail "expected an explicit 'no pending contexts' report; got: $out"
|
||||
fi
|
||||
|
||||
echo "=== needle: the broken construct is caught, not merely absent today ==="
|
||||
|
||||
# Rebuild the pre-fix form and assert this suite would have failed against it.
|
||||
# Without this, the suite proves the current file is correct but not that it can
|
||||
# detect the defect returning.
|
||||
BROKEN="$TMPDIR_T/broken.sh"
|
||||
cat > "$BROKEN" <<'BROKEN_EOF'
|
||||
get_state_from_status_json() {
|
||||
python3 - <<'PY'
|
||||
import json
|
||||
import sys
|
||||
|
||||
try:
|
||||
payload = json.load(sys.stdin)
|
||||
except Exception:
|
||||
print("unknown")
|
||||
raise SystemExit(0)
|
||||
print("terminal-success" if (payload.get("state") or "") == "success" else "pending")
|
||||
PY
|
||||
}
|
||||
BROKEN_EOF
|
||||
|
||||
broken_got="$( ( source "$BROKEN"; printf '%s' '{"state":"success","statuses":[]}' | get_state_from_status_json ) )"
|
||||
if [[ "$broken_got" == "unknown" ]]; then
|
||||
pass "[NEEDLE ] pre-fix construct reproduces the defect (returns 'unknown' for a success payload)"
|
||||
else
|
||||
fail "[NEEDLE ] pre-fix construct did NOT reproduce the defect; got '$broken_got' — the needle no longer pins anything"
|
||||
fi
|
||||
|
||||
# And assert the shipped wrapper does not contain that construct.
|
||||
#
|
||||
# The check must have HEREDOC SEMANTICS, not merely match the text. `json.load(sys.stdin)`
|
||||
# is perfectly correct under `python3 -c '...'` — there the program comes from argv, so
|
||||
# stdin really is the payload, and ci-queue-wait.sh uses that form legitimately in
|
||||
# gitea_get_branch_head_sha. Only `python3 - <<DELIM` is defective, because `-` has
|
||||
# already bound stdin to the program text.
|
||||
#
|
||||
# A flat `grep json\.load\(sys\.stdin\)` therefore fails on correct code. It did: the
|
||||
# first version of this needle flagged the innocent `python3 -c` site. Same shape as
|
||||
# #1018 F1 (a regex over raw text has no syntax semantics) reproduced inside the needle
|
||||
# written to pin a different instance of it.
|
||||
heredoc_stdin_sites() {
|
||||
awk '
|
||||
/python3[[:space:]]+-[[:space:]]*<</ {
|
||||
d = $0
|
||||
sub(/.*<<[[:space:]]*/, "", d)
|
||||
gsub(/['"'"'"]/, "", d)
|
||||
delim = d; inhd = 1; next
|
||||
}
|
||||
inhd && $0 == delim { inhd = 0; next }
|
||||
inhd {
|
||||
line = $0
|
||||
sub(/[[:space:]]*#.*$/, "", line)
|
||||
if (line ~ /sys\.stdin/) printf "%d: %s\n", FNR, $0
|
||||
}
|
||||
' "$1"
|
||||
}
|
||||
|
||||
# Positive control FIRST. An empty result from a broken scanner looks exactly like a
|
||||
# clean file, so the scanner is proven to fire before its silence is trusted.
|
||||
if [[ -n "$(heredoc_stdin_sites "$BROKEN")" ]]; then
|
||||
pass "[NEEDLE ] scanner detects a stdin read inside a python3-heredoc"
|
||||
else
|
||||
fail "[NEEDLE ] scanner did NOT fire on the known-broken form — its silence proves nothing"
|
||||
fi
|
||||
|
||||
sites="$(heredoc_stdin_sites "$WRAPPER")"
|
||||
if [[ -n "$sites" ]]; then
|
||||
fail "[NEEDLE ] wrapper reads stdin inside a python3-heredoc at: $sites"
|
||||
else
|
||||
pass "[NEEDLE ] wrapper has no stdin read inside any python3-heredoc"
|
||||
fi
|
||||
|
||||
printf '\nci-queue-wait parse: %d passed, %d failed\n' "$PASS" "$FAIL"
|
||||
[[ "$FAIL" -eq 0 ]] || exit 1
|
||||
@@ -41,13 +41,7 @@
|
||||
# that other host's credential cross-host;
|
||||
# 10. leaves NO temp files behind (POST/GET bodies + metadata) on either the
|
||||
# success or the failure path — nested function-scoped RETURN traps do not
|
||||
# clobber each other and every scratch file is removed on all exit paths;
|
||||
# 11. (#991) ACCEPTS a record whose provider-returned URL differs from the repo
|
||||
# remote ONLY in http-vs-https — the standing state on a deployment whose
|
||||
# Gitea ROOT_URL is misconfigured — while still REJECTING a different
|
||||
# explicit port and a non-web scheme. Every prior fixture here was https://
|
||||
# and every negative varied only host or path, so the one axis that fails
|
||||
# in production had zero coverage: the fixtures encoded the assumption.
|
||||
# clobber each other and every scratch file is removed on all exit paths.
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
@@ -67,29 +61,15 @@ STATE_FILE="$WORK_DIR/comments.json"
|
||||
# A dedicated scratch dir the wrapper is pointed at via TMPDIR, so the leak
|
||||
# check can assert every POST/GET body + metadata temp file is cleaned up.
|
||||
TMP_SCRATCH="$WORK_DIR/scratch"
|
||||
# #1007: a sandboxed HOME. detect-platform.sh's step-0 per-agent identity lookup
|
||||
# reads ~/.config/mosaic/gitea-tokens/<identity>, which is OUTSIDE both
|
||||
# XDG_CONFIG_HOME and MOSAIC_CREDENTIALS_FILE — so on any seat that has a real
|
||||
# per-agent token the suite resolves a PRODUCTION credential and dies at the
|
||||
# authenticated-identity read (HTTP 401) before reaching case 1. Pointing HOME
|
||||
# at the work dir keeps that lookup inside the sandbox.
|
||||
HOME_DIR="$WORK_DIR/home"
|
||||
|
||||
cleanup() {
|
||||
rm -rf "$WORK_DIR"
|
||||
}
|
||||
trap cleanup EXIT
|
||||
|
||||
mkdir -p "$REPO_DIR" "$BIN_DIR" "$XDG_DIR" "$TMP_SCRATCH" "$HOME_DIR"
|
||||
mkdir -p "$REPO_DIR" "$BIN_DIR" "$XDG_DIR" "$TMP_SCRATCH"
|
||||
git -C "$REPO_DIR" init -q
|
||||
git -C "$REPO_DIR" remote add origin https://git.mosaicstack.dev/mosaicstack/stack.git
|
||||
# #1007: shadow any GLOBAL `mosaic.gitIdentity` with an empty REPO-LOCAL value.
|
||||
# A repo-local key shadows the global and reads back empty at rc=0, which is the
|
||||
# only way to make step 0 fall through inside a sandbox. Note the env-var route
|
||||
# (MOSAIC_GIT_IDENTITY="") does NOT work: detect-platform.sh reads it with
|
||||
# `${MOSAIC_GIT_IDENTITY:-}`, and `:-` treats set-but-empty identically to
|
||||
# unset, so setting it empty is silently the same as not setting it at all.
|
||||
git -C "$REPO_DIR" config mosaic.gitIdentity ""
|
||||
|
||||
ISSUE_NUMBER=7
|
||||
REPO_SLUG="mosaicstack/stack"
|
||||
@@ -286,19 +266,6 @@ elif mode == "url-wrong-repo":
|
||||
elif mode == "url-suffix-injection":
|
||||
# Prefix-injected: a bare endswith("/<slug>/issues/7") test would ACCEPT this.
|
||||
issue_url = f"https://git.mosaicstack.dev/deceptive/{repo}/issues/7"
|
||||
elif mode == "url-scheme-downgrade":
|
||||
# #991, and the ONLY one of these modes that must be ACCEPTED. A Gitea whose
|
||||
# ROOT_URL is http:// returns http:// object URLs for a repo cloned over
|
||||
# https://. Same host, same path, correct record — the provider is telling
|
||||
# the truth about a write that landed.
|
||||
issue_url = f"http://git.mosaicstack.dev/{repo}/issues/7"
|
||||
elif mode == "url-wrong-port":
|
||||
# An EXPLICIT non-default port is a different service on the same host and
|
||||
# must stay distinguishing — collapsing the scheme must not collapse this.
|
||||
issue_url = f"https://git.mosaicstack.dev:8443/{repo}/issues/7"
|
||||
elif mode == "url-non-web-scheme":
|
||||
# Only http/https collapse. Any other scheme stays distinguishing.
|
||||
issue_url = f"ftp://git.mosaicstack.dev/{repo}/issues/7"
|
||||
record = {
|
||||
"id": new_id,
|
||||
"body": body,
|
||||
@@ -398,7 +365,6 @@ run_comment() {
|
||||
(
|
||||
cd "$REPO_DIR"
|
||||
PATH="$BIN_DIR:$PATH" \
|
||||
HOME="$HOME_DIR" \
|
||||
TMPDIR="$TMP_SCRATCH" \
|
||||
XDG_CONFIG_HOME="$XDG_DIR" \
|
||||
MOSAIC_CREDENTIALS_FILE="$CREDENTIALS_FILE" \
|
||||
@@ -579,17 +545,13 @@ if grep -q " $ACTING_LOGIN\$" "$AUTH_LOG"; then
|
||||
fi
|
||||
assert_no_temp_leak "cross-host"
|
||||
|
||||
# Cases 7-12 (#865 Blocker 3): the created record's id/author/body are all
|
||||
# correct, but its provider-returned issue_url does not belong to this issue on
|
||||
# this provider/repo. Verification pins the URL's ORIGIN (scheme-class + host +
|
||||
# explicit non-default port) and its FULL path (deployment prefix + exact
|
||||
# owner/repo + kind + number), so each must FAIL CLOSED. A bare endswith/suffix
|
||||
# test would wrongly accept the look-alike-host and prefix-injection variants.
|
||||
# url-wrong-port and url-non-web-scheme (#991) bound the scheme relaxation from
|
||||
# the other side: collapsing http/https must not also collapse a different port
|
||||
# or a different scheme family.
|
||||
for bad_mode in url-wrong-host url-wrong-owner url-wrong-repo url-suffix-injection \
|
||||
url-wrong-port url-non-web-scheme; do
|
||||
# Cases 7-10 (#865 Blocker 3): the created record's id/author/body are all
|
||||
# correct, but its provider-returned issue_url is forged. Verification pins the
|
||||
# URL's ORIGIN (scheme+host+effective-port) and its FULL path (deployment prefix
|
||||
# + exact owner/repo + kind + number), so each forgery must FAIL CLOSED. A bare
|
||||
# endswith/suffix test would wrongly accept the look-alike-host and
|
||||
# prefix-injection variants.
|
||||
for bad_mode in url-wrong-host url-wrong-owner url-wrong-repo url-suffix-injection; do
|
||||
if run_comment "$bad_mode"; then
|
||||
echo "FAIL: forged comment URL ($bad_mode) was accepted" >&2
|
||||
cat "$OUTPUT_FILE" >&2
|
||||
@@ -606,19 +568,4 @@ done
|
||||
# issue_url (already exercised by Case 1's fresh-success), so the tightened check
|
||||
# is not rejecting genuine writes.
|
||||
|
||||
# Case 13 (#991): the deployment's ROOT_URL is http:// while the repo remote is
|
||||
# https://, so the provider returns an http:// issue_url for a record that is
|
||||
# otherwise entirely correct. This is not a forgery — it is the provider's own
|
||||
# truthful answer about a write that landed — and a scheme-strict comparison
|
||||
# rejects it deterministically, on EVERY comment, converting a successful write
|
||||
# into a reported failure. It must be ACCEPTED. Host, path, owner, repo, kind
|
||||
# and number all remain strict; only the http/https distinction is relaxed.
|
||||
run_comment url-scheme-downgrade
|
||||
grep -q 'Added and verified comment on Gitea issue #7 (comment ID 1)' "$OUTPUT_FILE"
|
||||
# The write really happened and was read back by exact id — this case passes
|
||||
# through the same POST/GET chain as case 1, not around it.
|
||||
grep -q "^POST $API_BASE/issues/7/comments$" "$CURL_LOG"
|
||||
grep -q "^GET $API_BASE/issues/comments/1$" "$CURL_LOG"
|
||||
assert_no_temp_leak "url-scheme-downgrade"
|
||||
|
||||
echo "issue-comment.sh REST create + exact-id read-back regression passed"
|
||||
|
||||
@@ -436,19 +436,6 @@ elif mode == "comment-url-wrong-repo":
|
||||
elif mode == "comment-url-suffix-injection":
|
||||
# Prefix-injected: a bare endswith("/<slug>/pulls/123") test would ACCEPT it.
|
||||
pr_url = f"{_origin}/deceptive{_slug}/pulls/123"
|
||||
elif mode == "comment-url-wrong-port":
|
||||
# #991 bound: an EXPLICIT non-default port is a different service on the same
|
||||
# host. Relaxing http-vs-https must NOT relax this.
|
||||
pr_url = f"{_p.scheme}://{_p.hostname}:8443{_slug}/pulls/123"
|
||||
elif mode == "comment-url-non-web-scheme":
|
||||
# #991 bound: ONLY http/https collapse; any other scheme stays distinguishing.
|
||||
pr_url = f"ftp://{_p.netloc}{_slug}/pulls/123"
|
||||
elif mode == "comment-url-scheme-downgrade":
|
||||
# #991, and the only URL mode here that must be ACCEPTED. A Gitea whose
|
||||
# ROOT_URL is http:// returns http:// object URLs for a repo reached over
|
||||
# https://. Same host, same path, correct record — a truthful provider
|
||||
# answer about a comment that landed, not a forgery.
|
||||
pr_url = f"http://{_p.netloc}{_slug}/pulls/123"
|
||||
elif mode == "comment-mixed-case-slug":
|
||||
# #875: EXPECTED_REPO_SLUG is taken verbatim from GITEA_API_BASE and can be
|
||||
# mixed-case (e.g. "USC/uconnect"), but Gitea canonicalizes the returned
|
||||
@@ -903,16 +890,11 @@ fi
|
||||
assert_no_temp_leak "review-body-reuse"
|
||||
|
||||
# Cases 12-15 (#865 Blocker 3): a PR comment whose id/author/body are all correct
|
||||
# but whose provider-returned pull_request_url does not belong to this PR must
|
||||
# FAIL CLOSED. Verification pins the URL's ORIGIN (scheme-class + host + explicit
|
||||
# non-default port) and FULL path (deployment prefix + exact owner/repo + kind +
|
||||
# number); a bare endswith/suffix test would wrongly accept the look-alike-host
|
||||
# and prefix-injection variants. comment-url-wrong-port and
|
||||
# comment-url-non-web-scheme (#991) bound the scheme relaxation from the other
|
||||
# side: collapsing http/https must not also collapse a different port or a
|
||||
# different scheme family.
|
||||
for bad_mode in comment-url-wrong-host comment-url-wrong-owner comment-url-wrong-repo \
|
||||
comment-url-suffix-injection comment-url-wrong-port comment-url-non-web-scheme; do
|
||||
# but whose provider-returned pull_request_url is forged must FAIL CLOSED.
|
||||
# Verification pins the URL's ORIGIN (scheme+host+effective-port) and FULL path
|
||||
# (deployment prefix + exact owner/repo + kind + number); a bare endswith/suffix
|
||||
# test would wrongly accept the look-alike-host and prefix-injection variants.
|
||||
for bad_mode in comment-url-wrong-host comment-url-wrong-owner comment-url-wrong-repo comment-url-suffix-injection; do
|
||||
if run_review "$bad_mode" comment durable-body; then
|
||||
echo "FAIL: forged comment URL ($bad_mode) was accepted" >&2
|
||||
cat "$OUTPUT_FILE" >&2
|
||||
@@ -938,19 +920,6 @@ run_review comment-mixed-case-slug comment durable-body https://git.mosaicstack.
|
||||
grep -q 'Added and verified comment on Gitea PR #123' "$OUTPUT_FILE"
|
||||
assert_no_temp_leak "comment-mixed-case-slug"
|
||||
|
||||
# Case 15c (#991): the deployment's Gitea ROOT_URL is http:// while every client
|
||||
# reaches it over https://, so the provider returns an http:// pull_request_url
|
||||
# for a comment that is otherwise entirely correct. Same class as 15b — a
|
||||
# legitimate provider response, not a spoof — and a scheme-strict compare
|
||||
# rejects it on EVERY comment, deterministically. That is not a cosmetic false
|
||||
# negative here: on a host where no seat can create a review OBJECT, this
|
||||
# comment-form record is the only gate-16 evidence obtainable, and the wrapper
|
||||
# refuses all of it while the comment sits durably on the PR. Host, path, owner,
|
||||
# repo, kind and number stay strict; only http-vs-https is relaxed.
|
||||
run_review comment-url-scheme-downgrade comment durable-body
|
||||
grep -q 'Added and verified comment on Gitea PR #123' "$OUTPUT_FILE"
|
||||
assert_no_temp_leak "comment-url-scheme-downgrade"
|
||||
|
||||
# Case 16 (#865 ITEM 1, current-head TOCTOU): the PR head advances between the
|
||||
# pre-submit head read (which pins the review) and the post-verify re-read. The
|
||||
# review is genuinely created and verified as pinned to the OLD head, but the
|
||||
|
||||
@@ -25,7 +25,7 @@
|
||||
"lint": "eslint src",
|
||||
"typecheck": "tsc --noEmit",
|
||||
"test": "vitest run --passWithNoTests && pnpm run test:framework-shell",
|
||||
"test:framework-shell": "python3 src/lease-broker/daemon_deadline_unittest.py && python3 src/lease-broker/normative_fragments_unittest.py && python3 src/lease-broker/receipt_challenge_unittest.py && python3 src/lease-broker/context_recovery_unittest.py && python3 src/lease-broker/recovery_runtime_unittest.py && python3 src/lease-broker/recovery_b1_adversarial_unittest.py && python3 src/lease-broker/framework_skill_portability_unittest.py && python3 src/mutator-gate/runtime_tools_unittest.py && python3 src/mutator-gate/runtime_launch_guard_unittest.py && python3 src/mutator-gate/version_coupling_unittest.py && python3 framework/tools/lease-broker/check-runtime-launches.py --root ../.. && bash framework/tools/codex/test-pr-diff-context.sh && bash framework/tools/qa/test-deps-preflight.sh && bash framework/tools/git/test-pr-review-gitea-comment.sh && bash framework/tools/git/test-pr-review-repo-host-override.sh && bash framework/tools/git/test-ci-queue-wait-branch-absent.sh && bash framework/tools/git/test-git-credential-mosaic.sh && bash framework/tools/git/test-gitea-token-identity.sh && bash framework/tools/_scripts/test-install-ordering-guard.sh && bash framework/tools/tmux/agent-send.test.sh && bash framework/tools/wake/test-wake-store-ack.sh && bash framework/tools/wake/test-wake-store-enqueue-race.sh && bash framework/tools/wake/test-wake-digest-hmac.sh && bash framework/tools/wake/test-wake-digest-quarantine.sh && bash framework/tools/wake/test-wake-detector.sh && bash framework/tools/wake/test-wake-fn-oracle.sh && bash framework/tools/wake/test-wake-reconcile.sh && bash framework/tools/wake/test-wake-beacon.sh && bash framework/tools/wake/test-wake-preimage.sh && bash framework/tools/wake/test-wake-install.sh"
|
||||
"test:framework-shell": "python3 src/lease-broker/daemon_deadline_unittest.py && python3 src/lease-broker/normative_fragments_unittest.py && python3 src/lease-broker/receipt_challenge_unittest.py && python3 src/lease-broker/context_recovery_unittest.py && python3 src/lease-broker/recovery_runtime_unittest.py && python3 src/lease-broker/recovery_b1_adversarial_unittest.py && python3 src/lease-broker/framework_skill_portability_unittest.py && python3 src/mutator-gate/runtime_tools_unittest.py && python3 src/mutator-gate/runtime_launch_guard_unittest.py && python3 src/mutator-gate/version_coupling_unittest.py && python3 framework/tools/lease-broker/check-runtime-launches.py --root ../.. && bash framework/tools/codex/test-pr-diff-context.sh && bash framework/tools/qa/test-deps-preflight.sh && bash framework/tools/git/test-pr-review-gitea-comment.sh && bash framework/tools/git/test-pr-review-repo-host-override.sh && bash framework/tools/git/test-ci-queue-wait-branch-absent.sh && bash framework/tools/git/test-ci-queue-wait-parse.sh && bash framework/tools/git/test-git-credential-mosaic.sh && bash framework/tools/git/test-gitea-token-identity.sh && bash framework/tools/_scripts/test-install-ordering-guard.sh && bash framework/tools/tmux/agent-send.test.sh && bash framework/tools/wake/test-wake-store-ack.sh && bash framework/tools/wake/test-wake-store-enqueue-race.sh && bash framework/tools/wake/test-wake-digest-hmac.sh && bash framework/tools/wake/test-wake-digest-quarantine.sh && bash framework/tools/wake/test-wake-detector.sh && bash framework/tools/wake/test-wake-fn-oracle.sh && bash framework/tools/wake/test-wake-reconcile.sh && bash framework/tools/wake/test-wake-beacon.sh && bash framework/tools/wake/test-wake-preimage.sh && bash framework/tools/wake/test-wake-install.sh"
|
||||
},
|
||||
"dependencies": {
|
||||
"@mosaicstack/brain": "workspace:*",
|
||||
|
||||
Reference in New Issue
Block a user