Themes projects no longer show images, other cvs.drupal.org links may be broken as well.

Is there a plan to fix this vulnerability or should we start migrating to a new solution?

The vulnerability has been fixed. We need a new way to allow for themes to show their screenshots.

Just some alternatives in case people don't like the idea of attaching images to an issue somewhere:

1. As attachments on the project nodes.
2. As images in the themes gallery: http://drupal.org/image/tid/39
3. Using the "image attach" feature of image module (image module is running on d.o, image attach is not)
4. Installing cck and imagefield and using that

Putting them in the themes gallery is the one I like the most personally because then that gallery will represent the themes and people can browse that. This may require a permission change so that anyone with "create project" can also "create image". I don't know what all the roles are.

Please don't design a new system with the same evil workflow. If we're changing this, let's fix http://drupal.org/node/49575, too. See also http://groups.drupal.org/node/6186.

Every project node should have a small gallery of images you can associate with it, not just themes. There should be a way to select a specific image as the "teaser image" for each project, which is displayed with the teaser on browsing views. Manually placing img tags and tweaking the filter to full HTML involves *MASSIVE* amounts of manual effort from d.o maintainers, confusion for theme maintainers, and absolutely must go.

Attaching to images seems like a hack, and doesn't fix the workflow. Attachments to the project is better, but still doesn't solve the img tags. images in the themes gallery would limit it to themes, or require more categorization for the galleries (== more work for d.o admins), and *still* doesn't solve img tags.

image_attach or CCK imagefield seem like the best, most flexible solutions. image_attach could work, since we're already running image on d.o, so it's not a big stretch. d.o doesn't currently run CCK, so it wouldn't just be imagefield we'd need to install. :(

Basically, I'm torn on those 2 options. So, I welcome input from killes (d.o maintainer), heine (security team head), and anyone else. But, I'm strongly opposed to all the other proposed solutions.

Thanks,
-Derek

I forgot: CCK would be great to have on d.o, anyway (in 6.x-2.* we're planning to port big chunks of project* to CCK). This certainly seems like the direction for the future of image handling. For those reasons, I'd be in favor of installing CCK and image_field for this, but I wouldn't object to sticking with image + image_attach for now if others thought that was better as a first step...

I don't want to install cck/imagefield on d.o now since it would only serve a single purpose.

I think that the images need to be attached to the project nodes.

The problem is that there is no "upload stuff to your own nodes" permission.

I don't know image_attach.

I had a look at image_attach and I think it is good fit.

The only problem I see is that we'd need to add all cvs account holders to a role that would allow them to upload images. I think we could put this in drupalorg module and do a one-time script for existing accounts.

Also, we'd need to check how good this looks on project pages.

I have installed and configured image attach on scratch.d.o and attached an image to event.module's project page:

http://scratch.drupal.org/project/event

Not too bad.

Perfect. image_attach seems great for now. we can always migrate to CCK + image_field later if we start needing CCK for other things. The idea for an auto-populated role for all CVS account holders has come up before, and your plan sounds like a good one. Any tweaks for bluebeach we'd need for either project browsing pages or project nodes should be relatively easy. Yay, one of my oldest d.o gripes being solved! ;)

Woot! I was about to say the same: there was so many hours of drupal.org admin time spent with setting back and forth from full HTML on the project nodes. All those time would solve this shortcoming if the time would have been spent on fixing the bug. I'm more than glad that this is on track now :)

I'm a little late on this and don't want to take away from the image_attach solution but this got me thinking. Maybe we should give all project maintainers documenter access? That gives them the rights to post images as well as makes it easier for them to care for their project's documentation. Not long ago, I had to go through a maintainer's massive documentation and change all the names to the person who wanted to help him out. Ideally that person would have joined the doc team but he didn't because he was only helping with that one thing. If the maintainer had been on the doc team, he could have handled it himself.

Just a thought. :)

Michelle

The code for adding cvs users to a special role exists now and has been verified to work. They'll also get removed from the role again if their cvs access should be revoked. All we need is a decision which role we should use. As I see it there are two proposals:

1) use a dedicated role for this

2) use the docs maintainer role.

I'd prefer 2) since we already have a lot of roles.

Well, if we're willing to give everyone with a CVS account perms with the doc maintainers role, and full access to the img tag, then we don't need image_attach, and we could have solved this issue over a year ago over at http://drupal.org/node/49575. The whole bother with this issue, all along, is that we didn't think we do enough to validate and authenticate people with CVS accounts, and we couldn't just trust all of them with an img tag. So, I'm pretty strongly opposed to adding everyone to the doc maintainer role for the following reasons:

a) fine grained perms and roles are better -- we have a lot of roles on d.o b/c we have a lot of classes of users. it's no big deal that the permission page gets a little wider.

b) it's safer for d.o, since we don't do enough to authenticate/validate all CVS account holders.

c) it would have been a colossal waste of time (both my own, and dozens of hours of d.o maintainer time), if we decided at this point to punt and give everyone with a CVS account full access to img tags.

Let's just make a new role for this. We can even give this role limited doc powers, like the ability to edit any book pages that aren't at full html or php, etc, if we want to encourage more of them to join in documentation efforts. That's easily tracked, and the harm they can do is limited to vandalism, not XSS.

p.s. @killes: where's the patch for this code to automatically tie CVS accounts to a role? i didn't see an issue about it. is it a patch against http://drupal.org/project/drupalorg or http://drupal.org/project/cvslog? originally, i assumed drupalorg, but if it was flexible enough to let you select the role to tie it to, i could see this feature being useful for anyone using cvslog. thanks.

@dww: Sorry, I hadn't created an issue, the patch is here:

http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/drupalorg/d...

I hadn't thought that the docs maintainers all had the right to use img tags. I agree that we shouldn't hand this out to all cvs people.

So we'll have to use an extra role.

The question is if we should ask the docs people to use image attach instead of img tags too.

re: the patch: ok, thanks for the pointer.

re: doc maintainers and the img tag, behold:
http://drupal.org/admin/settings/filters/5/configure
"Allowed HTML tags" definitely includes <img>.

Yes, definitely. When we modify the theme pages to show the screenshots, we change it to the doc maintainers input format. That was my whole reasoning behind suggesting CVS holding people are made doc maintainers. I didn't realize there was an issue with them having access to the img tag. No, they aren't really screened other than making sure they have something of value to contribute. Then again, I don't think doc maintainers are, either.

Anyway, I don't want to derail all the work you put into image assist. I was just offering an alternate. :)

Michelle

I've installed the module on d.o and conbfigured it. Now we need some SQL to give the active CVS users this role.

INSERT INTO users_roles (uid, rid) SELECT DISTINCT(uid), 8 FROM cvs_messages WHERE created > 1167609600;

This code has been run (thanks ajk!) and the role has the "create image nodes" and "edit own image nodes" permission.

Now let's see what happens.

Could someone make the image node type not allow comments by default? Otherwise all these screenshots are open for commenting.

Thanks,

Michelle

@Michelle - seems reasonable, done.

sorry, just saw this. image_attach is a horribly maintained module. i think james would agree. last i checked, it is impossible to delete an attached image. so, expect some support requests any minute now.

Erm... Yikes... I didn't notice that. I did a sorta crappy screenshot of my module just to be able to do a walk through before I made the announcement post for killes. I didn't realize I was going to be stuck with that image forever. :(

I think we need some FR's implemented on image_attach real quick like. LOL

Michelle

People should be able to edit images they uploaded. they should thus be able to exchange crappy screenshots for better ones.

People should be able to edit images they uploaded.

The method to edit the image is not readily apparent. I can’t edit the image directly; i.e. there are no view/edit tabs on the image node: http://drupal.org/node/205369

So the only way to edit the image is while editing the project. However, the Attached Images fieldset only shows a “Upload Image:” widget underneath the existing image. There's no indication that uploading a new image will replace the original image. A simple field description would prevent any confusion.

Also, it should be noted that if you want multiple screenshots, you'll still need to use the old documentation-input-filter/attach-image-to-issue method.

You can't edit the image because it belongs to the author of the project node, jjeff.

I've actually no idea what happens when you upload a second screenshot...

Multiple screenshots aren't supported, that's true.

Here is one affected handbook page; http://drupal.org/node/9068

Due to some security auditing by Bart Jansens and myself we were able to switch on the viewcvs checkout view again. This means that all screenshots etc will work again.

Theme images will continue to use the image_attach module since this saves the admins a lot of time.

The handbook page above (http://drupal.org/node/9068) is fixed -- thanks.