fix(webhook): kms:ViaService pins, https-only delivery, cross-account principal CI pin

GPT-4.1 cross-family review of the policy surface (no BLOCK): FIX applied
to the cross-account shoc-backend-dev Decrypt statement and both Lambda
role KMS grants (the key is only ever used via Secrets Manager); its
invariant-enforcement QUESTION answered durably with
tests/test_cross_account_principal_pin.py (any new foreign IAM principal
in cdk/ fails CI). Scanner mediums fixed: delivery.py and the replay
script now refuse non-https URLs (urllib follows file:// and http://).
SQS metadata-action and dynamodb:ListStreams NITs skipped: standard CDK
grant shapes; ListStreams has no resource-level scoping. The 4 gitleaks
HIGHs on docs/shoc-webhook-test-vectors.json are deliberate non-secrets
(shared receiver-verification vectors) suppressed machine-level with
justification.
This commit is contained in:
Adam Moussa 2026-07-24 15:00:15 -04:00
parent eb56d39b93
commit 2f2fc83a82
No known key found for this signature in database
4 changed files with 109 additions and 2 deletions

View file

@ -477,11 +477,19 @@ def _add_shoc_webhook_emitter(stack, work_orders_table, comments_table, alarm_to
# policy below is the other half; either one alone fails silently at
# the receiver). resources=["*"] is key-scoped, not account-wide --
# KMS key policies only ever apply to this key.
# kms:ViaService pins the grant to Secrets Manager decrypt paths only
# (GPT-4.1 cross-review FIX): a compromised shoc-backend-dev cannot use
# this key for arbitrary KMS operations outside the secret fetch.
shoc_webhook_key.add_to_resource_policy(
iam.PolicyStatement(
actions=["kms:Decrypt"],
principals=[shoc_consumer_principal],
resources=["*"],
conditions={
"StringEquals": {
"kms:ViaService": (f"secretsmanager.{stack.region}.amazonaws.com")
}
},
)
)
@ -577,7 +585,25 @@ def _add_shoc_webhook_emitter(stack, work_orders_table, comments_table, alarm_to
resources=[shoc_hmac_secret.secret_arn],
)
)
shoc_webhook_key.grant_encrypt_decrypt(shoc_hmac_rotator)
# Explicit statement instead of grant_encrypt_decrypt so the grant can
# carry kms:ViaService (cross-review FIX): the rotator only ever touches
# this key through Secrets Manager put/get, never the KMS API directly.
shoc_hmac_rotator.add_to_role_policy(
iam.PolicyStatement(
actions=[
"kms:Decrypt",
"kms:Encrypt",
"kms:GenerateDataKey*",
"kms:ReEncrypt*",
],
resources=[shoc_webhook_key.key_arn],
conditions={
"StringEquals": {
"kms:ViaService": (f"secretsmanager.{stack.region}.amazonaws.com")
}
},
)
)
shoc_hmac_secret.add_rotation_schedule(
"Rotation",
rotation_lambda=shoc_hmac_rotator,
@ -669,7 +695,20 @@ def _add_shoc_webhook_emitter(stack, work_orders_table, comments_table, alarm_to
work_orders_table.grant_stream_read(shoc_emitter)
comments_table.grant_stream_read(shoc_emitter)
shoc_hmac_secret.grant_read(shoc_emitter)
shoc_webhook_key.grant_decrypt(shoc_emitter)
# Explicit statement instead of grant_decrypt so the grant carries
# kms:ViaService (cross-review FIX): the emitter only decrypts this key
# through Secrets Manager GetSecretValue.
shoc_emitter.add_to_role_policy(
iam.PolicyStatement(
actions=["kms:Decrypt"],
resources=[shoc_webhook_key.key_arn],
conditions={
"StringEquals": {
"kms:ViaService": (f"secretsmanager.{stack.region}.amazonaws.com")
}
},
)
)
shoc_emitter_rejected_queue.grant_send_messages(shoc_emitter)
# ------------------------------------------------------------------

View file

@ -114,6 +114,13 @@ def deliver(envelope: dict) -> tuple[str, int]:
Raises RetryableDeliveryError for 429/5xx/timeout/connection failures so
the caller can block the shard (in-order retry, contract section 7).
"""
if not SHOC_WEBHOOK_URL.startswith("https://"):
# Fail closed on any non-HTTPS scheme (file://, http://, ...): the
# URL is operator-set env config, but urllib would happily follow
# other schemes and the HMAC only protects an HTTPS transport.
raise RetryableDeliveryError(
"SHOC_WEBHOOK_URL is not an https:// URL; refusing to deliver"
)
raw_body = json.dumps(envelope).encode("utf-8")
timestamp = int(time.time())
signing_key = _get_hmac_keys()[0]

View file

@ -355,6 +355,10 @@ def main():
if bool(args.work_order_id) == bool(args.since):
sys.exit("ERROR: pass exactly one of --work-order-id or --since.")
if not args.url.startswith("https://"):
# urllib follows file:// and http:// too; the feed is HTTPS-only.
sys.exit("ERROR: --url must be an https:// URL.")
session = boto3.Session(
profile_name=args.profile or "seahaven-prod", region_name=REGION
)

View file

@ -0,0 +1,57 @@
"""Pin the cross-account principal surface of the CDK app.
The SHOC integration deliberately trusts EXACTLY ONE foreign principal:
``arn:aws:iam::396287094661:role/shoc-backend-dev`` (read API resource
policy in procurement_api_stack.py, HMAC secret + KMS grants in
wo_stack.py). Future shoc-backend-staging/-prod roles are each a
deliberate, individually-reviewed policy addition — so any new foreign
account id or role ARN appearing in cdk/ must consciously update this
pin (and go through the mandatory GPT-4.1 cross-family IAM review).
Raised as a QUESTION in the 2026-07-24 cross-family review of the
webhook emitter policy surface: "how is the exact-one-principal
invariant enforced over time?" — this test is the answer.
"""
import re
from pathlib import Path
REPO_ROOT = Path(__file__).resolve().parents[1]
CDK_DIR = REPO_ROOT / "cdk"
# The one foreign principal the app may reference, and the only files
# allowed to reference it.
ALLOWED_FOREIGN_PRINCIPAL = "arn:aws:iam::396287094661:role/shoc-backend-dev"
ALLOWED_FILES = {"procurement_api_stack.py", "wo_stack.py"}
# Accounts that are not "foreign": seahaven-prod (the deploy target).
HOME_ACCOUNTS = {"011934824531"}
_IAM_ARN_RE = re.compile(r"arn:aws:iam::(\d{12}):\S*?(?=[\"'\s])")
def _cdk_sources():
return sorted(CDK_DIR.glob("*.py"))
def test_only_the_pinned_foreign_principal_appears_in_cdk_sources():
findings = []
for path in _cdk_sources():
for match in _IAM_ARN_RE.finditer(path.read_text()):
account = match.group(1)
if account in HOME_ACCOUNTS:
continue
findings.append((path.name, match.group(0)))
unexpected = [
(name, arn)
for name, arn in findings
if arn != ALLOWED_FOREIGN_PRINCIPAL or name not in ALLOWED_FILES
]
assert not unexpected, (
"Unexpected foreign IAM principal(s) in cdk/ — every cross-account "
f"trust addition must update this pin deliberately: {unexpected}"
)
# Both grant sites must still reference the pinned role (deleting one
# half of the secret/KMS grant pair fails silently at the receiver).
assert {name for name, _ in findings} == ALLOWED_FILES