POST endpoint for PBS

[ekoslow1-creator]

Description

POST request for sefaria.org/api/powered-by which is also specifically compatible with Formstack submission source

Code Changes

Updates to sefaria/powered_by/views.py

[gitvelocity-reviewer]

📊 Code Quality Score: 50/100

Base Score 62 × ESF 0.8 = 49.6, rounded to 50

Category	Score	Factors
🔭 Scope	12/20	Two files are modified: views.py grows from 22 to 298 lines with a new POST handler, a Formstack translation layer, and a validation function; powered_by_api_test.py adds 474 lines of tests. The change introduces one new public API endpoint (POST /api/powered-by) within the single powered_by subsystem.
🏗️ Architecture	10/20	clean_and_default_post_body is extracted as a standalone validation function that returns (cleaned, error) tuples, and translate_formstack_payload is introduced as a translation adapter between Formstack field IDs and Project field names. _writable_char_field_max_lengths introspects Project._meta at module load time to derive length constraints. No new service dependency is added; update_or_create replaces a hypothetical separate create/update split.
⚙️ Implementation	13/20	clean_and_default_post_body iterates WRITABLE_FIELDS and applies six distinct validation branches per field: choice validation via SubmissionSource/TechnicalExperience enums, boolean type enforcement, list type enforcement, URL validation via URLValidator, ISO 8601 datetime parsing via parse_datetime, and email validation via validate_email. translate_formstack_payload normalizes checkbox values through _formstack_checkbox_values, which handles None, list, and comma-separated str inputs, and merges 13 endpoint-specific Formstack fields into a single sefaria_tools_used list. _powered_by_post detects Formstack payloads by the presence of FormID and applies conditional defaulting of submission_source and submission_date only on create.
⚠️ Risk	14/20	@csrf_exempt is applied to the entire powered_by_api view, removing CSRF protection from the POST path for all callers, not just Formstack. The Formstack detection heuristic ('FormID' in body) is spoofable by any anonymous caller. The commented-out else: cleaned['is_published'] = False block — explicitly marked 'DO NOT COMMIT' — is absent from the committed code, leaving cleaned.setdefault('is_published', True) active on updates, which allows an anonymous caller to POST an update to a published project's link and keep it published without staff review. The upsert path has no concurrency guard around the filter().exists() / update_or_create sequence.
✅ Quality	11/15	powered_by_api_test.py adds 18 unit tests for clean_and_default_post_body, 8 unit tests for translate_formstack_payload, and 15 @pytest.mark.django_db integration tests for the POST endpoint covering create, update, idempotency, field preservation, staff vs. anonymous response shaping, and HTTP method restriction. Two tests (test_post_update_unpublishes_previously_published_project and test_post_update_preserves_staff_only_fields) assert is_published is False on update, which contradicts the current implementation and would fail. Imports of clean_and_default_post_body and translate_formstack_payload appear mid-file rather than at the top. No test exercises _formstack_checkbox_values with a None input directly, and no test covers the submitter or salesforce_id writable fields.
🔒 Perf / Security	2/5	Field allowlisting via WRITABLE_FIELDS prevents arbitrary model field writes. URLValidator, validate_email, and parse_datetime provide input validation on the relevant fields. @csrf_exempt removes CSRF protection from the POST path, and there is no Formstack webhook signature verification, so the FormID detection path is open to spoofing by any caller.

Was this score accurate? 👍 Yes · 👎 No

How this was scored →

_{Scored by GitVelocity · How are scores calculated?}

[Copilot]

Pull request overview

Adds unauthenticated POST/upsert support for Powered by Sefaria projects, including Formstack payload translation.

Changes:

• Adds validation, Formstack mapping, and project upserts.
• Expands API tests for creation, updates, and validation.
• Documents the endpoint design and implementation plan.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
powered_by/views.py	Implements POST handling and Formstack translation.
powered_by/tests/powered_by_api_test.py	Tests POST behavior and validation.
docs/superpowers/specs/2026-08-12-powered-by-post-endpoint-design.md	Documents the API design.
docs/superpowers/plans/2026-08-12-powered-by-post-endpoint.md	Records the implementation plan.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[saengel]

Let's use an ID instead.

[saengel]

This is resolved by us removing the ability to upsert for now.

[saengel]

Still needs some polishing, great progress in the right direction.

[saengel]

Why are these fields separated out? Shouldn't they all live in the same dict?

[saengel]

This makes sense to keep separate

[saengel]

This comment needs to be cleaned up

[saengel]

Let's use an ID instead.

[saengel]

This is resolved by us removing the ability to upsert for now.

[saengel]

Looks like Formstack can accept some kind of key passed in the headers for webhooks (see: https://help.formstack.com/hc/en-us/articles/44592107329683-Webhooks) let's also add a key of sorts to avoid any un-authenticated POST issues.

[yitzhakc]

Overall looks good. I would ideally want a way for us to be notified if we're getting invalid submissions from Formstack in a situation where their fields or structure changes, but that can be a separate story.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

powered_by/views.py:275

• HandshakeKey is attacker-controlled Unicode, but hmac.compare_digest only accepts ASCII when comparing str values. A request such as {"HandshakeKey": "é"} therefore raises TypeError and returns 500 instead of 401. Compare UTF-8 bytes (and validate the configured value's type) so every invalid key is rejected cleanly.

    expected = getattr(settings, "FORMSTACK_HANDSHAKE_KEY", None)
    provided = body.get("HandshakeKey") if isinstance(body, dict) else None
    return bool(expected) and isinstance(provided, str) and hmac.compare_digest(provided, expected)

[yodem]

Independent review pass on the handshake implementation, run with multi-agent verification and every claim re-checked against commit 3e20344c before posting. Findings below are only the ones not already covered by Copilot or @saengel, and each was executed or grepped rather than reasoned about.

Two things I checked and am NOT reporting, because they turned out to be wrong:

• Non-string image_url / project_source_code causes a 500. Refuted — Django 6.0.4's URLValidator.__call__ opens with if not isinstance(value, str) ... raise ValidationError, so it's caught at views.py:206-208 and returns a clean 400. (This was true of the creator_email path before 3e20344 fixed it; it was never true of the URL fields.)
• The handshake key gets captured by request logging. Refuted for this deployment — sefaria/system/logging.py:decompose_request_info binds only requestUrl and requestMethod, no body, and no Sentry/APM body capture is configured. The generic risk of a body-carried secret stands, but there is no live exposure here.

The design call itself (handshake key rather than the HMAC signature Formstack also offers) is defensible: the credential is fail-closed, compared with compare_digest, and auth sits inside the POST branch so the public GET is untouched. The findings below are about the edges around it, not the choice.

[yodem]

submitter, salesforce_id and notes are writable here, but powered_by/models.py:43-46 declares all three in PRIVATE_FIELDS — "PII / internal staff metadata." They're stripped on read and settable on write.

The risk is trust rather than data loss: a forged notes reads in the admin as if a colleague already vetted the submission, and salesforce_id is a soft FK something may cross-reference. is_published/featured/tags/status are correctly excluded.

Suggest dropping submitter and salesforce_id from WRITABLE_FIELDS.

[yodem]

Worth settling before the webhook goes live: per Formstack's docs, "Post using field names" is the default and field IDs are an opt-in checkbox (postDataFieldKeys: field_names | field_ids | api_friendly_field_names | ...).

Left on the default, keys arrive as labels ("Project Name"), this lookup finds nothing, project_link is missing, and the whole submission 400s — every real submission dropped.

Not a code change: a Formstack config setting plus one captured real payload to test the map against.

[yitzhakc]

Did we check this with Elise? @ekoslow1-creator

[yodem]

Blocker — this setting never reaches any k8s environment, so every POST 401s on cauldron, staging and prod.

sefaria/settings.py:362-370 imports sefaria.local_settings, which in k8s is rendered from helm-chart/sefaria/templates/configmap/local-settings-file.yaml and mounted with subPath: local_settings.py (rollout/web.yaml:210-213). That template has no FORMSTACK line, so getattr(...) at views.py:275 returns None and line 286 returns 401 — even once the cluster secret exists, because nothing turns the env var into a Django setting.

Fix: add the os.getenv line to local-settings-file.yaml beside WEBHOOK_USERNAME (~line 399). Cluster secrets are staged in Sefaria/infrastructure#671 (dev) and #672 (prod, draft).

[yodem]

These tests never run in CI, so a green pytest-job here says nothing about them.

build/ci/createJobFromRollout.sh:27 runs pytest ... ./sefaria ./sso ./reader — no ./powered_by — and pytest.ini's python_files has no powered_by pattern, so the file is excluded twice. The job is triggered; it just collects nothing from here.

Pre-existing, but this PR adds ~470 lines of security-relevant tests into the blind spot. Fix: add ./powered_by to the pytest args and powered_by/tests/*_test.py to python_files.

[yodem]

Recommendation: switch the auth to Formstack's HMAC signature

I should have raised this in my review above instead of reviewing around it — apologies. The design we agreed on was Formstack's HMAC signature, and this PR implements the handshake key instead. They're two separate Formstack features (hmacSecret + customHmacHeader vs sharedSecret), and git grep -i "x-fs\|signature" -- powered_by/ on this branch returns nothing.

Both are shared-secret schemes, so this isn't "simple vs. enterprise." The difference is where the secret travels:

	Handshake key (as built)	HMAC signature
Secret on the wire	every request, cleartext	never — only a digest
A stolen credential lets an attacker…	forge unlimited new submissions	replay only the one payload it signed
Payload tampering	undetectable	detected
Authenticate before parsing	impossible by design	yes — verify raw bytes first
Lines of code	~4	~8

The second row is the crux: a handshake key is a bearer secret, so anyone who sees one delivery can mint submissions indefinitely. A signature is bound to the bytes it signed.

It also removes two real bugs rather than patching them. I originally filed both as inline comments and have deleted them, because switching makes them vanish:

1. json.loads currently runs before the auth check (unavoidable — the credential is inside the JSON). A ~20 KB deeply-nested body raises RecursionError, which isn't in the except clause and isn't caught by catch_error_as_json → unauthenticated 500, verified end-to-end with django.test.Client. HMAC verifies request.body bytes first, so an unsigned request never reaches the parser.
2. hmac.compare_digest on str requires ASCII on both sides; a HandshakeKey of "café" raises TypeError → another unauthenticated 500 (also verified). Comparing hex digests makes that unreachable.

Sketch

def _formstack_signature_ok(request):
    secret = getattr(settings, "POWERED_BY_FORMSTACK_HMAC_SECRET", None)
    sent = request.headers.get("X-FS-Signature", "")
    if not secret or not sent:
        return False
    # request.body = the exact bytes received; never re-serialise a parsed dict
    digest = hmac.new(secret.encode(), request.body, hashlib.sha256).hexdigest()
    return hmac.compare_digest(f"sha256={digest}", sent)


def _powered_by_post(request):
    if not _formstack_signature_ok(request):          # BEFORE json.loads
        return jsonResponse({"error": "Unauthorized"}, status=401)
    try:
        body = json.loads(request.body)
    ...

Formstack side: set HMAC Key (not Shared Secret) and keep Payload Format = JSON — with urlencoded, touching request.POST first consumes the stream and breaks verification. Cluster secrets are already staged under this name in Sefaria/infrastructure#671 (dev) and #672 (prod, draft), so they merge unchanged.

What this does not solve

Formstack signs the body with no timestamp, so a captured signed request stays replayable indefinitely. Idempotency on the Formstack submission ID is still needed either way — which is the same change as @saengel's "let's use an ID instead."

Happy to push this as a commit on the branch if that's easier than reworking it.

[ekoslow1-creator]

Changed from Handshake to HMAC key. One thing worth flagging: the infra tickets (Sefaria/infrastructure#671 dev, #672 prod) were staged for a FORMSTACK_HANDSHAKE_KEY secret — those need to be updated to provision POWERED_BY_FORMSTACK_HMAC_SECRET instead, or the endpoint will 401 in those environments the same way as before.

[yitzhakc]

Did we check this with Elise? @ekoslow1-creator

[gitvelocity-reviewer]

📊 Code Quality Score: 50/100

63 × 0.8 (Large ESF) = 50

Category	Score	Factors
🔭 Scope	12/20	Seven files are modified: powered_by/views.py gains the POST path; powered_by/tests/powered_by_api_test.py gains ~400 lines of tests; pytest.ini and build/ci/createJobFromRollout.sh register the powered_by test directory; and POWERED_BY_FORMSTACK_HMAC_SECRET is added to helm-chart/sefaria/templates/configmap/local-settings-file.yaml, sefaria/local_settings_ci.py, sefaria/local_settings_coolify.py, and sefaria/local_settings_example.py.
🏗️ Architecture	10/20	powered_by/views.py is restructured from a single @require_GET function into a dispatcher (powered_by_api) that delegates to _powered_by_post, with clean_and_default_post_body and translate_formstack_payload extracted as pure functions and _formstack_signature_ok as a helper. The view gains @csrf_exempt to accept the Formstack webhook. No new module boundary or external service dependency is introduced.
⚙️ Implementation	14/20	clean_and_default_post_body enforces an allowlist (WRITABLE_FIELDS), required-field checks, type guards for booleans and lists, URL validation via Django's URLValidator, ISO 8601 datetime parsing via parse_datetime, email validation with a non-string guard to avoid TypeError from validate_email, and per-field max_length introspected from Project._meta at import time via _writable_char_field_max_lengths. translate_formstack_payload normalizes Formstack's field-ID-keyed payload (handling Field, field, and bare-ID key variants, plus list-vs-comma-string checkbox values from _formstack_checkbox_values) into the clean-field dict. _formstack_signature_ok uses hmac.compare_digest and runs before json.loads.
⚠️ Risk	10/20	The POST path is @csrf_exempt, which is appropriate for a webhook but widens the attack surface if POWERED_BY_FORMSTACK_HMAC_SECRET is leaked. The no-upsert design (every POST inserts a new row) prevents project_link-keyed overwrites of existing published projects. getattr(settings, 'POWERED_BY_FORMSTACK_HMAC_SECRET', None) returns 401 rather than crashing when the env var is absent. No database migration is included; the model fields already exist.
✅ Quality	13/15	powered_by/tests/powered_by_api_test.py adds 18 unit tests for clean_and_default_post_body (including the non-string email edge case documented in a comment), 8 unit tests for translate_formstack_payload covering field mapping, boolean conversion, checkbox normalization, and omission semantics, and 17 integration tests for the POST view covering 201 creation, 400 validation errors, 401 HMAC rejection for missing/wrong/unconfigured secrets, staff vs. anonymous field visibility, no-upsert behavior, and HTTP method restriction. The formstack_hmac_secret fixture is autouse=True. No test covers the Helm chart or CI script changes.
🔒 Perf / Security	4/5	_formstack_signature_ok uses hmac.compare_digest for constant-time comparison and runs before json.loads so unsigned bodies never reach the parser. POWERED_BY_FORMSTACK_HMAC_SECRET is sourced from env and absent from source control. WRITABLE_FIELDS allowlist prevents mass-assignment of is_published, featured, tags, and is_buggy.

Was this score accurate? 👍 Yes · 👎 No

How this was scored →

_{Scored by GitVelocity · How are scores calculated?}