Ver Fonte

Let a post type say which of its fields are for editors only

A page is rarely all public or all private, and until now the only way to hold
one field back was to remember to wrap it in $can_edit in the template. That is
the wrong place for it: a type can be rendered by more than one template,
templates get edited by people who did not write the type, and forgetting is
silent. It is how the connection uri ended up on a public page twice.

Each custom field in the wizard now carries a visibility of its own -- shown to
everyone by default, or to editors only -- recorded as x-tcms-private in the
sidecar. Fields marked private are dropped from the post on its way to the
template, so an author can refer to one in a display template without guarding
it and a reader who cannot edit simply never receives the value.

Redaction happens at the point the data becomes a page, deliberately after any
datasource has run: Trog::DataSource::Virt needs the connection uri to reach a
hypervisor at all, and the guests page is meant to work for logged out
visitors. So the uri does its job and still never reaches them.

hypervisor.tx is regenerated to use this rather than its hand-written guard,
which is the proof it works: the template refers to conn_uri unguarded, an
admin sees it, and a logged out visitor gets nothing, while the guests page
continues to list every guest.

It recurses, since a post's relations are posts too and carry their own private
fields, and it keeps a seen set because a relation can point back at whatever
pulled it in.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U3k2hhSZpcsoe6uBmhF9nB
George Baugh há 2 meses atrás
pai
commit
c8bdde2684

+ 16 - 0
lib/Trog/DataModule.pm

@@ -441,6 +441,22 @@ sub type_meta_for ( $form = '' ) {
     return $cache{$generation}{$type};
 }
 
+=head2 private_fields_for($form)
+
+The fields of a post type which only an editor may see, as declared by
+x-tcms-private in the sidecar.
+
+A field is public unless it says otherwise.  The point of declaring it here
+rather than guarding it in the template is that the value can then be dropped
+before rendering, so no amount of getting a template wrong can leak it.
+
+=cut
+
+sub private_fields_for ( $form = '' ) {
+    my $properties = schema_for($form)->{properties} // {};
+    return [ grep { $properties->{$_}{'x-tcms-private'} } keys(%$properties) ];
+}
+
 =head2 relations_for($form)
 
 The x-tcms-relations table for a post type, as a hashref keyed on the template

+ 47 - 2
lib/Trog/Routes/HTML.pm

@@ -1497,6 +1497,16 @@ sub posts ( $query, $direct = 0 ) {
 
 
     #XXX the only reason this is needed is due to direct=1
+    # Last thing before the data becomes a page: anything a type marks private
+    # goes no further for a reader who cannot edit.  Deliberately after the
+    # datasource has run -- a datasource may well need a private field, like
+    # the connection uri it takes to reach a hypervisor, to produce anything
+    # at all.
+    if ( !$is_admin ) {
+        my %seen;
+        _redact_private( $_, \%seen ) foreach ( @posts, $query->{primary_post} );
+    }
+
     #XXX is this even used?
     my $content = Trog::Renderer->render(
         template => 'posts.tx',
@@ -1553,6 +1563,35 @@ sub _themed_title ($path) {
     return $Theme::paths{$path} ? $Theme::paths{$path} : $path;
 }
 
+=head2 _redact_private($post, $seen)
+
+Drop the fields a post type marks private, for a reader who isn't an editor.
+
+Done to the data on its way to the template rather than with a guard inside the
+template: a type can be rendered by more than one template, templates get
+edited by people who did not write the type, and a value that was never handed
+over cannot be printed by mistake.  Recurses, because a post's relations are
+posts too and carry their own private fields.
+
+=cut
+
+sub _redact_private ( $post, $seen = {} ) {
+    return unless Ref::Util::is_hashref($post);
+
+    # Relations can point back at what pulled them in.
+    return if $seen->{$post}++;
+
+    if ( $post->{form} ) {
+        delete $post->{$_} foreach @{ Trog::DataModule::private_fields_for( $post->{form} ) };
+    }
+
+    foreach my $value ( values(%$post) ) {
+        if    ( Ref::Util::is_hashref($value) )  { _redact_private( $value, $seen ) }
+        elsif ( Ref::Util::is_arrayref($value) ) { _redact_private( $_,     $seen ) foreach @$value }
+    }
+    return;
+}
+
 =head2 _datasource_posts($query, $posts)
 
 Swap a series' children for whatever its datasource says they are, if its child
@@ -2302,8 +2341,9 @@ sub _wizard_fields ($query) {
     my $labels = Trog::Utils::coerce_array( $query->{param_label} );
     my $phs    = Trog::Utils::coerce_array( $query->{param_placeholder} );
     my $reqs   = Trog::Utils::coerce_array( $query->{param_required} );
-    my $rforms = Trog::Utils::coerce_array( $query->{param_relation_form} );
-    my $rmodes = Trog::Utils::coerce_array( $query->{param_relation_mode} );
+    my $rforms  = Trog::Utils::coerce_array( $query->{param_relation_form} );
+    my $rmodes  = Trog::Utils::coerce_array( $query->{param_relation_mode} );
+    my $private = Trog::Utils::coerce_array( $query->{param_private} );
 
     my ( @fields, %seen );
     foreach my $index ( 0 .. $#$names ) {
@@ -2336,6 +2376,7 @@ sub _wizard_fields ($query) {
                 required      => _wizard_scalar( $reqs->[$index] ) ? 1 : 0,
                 relation_form => $rform,
                 relation_mode => $rmode,
+                private       => _wizard_scalar( $private->[$index] ) ? 1 : 0,
             }
         );
     }
@@ -2373,6 +2414,10 @@ sub _wizard_sidecar ( $name, $fields, $includes, $body_form ) {
         $property->{'x-tcms-label'} = $field->{label};
         $property->{'x-tcms-placeholder'} = $field->{placeholder} if length $field->{placeholder};
 
+        # Only written when it is true: a field is public unless it says so,
+        # and a sidecar full of x-tcms-private:false reads like the opposite.
+        $property->{'x-tcms-private'} = JSON::MaybeXS::true() if $field->{private};
+
         if ( $field->{type} eq 'relation' ) {
             $property->{'x-tcms-relation-form'} = $field->{relation_form};
 

+ 54 - 9
t/Trog-DataSource-Virt.t

@@ -5,6 +5,7 @@ use Test::More;
 use Test::MockModule qw{strict};
 use Test::Fatal qw{exception};
 use Path::Tiny();
+use JSON::MaybeXS();
 use FindBin;
 
 use lib "$FindBin::Bin/../lib";
@@ -289,18 +290,62 @@ subtest 'guests show nothing an unprivileged viewer cannot use' => sub {
     }
 };
 
+subtest 'private fields are dropped before a non-editor sees them' => sub {
+    require Trog::Routes::HTML;
+    require Trog::DataModule;
+
+    my $dmmock = Test::MockModule->new('Trog::DataModule');
+    $dmmock->redefine( private_fields_for => sub { return $_[0] eq 'secretive.tx' ? ['hidden'] : [] } );
+
+    my $post = {
+        form   => 'secretive.tx',
+        title  => 'a post',
+        hidden => 'the secret',
+        shown  => 'not a secret',
+
+        # A relation is a post in its own right, with its own private fields.
+        related => { form => 'secretive.tx', title => 'related', hidden => 'another secret' },
+        many    => [ { form => 'secretive.tx', hidden => 'a third secret' } ],
+        other   => { form => 'ordinary.tx',   hidden => 'not declared private here' },
+    };
+
+    Trog::Routes::HTML::_redact_private($post);
+
+    ok( !exists $post->{hidden},              'the private field is gone' );
+    is( $post->{shown}, 'not a secret',       'and the public ones are not' );
+    is( $post->{title}, 'a post',             '...including the ones every post has' );
+    ok( !exists $post->{related}{hidden},     'a related post is redacted too' );
+    ok( !exists $post->{many}[0]{hidden},     'and so is one reached through a list' );
+    is( $post->{other}{hidden}, 'not declared private here', 'a type that declares nothing keeps everything' );
+};
+
+subtest 'a post that refers to itself does not hang the redactor' => sub {
+    require Trog::Routes::HTML;
+    require Trog::DataModule;
+
+    my $dmmock = Test::MockModule->new('Trog::DataModule');
+    $dmmock->redefine( private_fields_for => sub { return ['hidden'] } );
+
+    my $post = { form => 'loop.tx', hidden => 'secret' };
+    $post->{itself} = $post;
+
+    # A relation can point back at whatever pulled it in.
+    is( exception { Trog::Routes::HTML::_redact_private($post) }, undef, 'it terminates' );
+    ok( !exists $post->{hidden}, 'having done the job' );
+};
+
 subtest 'a hypervisor does not show its connection uri to everyone' => sub {
-    my $form = Path::Tiny->new("$FindBin::Bin/../www/templates/html/components/forms/hypervisor.tx")->slurp_utf8;
+    require Trog::DataModule;
 
     # The uri carries a host and a username, and these pages are public.  It is
-    # rendered in the display half, so it needs its own guard -- the edit half's
-    # is too late.
-    my ($display) = $form =~ m/:\s*if\s*\(\s*!\$post\.addpost\s*\)\s*\{(.*?):\s*\}/s;
-    ok( defined $display, 'the template has a display half' ) or return;
-
-    like( $display, qr/\$post\.conn_uri/, 'which mentions the connection uri' );
-    like( $display, qr/:\s*if\s*\(\s*\$can_edit\s*\).*\$post\.conn_uri/s,
-        'behind a $can_edit guard, so only an admin sees it' );
+    # declared private rather than guarded in the template, so the value is
+    # dropped before rendering and the template needs no guard of its own.
+    my $private = Trog::DataModule::private_fields_for('hypervisor.tx');
+    is_deeply( $private, ['conn_uri'], 'the connection uri is declared private' );
+
+    my $sidecar = JSON::MaybeXS::decode_json(
+        Path::Tiny->new("$FindBin::Bin/../www/templates/html/components/forms/hypervisor.json")->slurp_utf8 );
+    ok( $sidecar->{properties}{conn_uri}{'x-tcms-private'}, 'and says so in its sidecar' );
 };
 
 done_testing();

+ 63 - 0
t/Trog-Routes-HTML-series-lifecycle.t

@@ -447,6 +447,69 @@ subtest 'the post wizard invents a new post type' => sub {
     foreach my $gone (qw{inc_title_input inc_visibility inc_acls}) {
         unlike( $body, qr/name="\Q$gone\E"/, "the wizard no longer offers '$gone' as a choice" );
     }
+
+    like( $body, qr/name="param_private"/, 'and it offers per-field privacy' );
+};
+
+subtest 'a field can be declared editors-only' => sub {
+    my ( $code, $body, $err ) = _render(
+        _admin(
+            route             => '/admin/wyzzerdd/save',
+            method            => 'POST',
+            name              => 'spec_secretive',
+            body_form         => 'form_common.tx',
+            wrapper           => 1,
+            inc_post_title    => 1,
+            inc_tags          => 1,
+            param_name        => [ 'shown',            'hidden' ],
+            param_type        => [ 'text',             'text' ],
+            param_label       => [ 'Shown',            'Hidden' ],
+            param_placeholder => [ '',                 '' ],
+            param_required    => [ 0,                  0 ],
+            param_private     => [ 0,                  1 ],
+            param_relation_form => [ 'blog.tx',        'blog.tx' ],
+            param_relation_mode => [ 'one',            'one' ],
+
+            # Deliberately unguarded: declaring the field private is supposed
+            # to be enough, so an author does not have to remember.
+            display => q{<div class="s"><: $post.shown :></div><div class="h"><: $post.hidden :></div>},
+        ),
+        \&Trog::Routes::HTML::post_wizard_save,
+    );
+    is( $code, 200, 'the type was created' ) or diag($err);
+
+    my $sidecar = JSON::MaybeXS::decode_json(
+        Path::Tiny->new('www/templates/html/components/forms/spec_secretive.json')->slurp_utf8 );
+    ok( $sidecar->{properties}{hidden}{'x-tcms-private'},   'the private field is marked so' );
+    ok( !exists $sidecar->{properties}{shown}{'x-tcms-private'}, 'and a public one carries no flag at all' );
+
+    _reindex();
+
+    _make_series( 'spec_secretive', 'specsecretive', 'spec_secretive.tx', 'Spec Secretive Series', no_topbar => 1 ) or return;
+    my $child = _save_post(
+        'spec_secretive child',
+        form       => 'spec_secretive.tx',
+        title      => 'Spec Secretive Child',
+        shown      => 'public value',
+        hidden     => 'private value',
+        visibility => 'public',
+        tags       => ['specsecretive'],
+        to         => '/specsecretive',
+    ) or return;
+
+    is( $child->{hidden}, 'private value', 'the value is stored like any other' );
+
+    # An editor sees both.
+    my ( $acode, $abody ) = _render( _admin( route => '/specsecretive' ), \&Trog::Routes::HTML::series );
+    is( $acode, 200, 'the series renders for an admin' );
+    like( $abody, qr{<div class="h">private value</div>}, 'who sees the private field' );
+
+    # A logged out reader sees only the public one, without the template
+    # having guarded anything.
+    my ( $ncode, $nbody ) = _render( _anon( route => '/specsecretive' ), \&Trog::Routes::HTML::series );
+    is( $ncode, 200, 'and for everyone else' );
+    like( $nbody, qr{<div class="s">public value</div>}, 'who still sees the public field' );
+    unlike( $nbody, qr/private value/, 'but not the private one' );
 };
 
 #--------------------------------------------------------------------------

+ 2 - 1
www/templates/html/components/forms/hypervisor.json

@@ -4,7 +4,8 @@
          "type" : "string",
          "x-tcms-input" : "url",
          "x-tcms-label" : "Libvirt Connection URI",
-         "x-tcms-placeholder" : "libvirt://localhost"
+         "x-tcms-placeholder" : "libvirt://localhost",
+         "x-tcms-private" : true
       }
    },
    "required" : [

+ 0 - 2
www/templates/html/components/forms/hypervisor.tx

@@ -4,11 +4,9 @@
         : include "post_title.tx";
         : include "post_tags.tx";
 <div class="postData responsive-text">
-: if ( $can_edit ) {
     Connection URI:<br />
     <a href="<: $post.conn_uri :>"><: $post.conn_uri :></a>
     <br />
-: }
     <: render_it($post.data) | mark_raw :>
 </div>
 

+ 8 - 0
www/templates/html/components/post_wizard.tx

@@ -55,6 +55,10 @@ any custom fields, so that the data model knows to keep them when a post is save
     as <code>&lt;: $post.your_field_name :&gt;</code>.
     Uploads aren't supported here &mdash; use the Attachment uploader above for those.
     <br />
+    A field set to <em>editors only</em> is dropped before the page is rendered for anyone who
+    cannot edit it, so it is safe to refer to in the display template below without guarding it
+    yourself &mdash; useful for a connection string or anything else that shouldn't be public.
+    <br />
     An <em>Another Post</em> field depends on a different post type.
     <em>Pick one</em> stores a single post's ID, chosen from a dropdown in the editor, and hands you
     the whole post as <code>&lt;: $post.your_field_name_post :&gt;</code>.
@@ -113,6 +117,10 @@ any custom fields, so that the data model knows to keep them when a post is save
             <option value="0">Optional</option>
             <option value="1">Required</option>
         </select>
+        <select class="cooltext" name="param_private">
+            <option value="0">Shown to everyone</option>
+            <option value="1">Shown to editors only</option>
+        </select>
         <button type="button" class="coolbutton" onclick="delParam(this); return false;">Remove</button>
     </div>
 </template>